-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Revised local state docs #4378
Revised local state docs #4378
Conversation
This commit covers Apollo Client local state management docs changes, for the new integrated local state functionality. It’s essentially a remix of the existing local state docs and `apollo-link-state` docs merged with new feature content.
I'm not sure why, but for some reason Hexo corrupts the `client-schema.png` when it's in the `source/assets` directory. Moving it to another location fixes the issue, and since Hexo is going to be replaced, I'm not going to dig into this further.
People already using Boost won't have to change anything to use AC's integrated local state handling.
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.
Looking good!
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.
Really amazing work on these docs @hwillson! I love how thorough they are, and I'm super happy to see some of the new things you've built with the project. Before we merge, I have some feedback:
-
Client-side schemas should have way more visibility, both in the docs and in the release post. All of the amazing tooling benefits that we promised a year ago are finally here, so we should show them off. No other state management solution gives you autocomplete + intellisense in VS Code and automatic generation of TypeScript definitions! Another cool use case that @debergalis came up with is mocking on the client -- before a product team builds a subsection of the schema into their API, they can easily mock it by creating a client schema and resolvers to contextually mock out their data. When they're ready to integrate it into the remote API, they can drop the client directives. Several customers have been really impressed by that suggestion, so we should include them in the docs.
-
What about code splitting? We mention the methods for dynamically adding resolvers in the API section but don't actually provide a use case. Now that initializers are gone, I'd also like to see an example of how to write values to the cache from a dynamically loaded component.
-
Cache persistence is another initializer use case that just came to mind.
-
The docs are thorough, but they feel a bit long for an essentials guide. I think moving the cache methods out of this guide should help a bit -- do you think there's anything else that could be moved or cut?
-
Are subscriptions a thing now? I don't know how useful they will be, but we should include an example if they work!
-
Versioning the docs: Do we want to version them for this release? We have versioning built into the Apollo Server docs and the new Gatsby infra.
Happy to jump on a call if you want to talk any of this over! 😀
<dt>`resolvers?`: Resolvers | Resolvers[]</dt> | ||
<dd>A map of resolver functions that your GraphQL queries and mutations call in order to read and write to the cache.</dd> | ||
<dt>`typeDefs?`: string | string[] | DocumentNode | DocumentNode[];<string></dt> | ||
<dd>A string representing your client-side schema written in the [Schema Definition Language](/docs/graphql-tools/generate-schema.html#schema-language). This schema is not used for validation, but is used for introspection by the [Apollo Client Devtools](https://github.com/apollographql/apollo-client-devtools).</dd> |
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 main benefit of the client-side schema is for autocomplete and intellisense within Apollo VS Code, as well as automatic type definitions with the Apollo CLI. Since the tooling benefits are a huge part of the launch of AC 2.5, we need to find a way to make these features more prominent throughout the doc.
|
||
3. If you define multiple `@export` variables that use the same name, in a single operation, the value of the last `@export` variable will be used as the variable value moving forward. When this happens Apollo Client will log a warning message (dev only). | ||
|
||
<h2 id="managing-the-cache">Managing the cache</h2> |
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 think we should remove this entire section and direct them to the advanced caching guide. Otherwise, this guide becomes way too long. The examples in the advanced caching guide need a refresh anyway :)
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 on the fence on this one. I definitely hear what you’re saying, but working with the cache is such an integral part of the AC local state experience, that I wanted to make sure people could find out how to use it, without jumping around the docs. If we remove the Managing the cache
section, and just point people to https://www.apollographql.com/docs/react/advanced/caching.html#direct, are we having them jump around a bit too much? Maybe not … one things for sure though, we’ll need to add cache.writeData
to the Direct Cache Access
section, since it isn’t mentioned anywhere.
So just to confirm @peggyrayzis, when you say point people to the advanced caching guide, you mean point them to Direct Cache Access
, correct? If so I’ll add cache.writeData
to that section, remove Managing the cache
, and update all links. Thanks!
|
||
To write the data to the cache, you can use either `cache.writeFragment` or `cache.writeData`. The only difference between the two is that `cache.writeFragment` requires that you pass in a fragment to validate that the shape of the data you're writing to the cache node is the same as the shape of the data required by the fragment. Under the hood, `cache.writeData` automatically constructs a fragment from the `data` object and `id` you pass in and calls `cache.writeFragment`. | ||
|
||
<h2 id="client-schema">Client-side schema</h2> |
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.
We should move this section up to the front. Maybe even before we show people how to write resolvers, since it's so important now that the tooling benefits with VS Code and the CLI are here. Do you think we should recommend that people practice schema first development when modeling their local state now that they're actually useful?
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.
Hmm - good question. I'll think about this one a bit longer ...
Hi Apollo team, Tonight, I had decided on diving deep into the issue that I wasn't able to get my head around since a while: Handling local (application) state within a React app that use Apollo Client. Previously, I remember that I had read that Apollo Client was internally using Redux in the official docs. Then, I learnt that that was the case at the first version but you no longer use Redux internally. After that I dived deep into Apollo Link State docs to learn more about current implementation and how to use it. I read the announcement post, watched the whole Video tutorial by Sara Vieira. Finally, I've seen that link-state is being moved into core at the version 2.5 and the docs that I was supposed to be reading was this. It was a little frustrating for me to waste a lot of time with out-dated docs (Especially the YouTube video. Using higher order components for event based mutations?). I think you'd better update all related docs/blog posts/links that I mentioned above accordingly or others that I didn't encounter. The first thing I'd do would be removing the first line from here and add a link navigating to here. Thanks for all the effort and the good job. |
Thanks @scaryguy - as soon as these docs are ready (early this week) and we launch Apollo Client 2.5.0 (later this week), we're going to update/deprecate all of the out of date local state docs, across the board. Sorry for the current headache, and thanks for the thoughtful feedback! |
@hwillson who knows how many people in a week may have the bad experience that I've just had. I'd like to contribute by removing the first line from here and updating the issue # at the beginning of this README if you think that they'd help? |
Thanks for the extremely thorough feedback @peggyrayzis! I've addressed most of your comments, but have a few questions/comments on the items in your main comment:
It would be great to highlight this more thoroughly, but does this belong in the local state docs, or would it be more at home in our tooling docs? I'm guessing this should probably live in our tooling docs, and I would then add something to the client schema section of these docs, mentioning that client side schemas can feed into our tooling, and point people to the tooling docs.
I believe this is now going to be addressed separately (and more thoroughly). Should we leave this one for now, and update the docs after launch, when the other local mocking initiaive is in place?
Sounds good - I'll add this in.
AC 2.5 should still be able to work with cache persistence without initializers. People can just call
I agree, but I'm at a loss for what else should be cut out 🙁. Definitely open to more suggestions!
They are! I'll get an example in.
Good point - we'll want versioning. I'll look into getting this wired up. |
The 2.4 version won't work until these changes are pushed into production, at which point the `version-2.4` branch will be referenced automatically (thanks to https://github.com/apollographql/hexo-versioned-netlify-redirects), and https://version-2-4--apollo-client-docs.netlify.com/docs/react/.
setTypeDefs is mentioned at https://www.apollographql.com/docs/react/essentials/local-state/, although this method doesn't exist anymore |
Preview link.
This PR covers the Apollo Client local state management docs changes. It’s essentially a remix of the existing Apollo Client local state docs and
apollo-link-state
docs, merged with new feature content.A few notes:
These docs aren’t intended to be a step by step tutorial, or line up with a reference or tutorial app. My thinking here was that since we’re already doing this with the Apollo tutorial (which includes AC local state), we shouldn’t try to replicate things with another tutorial based approach (that we then have to keep up to date). These docs use code samples to explain things, but those code samples aren’t necessarily taken from a reference application (in some cases they are borrowed from the
fullstack-tutorial
app). If anyone disagrees with this approach, definitely let me know.I had a hard time deciding on section ordering. For example, should we explain how to manage the cache first (using
cache.writeData
for example), then how to use local resolvers? Or should resolvers be explained first, so people know why they might need to use something likecache.writeData
? Should we show people how to use resolvers for mutations first so they have data in place before trying to query with a local resolver? Or should we explain querying with resolvers first since it’s easier to grasp, before diving into mutations? I’ve gone with the approach implemented, but let me know if the ordering seems off - happy to re-work any of it!I think the section titles need work - feedback appreciated!
Since knowing how to use certain
ApolloCache
methods is important when working with local state, I’m thinking it might make sense to add theApolloCache
API to the left hand docs menu, down at the bottom in the API section (probably underApollo Client
/ beforeReact Apollo
). Let me know if others agree.I think we should probably add an
Advanced
section, and in it give examples of doing things like loading resolvers on specific routes to better leverage code-splitting. Then again, maybe that’s overkill for the initial release (we can always add it in afterwards).Thanks!
Note: I've left the base branch for this PR as
master
to use our Netlify docs preview. It won't work when the base branch is set to anything else, but when we're happy with the changes here, the base branch should be changed torelease-2.5.0
.