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 increment lastRequestId when using links to get results #7956

Merged
merged 4 commits into from
Apr 7, 2021

Conversation

dannycochran
Copy link
Contributor

fetchQueryByPolicy currently always increments queryInfo.lastRequestId, even when a new request is not necessary and results are instead read directly from cache.

An example way to trigger this bug:

git clone https://github.com/dannycochran/usequery/tree/refetchPolicy
cd usequery
npm install
npm run start

Then, try deleting an item. You’ll notice in the network tab the mutation fires successfully and so does the refetch query with the correct results of one less bid item. However, the Apollo cache doesn’t update so 4 items will remain. Subsequent deletes / adds work just fine.

The cache is not updating because we are updating fetchPolicy manually from our useQuery invocation, which triggers fetchQueryByPolicy, but the result ends up just reading from cache. However, since it also updates the lastRequestid, the conditions to write the result to cache are no longer satisfied: lastRequestId is not greater than queryInfo.lastRequestId, so the cache does not get updated after refetch: https://github.com/apollographql/apollo-client/blob/main/src/core/QueryManager.ts#L869

@benjamn benjamn self-requested a review April 7, 2021 14:18
@benjamn benjamn changed the base branch from main to release-3.4 April 7, 2021 16:36
Copy link
Member

@benjamn benjamn left a comment

Choose a reason for hiding this comment

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

@dannycochran This looks great to me, especially with the test (which I verified does fail if I revert your first commit). I'm going to merge it into release-3.4 and release another beta version for testing, but I think we can back-port this to main (v3.3.x) once we know it solves the problem.

@benjamn benjamn merged commit 35158e3 into apollographql:release-3.4 Apr 7, 2021
benjamn pushed a commit that referenced this pull request Apr 12, 2021
PR #7956 originally landed on the release-3.4 branch (so we could test it
in a beta release), but the change itself should be backwards compatible
for the @apollo/client@3.3.x release line, which is currently published
from the main branch.
benjamn added a commit that referenced this pull request Apr 13, 2021
PR #7956 originally landed on the release-3.4 branch (so we could test it
in a beta release), but the change itself should be backwards compatible
for the @apollo/client@3.3.x release line, which is currently published
from the main branch.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants