-
Notifications
You must be signed in to change notification settings - Fork 28
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
Improve API docs, including instructions on graphql_ppx_re. #125
Conversation
} | ||
``` | ||
|
||
Great – we've successfully executed a mutation to like a dog! Existing queries that reference the mutated data will be notified that data has changed, meaning you don't need to think at all about refetching your data – it's all updated for you by `reason-urql`. |
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.
Oh, this is quite nice. I didn't know that it worked like this 🤔
How do I make sure I'm using this feature?
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.
It has to do with urql
's observable system, which is powered by wonka
. Essentially, every operation that passes through the Client can have subscribers, and this is particularly important in the case of queries that are dependent on mutation results. When a mutation is executed, urql
will aggressively invalidate the cache for any entity that has a shared __typename
with the mutation. This then notifies subscribers to those cache entries (really any query that requests data with the same __typename
) that they should update: https://github.com/FormidableLabs/urql#document-caching. The first example project in this repo shows this pretty well – if you hit the button to fire executeMutation
, the query will be notified that the cache was invalidated and pull anew from it, causing a re-render. Hope that makes sense! You don't need to do anything to opt into this, it should just happen for you.
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.
Looks good to me! Only thing is I saw that you removed mention of graphql_ppx
but introduced again in the new mutation examples, maybe you missed it
docs/getting_started.md
Outdated
```reason | ||
[@react.component] | ||
let make = (~key: string) => { | ||
/* Build your request by calling .make on the graphql_ppx module, passing variables as labeled arguments. */ |
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.
Here you mention graphql_ppx
but in other places you removed the mention
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.
Good catch, fixed here: f67eb9f
Fix #121.
This PR begins the work to improve our API documentation and clarify language around using
graphql_ppx_re
. The biggest hurdle with the new lib is the difference in how a user derives agraphql_schema.json
file (viagraphql-cli
rather than a local Node script that ships with the lib).We'll probably want to begin converting our examples over gradually to use the new PPX – perhaps a good issue for a first time contributor.
I also added a bit more to our
getting_started.md
to discussuseMutation
. Recent changes to theurql
README added a lot of good information that it may be worthwhile to piggyback off of here as well, so I'll keep working on making the docs more usable. Open to any suggestions on how to improve things.The last change in this PR is just ensuring we're using
CombinedError.t
instead ofCombinedError.combinedError
to type that record across the lib.