-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update caching docs #1818
base: source
Are you sure you want to change the base?
Update caching docs #1818
Conversation
@mandiwise is attempting to deploy a commit to the The GraphQL Foundation Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's keep a dedicated caching page on that page we can talk about different caching techniques used within community: Document Caching, Response Caching, Partial Query Caching.
We can talk about performance and add more things to that page. The performace page can link to the caching page. Performance is a broader topic, we can even take things like Trusted documents and refer in performance section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great content!
I agree with @saihaj 's sentiment around leaving the caching specific page and linking to it from there (there is a lot of opportunities to expand that page and a lot of users are Googling that terms)
@benjie I can transfer the "Globally unique IDs" content back to a dedicated Caching page and leave the rest of the content on the new Performance page. A deep dive into focused caching topics on the Caching page would need to be scoped to a separate project. Let me know how you would like to proceed. |
This involved a little thought... Really Globally Unique IDs aren't just for performance, they're for identity which can relate to mutations and URLs and all sorts of things. From a quick scan of the existing page, I think what it was really talking about was normalized caching which also relates to rendering consistency, updating data after mutations, and other topics. Definitely we should move it off of the performance page... Saihaj is right that in addition to normalized caching (Global Object IDs or otherwise) there are other approaches to (client-side) caching and they each have their own trade-offs - but they also solve slightly different (but overlapping) problems - and I'm not sure that "caching" is even the right title for that... There's also other forms of caching that relate to GraphQL such as caching within proxies (like Cloudflare), caching of parse/validation results, and even caching in the business logic layer such as within DataLoader - and I'm not sure those would belong on the same page! They definitely seem more "performance" related to me. For now, let's do as you propose and move Node IDs (all the way up to the end of "Alternatives") back to the "caching" page. Any additional content about alternative techniques can be proposed via future PRs. Ideally we'll come up with a better name that essentially means "GraphQL-focussed client-side data storage and synchronisation techniques" but, you know, pithier... |
Addressed in 6f7fa7b |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The vast majority of this is great content as-is; I've noted some recommended changes to a few small areas below.
```graphql | ||
# { "graphiql": true } | ||
query HeroWithFriends { | ||
# 1 request for the hero | ||
hero { | ||
name | ||
# 3 more requests for each friend | ||
friends { | ||
name | ||
} | ||
} | ||
} | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this is a good demonstration of the problem since the hero.friends
field is still only called once; consider these resolvers:
function Query_hero(_, __, { userId }) {
return sql`select * from heroes where id = ${userId}`;
}
function Hero_friends(hero) {
return sql`select * from heroes where id in (
select friend_id from friends where hero_id = ${hero.id}
)`;
}
There's no N+1 problem here.
Instead, we should start with a list field, and then have a singular field within it, e.g. (not specifically this!)
{
# 1 fetch results in 6 records
films {
# This selection set (and everything in it) executes 6 times - this
# field is called 6 times.
mainHero {
name
}
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea here was a more naive implementation GraphQL wraps something like a REST API) and you're making 3 separate requests for each friend character to a /character/{id}
endpoint to get them by their IDs. Perhaps I can add more context to the example rather than building out more types and fields in schema to illustrate this?
Co-authored-by: Benjie <[email protected]>
|
||
## Client-side caching | ||
|
||
There is a range of client-side caching strategies that GraphQL clients may implement to help not only with performance but also to ensure that you render a consistent and responsive interface to your users. [Read more about client-side caching](/learn/caching). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@benjie I accepted the change but I'm not sure what the "please edit" note was in reference to here.
Description
This PR expands on the existing caching content for the Learn docs so that it addresses broader performance topics. Key changes include:
GET
to facilitate caching, the N+1 problem, performance monitoring, and more@benjie @jorydotcom