-
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
Cache redirects #921
Cache redirects #921
Conversation
@stubailo please review this when you have a minute. I think it should work, but a second pair of eyeballs on this would be 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.
I think this can be simplified since the stuff around the ID is mostly boilerplate.
customResolvers: { | ||
Query: { | ||
person: (_, args) => { | ||
return { |
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, this is probably not the optimal API right? Perhaps we should change this so that people can just return the ID?
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.
Haha, yes, good catch! I patched the tests to get things working and then forgot to build that in.
generated: false, | ||
}; | ||
}, | ||
person: (_, args) => toIdValue(args['id']), |
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, let's talk about this.
@stubailo I added a |
Why? |
How would you tell apart an id from a string otherwise? Personally I'm in favor of having as little fancy magic under the hood as possible, and making people use |
I don't know why I already merged this. I still need to update the changelog and write docs for it... |
Does this fix pagination? I have a component which utilizes pagination, but it fetches data every time the component mounts, even if the data from the exact same query is already cached in redux. Is this a known issue? Edit: I opened issue #933 |
These are the cherry-picked commits from #809 to implement only cache redirects (and not yet fully support client-side computed fields).
TODO: