apollographql / apollo-client

:rocket:  A fully-featured, production ready caching GraphQL client for every UI framework and GraphQL server.
https://apollographql.com/client
MIT License
19.28k stars 2.64k forks source link

Unsubscribing from request with optimistic response pins cache to last successful response #10271

Open koenpunt opened 1 year ago

koenpunt commented 1 year ago

I have a custom link that aborts an in-flight request, if a request with the same key is being executed.

Click here for the link implementation ```ts export class AbortLink extends ApolloLink { private subscriptions: { [key: string]: ZenObservable.Subscription } = {}; constructor(private contextKey: string) { super(); } request(operation: Operation, forward: (operation: Operation) => ObservableQuery) { const context = operation.getContext(); if (!context[this.contextKey]) { return forward(operation); } return new Observable((observer) => { this.subscriptions[context[this.contextKey]]?.unsubscribe(); const subscription = forward(operation).subscribe(observer); this.subscriptions[context[this.contextKey]] = subscription; return subscription; }); } } ```

And this correctly cancels the previous request:

image

Unfortunately, from that point on, cache updates are no longer processed, and the cache constantly reverts to the optimistic response of the cancelled request.

Before I implemented it using a custom AbortController, assigning fetchOptions.signal, which resulted in the behavior you'll see in the reproduction. And I also found some other issues discouraging the use of a custom AbortController. So I changed the implementation to actually unsubscribe, and let the request be aborted by the library implementation. But the same weird cache issue exist while doing so.

Intended outcome:

I would expect the cache updates to work correctly, and that when I unsubscribe from the request subscription, Apollo continues to process caching of response data as before I unsubscribed.

Actual outcome:

In this video you'll see that the amount in the stepper jumps back to a previous value after the request completes. And this previous value is from I think the optimistic response when the last request was cancelled.

https://user-images.githubusercontent.com/351038/140555524-62884221-dfc3-48a7-8702-6b497bf84202.mov

How to reproduce the issue:

I've created an application that reproduces the issue, which can be found here:

https://github.com/koenpunt/bugreport-react-apollo-unsubscribe

It has 2 buttons to increment the counter; one sets the abortKey property in the context, making it being processed by the abort link, the other just increments it.

When you click the "with abort" button multiple times quickly, you will see the counter jumping back to a previous value. Once this behavior is present, the normal increment doesn't work properly anymore either.

Versions

  System:
    OS: macOS 13
  Binaries:
    Node: 18.8.0 - ~/.nodenv/versions/18.8.0/bin/node
    Yarn: 1.22.17 - /opt/homebrew/bin/yarn
    npm: 8.18.0 - ~/.nodenv/versions/8.18.0/bin/npm
  Browsers:
    Chrome: 106.0.5249.119
    Safari: 16.1
  npmPackages:
    @apollo/client: 3.7.13 => 3.7.13
bignimbus commented 1 year ago

Hi @koenpunt 👋🏻 thanks for filing this issue and for the detailed description and reproduction, it's much appreciated! The team will take a look at this as soon as we can, thanks in advance for your patience 🙏🏻

koenpunt commented 1 year ago

@bignimbus thank you! I've actually opened the same issue about a year ago, but didn't know how to get it under the attention, but I'm happy to see that recreating worked :)

bignimbus commented 1 year ago

Thanks for that context @koenpunt - apologies for the delay on this! Our team of maintainers is growing so we'll be more on top of new Issues (and closing the loop on old ones) going forward. We appreciate your patience ❤️

siddhant928 commented 1 year ago

@bignimbus, any updates on this?

@koenpunt, were you able to find any workaround for this issue?

koenpunt commented 1 year ago

were you able to find any workaround for this issue?

No, and also couldn't find the source of the issue in the library, so haven't been able to propose a fix myself either..

siddhant928 commented 1 year ago

My problem got solved. I was cancelling the optimistic mutation even before apollo client got a chance to remove the optimistic layer.

tim-rellify commented 1 year ago

Any updates on this issue? I'm facing the same issue.

I also implemented a custom Link, that sets an AbortController and would abort it, if another mutation sets the same properties for the same mutation. That ran fine for about a year. But now with a combination of an optimistic response, it leads into the issue, described in this ticket.

An example:

  1. Execute mutation with variables { text: 'abc' } and an optimistic response for that. The custom Link waits 1 second for the same mutation to set the "text" property again
  2. Wait 500ms
  3. Execute the same mutation and an optimistic response, but with the variables { text: 'abc123' }
  4. After step 3 is done, I get the correct response from step 3 in the data-part of the cache. But the optimistic-data-part contains the optimistic response from step 1, which is then the overall data for my watch queries. The issue is, that the optimistic response from step 1 is not removed from the cached layers.

Update: Seems that my issue is related to https://github.com/apollographql/apollo-client/issues/10640 and I could solve it by returning an Observable to abort the mutation instead of using an AbortController:

return new Observable((sub) => {
   sub.error(new Error('Mutation aborted'));
});
koenpunt commented 1 year ago

I saw this commit and for a minute I had hope that it would fix this issue as well, but unfortunately it did not. But, maybe the authors of that commit, @phryneas and @benjamn, have a solution for this issue?

koenpunt commented 7 months ago

Any follow-up on the investigation?