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

[3.0] useQuery does not hit the cache when variables change #5782

Closed
jcane86 opened this issue Jan 13, 2020 · 6 comments
Closed

[3.0] useQuery does not hit the cache when variables change #5782

jcane86 opened this issue Jan 13, 2020 · 6 comments

Comments

@jcane86
Copy link

jcane86 commented Jan 13, 2020

I know there are similar issues open already, but couldn't really come to any conclusions after reading them, also I thought the reproduction below could help.

related: #5659

Intended outcome:
When variables change in useQuery, Apollo Client should repeat the full request lifecycle exactly as for the initial set of options, including checking the cache for possible hits.

I am using a TypePolicy to get the results from the cache, not sure if that is important to this issue.

Actual outcome:
on component mount, useQuery gets the data from the cache/network. if query variables are changed, useQuery refetches the query (if permitted by fetchPolicy), but never checks the cache with the new variables:

  • fetchPolicy: 'cache-and-network'
    shows the old value while waiting for new network response, even if the new values are already in cache.
  • fetchPolicy: 'cache-only' or 'cache-first'
    shows the value corresponding to the first set of variables forever, never goes back to the cache to update the result.

How to reproduce the issue:
reproduction here: https://codesandbox.io/s/dreamy-ride-gns90

The sandbox includes one query that fills the cache, and then another one that can be executed with changing variables. This second query is configured in typePolicies to grab results from the cache. The query results do not get updated from the cache after the first request.

Versions
"@apollo/client": "3.0.0-beta.21",
"graphql": "14.5.8",
"react": "16.12.0",
"react-dom": "16.12.0",
"react-scripts": "3.0.1"

@benjamn benjamn self-assigned this Jan 13, 2020
@benjamn benjamn added this to the Release 3.0 milestone Jan 13, 2020
@benjamn
Copy link
Member

benjamn commented Jan 13, 2020

@jcane86 Just wanted to say thanks for the excellent reproduction! Not fixed just yet, but I think I know what's (not) happening.

@jcane86
Copy link
Author

jcane86 commented Jan 14, 2020

@benjamn no worries mate, thank you for your work on the library. v3.0 is looking great BTW

benjamn added a commit that referenced this issue Jan 14, 2020
This partially reverts two commits from PR #5565, preserving related
improvements that have happened in the meantime:

* Revert "Remove partial field from ApolloCurrentQueryResult type."
  This reverts commit ca2cef6.

* Revert "Stop reading from cache in ObservableQuery#getCurrentResult."
  This reverts commit 01376a3.

One of the motivations behind the original changes was to lay the
foundations for eventually supporting asynchronous (Promise-based) cache
reads, but that form of asynchrony is no longer a goal of Apollo Client
3.0 or any planned future version, so there's not as much benefit to
weakening getCurrentResult in this way.

On the other hand, allowing getCurrentResult to read synchronously from
the cache ensures that custom field read functions can return new results
immediately, which helps with issues like #5782.

Immediate cache reads have also been the behavior of Apollo Client for
much longer than #5565, which makes this change appealing from an
ease-of-upgrading perspective.
benjamn added a commit that referenced this issue Jan 14, 2020
This partially reverts two commits from PR #5565, preserving related
improvements that have happened in the meantime:

* Revert "Remove partial field from ApolloCurrentQueryResult type."
  This reverts commit ca2cef6.

* Revert "Stop reading from cache in ObservableQuery#getCurrentResult."
  This reverts commit 01376a3.

One of the motivations behind the original changes was to lay the
foundations for eventually supporting asynchronous (Promise-based) cache
reads, but that form of asynchrony is no longer a goal of Apollo Client
3.0 or any planned future version, so there's not as much benefit to
weakening getCurrentResult in this way.

On the other hand, allowing getCurrentResult to read synchronously from
the cache ensures that custom field read functions can return new results
immediately, which helps with issues like #5782.

Immediate cache reads have also been the behavior of Apollo Client for
much longer than #5565, which makes this change appealing from an
ease-of-upgrading perspective.
benjamn added a commit that referenced this issue Jan 14, 2020
This partially reverts two commits from PR #5565, preserving related
improvements that have happened in the meantime:

* Revert "Remove partial field from ApolloCurrentQueryResult type."
  This reverts commit ca2cef6.

* Revert "Stop reading from cache in ObservableQuery#getCurrentResult."
  This reverts commit 01376a3.

One of the motivations behind the original changes was to lay the
foundations for eventually supporting asynchronous (Promise-based) cache
reads, but that form of asynchrony is no longer a goal of Apollo Client
3.0 or any planned future version, so there's not as much benefit to
weakening getCurrentResult in this way.

On the other hand, allowing getCurrentResult to read synchronously from
the cache ensures that custom field read functions can return new results
immediately, which helps with issues like #5782.

Immediate cache reads have also been the behavior of Apollo Client for
much longer than #5565, which makes this change appealing from an
ease-of-upgrading perspective.
benjamn added a commit that referenced this issue Jan 14, 2020
This partially reverts two commits from PR #5565, preserving related
improvements that have happened in the meantime:

* Revert "Remove partial field from ApolloCurrentQueryResult type."
  This reverts commit ca2cef6.

* Revert "Stop reading from cache in ObservableQuery#getCurrentResult."
  This reverts commit 01376a3.

One of the motivations behind the original changes was to lay the
foundations for eventually supporting asynchronous (Promise-based) cache
reads, but that form of asynchrony is no longer a goal of Apollo Client
3.0 or any planned future version, so there's not as much benefit to
weakening getCurrentResult in this way.

On the other hand, allowing getCurrentResult to read synchronously from
the cache ensures that custom field read functions can return new results
immediately, which helps with issues like #5782.

Immediate cache reads have also been the behavior of Apollo Client for
much longer than #5565, which makes this change appealing from an
ease-of-upgrading perspective.
@benjamn
Copy link
Member

benjamn commented Jan 15, 2020

@jcane86 This should now be fixed if you update to @apollo/[email protected] (thanks to #5791), judging from your reproduction.

@jcane86
Copy link
Author

jcane86 commented Jan 15, 2020

@benjamn brilliant, thanks for the quick fix! I'll give it a go in my project later and come back if anything goes wrong, but yeah, looks solved to me!

@hwillson
Copy link
Member

Resolved by @apollo/[email protected] - closing, thanks!

@cristiandley
Copy link

#6017

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants