Skip to content
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

Only return newData when querying current results for no-cache or network-only queries #4352

Merged
merged 4 commits into from
Feb 16, 2019

Conversation

danilobuerger
Copy link
Contributor

@danilobuerger danilobuerger commented Jan 21, 2019

Fixes

QueryManager.getCurrentQueryResult (and thus Observable.currentResult) should only return newData for the queries current result, as the query is not supposed to return cached data. This will then fix a long standing bug in react-apollo where cached data shows up on network-only queries.

@hwillson
Copy link
Member

This approach looks good @danilobuerger - thanks!

@danilobuerger danilobuerger added this to the Release 2.5.0 milestone Jan 23, 2019
@danilobuerger danilobuerger self-assigned this Jan 24, 2019
@danilobuerger danilobuerger changed the title [WIP] Only return newData when querying current results for no-cache or network-only queries Only return newData when querying current results for no-cache or network-only queries Jan 26, 2019
@danilobuerger danilobuerger removed their assignment Jan 26, 2019
@danilobuerger
Copy link
Contributor Author

@hwillson amended and rebased

Copy link
Member

@hwillson hwillson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this @danilobuerger - LGTM!

@hwillson hwillson merged commit 50a2616 into apollographql:master Feb 16, 2019
@danilobuerger danilobuerger deleted the cache branch February 17, 2019 10:36
@EugeneHerasymchuk
Copy link

@hwillson this functionality will be only in v2.5.0 ? Any info about date of release.

And huge thank you to @danilobuerger!!!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants