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

fetchMore's previous result may be stale #6471

Closed
mindnektar opened this issue Jun 21, 2020 · 3 comments
Closed

fetchMore's previous result may be stale #6471

mindnektar opened this issue Jun 21, 2020 · 3 comments
Assignees
Milestone

Comments

@mindnektar
Copy link

This issue has kept my mind occupied for weeks, as the number of users on our application grew and more and more reports kept coming in that every now and then the result of an action would not be reflected in the UI, causing the user to try the action again and the backend throwing an error telling them the action had already been performed. The frequency of this issue was increased if the user had a bad internet connection. I was never able to replicate the issue myself, until two days ago.

props.data.fetchMore({
    query: [...],
    variables: [...],
    updateQuery: (previousResult, { fetchMoreResult }) => (
        doSomethingToReturnNewCacheData(previousResult, fetchMoreResult)
    ),
});

This is a pretty standard call to fetchMore, and it works 99.9% of the time. It does not work when a mutation happens that alters the cache data of the query used by fetchMore, while the fetchMore request is happening. I was able to reproduce this without fail by following these steps:

  1. Start a fetchMore request.
  2. Make a mutation after the fetchMore request has started, and make sure that it completes before the fetchMore request has responded.
  3. Have that mutation's result alter the cache data of the fetchMore query.

So apparently the previousResult returned by fetchMore is the cache data at the time of the start of the request, and not the cache data at the time of its end. I could fix this easily by doing the following:

props.data.fetchMore({
    query: [...],
    variables: [...],
    updateQuery: (previousResult, { fetchMoreResult }) => {
        const cacheData = apolloClient.readQuery({
            query: [...],
            variables: [...],
        });
        return doSomethingToReturnNewCacheData(cacheData, fetchMoreResult);
    },
});

With this change, I can no longer reproduce the issue even once. And of course it was worse for users with a bad connection, because the fetchMore request would take longer. Now I'm wondering if this behaviour is by choice (if so: why?) or if fetchMore should rather return the cache data at the time of the request's end. Either way, I thought maybe someone might appreciate being aware of this pitfall - unfortunately I followed a lot of false leads before finally stumbling upon the solution.

@benjamn
Copy link
Member

benjamn commented Jun 22, 2020

What version of Apollo Client are you using?

If you update to @apollo/[email protected], you'll see a warning that updateQuery is now deprecated, which is explained in #6464. If you follow the advice of the deprecation warning, and write a field merge function for the query field, I am confident you will never find yourself in this situation again, since previousResult is no longer involved.

Here's the general structure of an InMemoryCache field policy:

new InMemoryCache({
  typePolicies: {
    TheParentType: {
      fields: {
        thePaginatedField: {
          merge(existing, incoming, { args }) {
            return doSomethingToReturnNewCacheData(existing, incoming, args);
          },
        },
      },
    },
  },
})

If you can put together a runnable reproduction, I can provide more specific advice.

@benjamn benjamn self-assigned this Jun 22, 2020
@mindnektar
Copy link
Author

Thanks for the feedback! I'm still on the latest version 2 release, but unfortunately due to certain circumstances there is currently no budget for upgrading a central system component and making sure everything still runs smoothly, so I'll have to be staying on 2 for the time being (though I do hope to see that change at some point). I'm sorry to say I'm also unable to provide a runnable reproduction at this time, but I understood your proposed solution and will migrate the code base accordingly once I'm able. Until then, my workaround will have to do (luckily, there have been no further complaints since it's in use). Thanks again!

@benjamn benjamn added this to the Release 2.x milestone Jun 25, 2020
@hwillson
Copy link
Member

hwillson commented May 3, 2021

This should no longer be an issue in @apollo/client@latest - let us know otherwise, thanks!

@hwillson hwillson closed this as completed May 3, 2021
@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.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants