[3.0] useQuery does not update when an optimistic response is rolled back

[jcane86]

I perform a mutation with an optimisticResponse, and use update function to update related queries. When the server responds with a network error, the optimistic response is rolled back (I can see it doesn't exist in the cache), but my useQuery hook that is showing the list does not rerender.

Related: #5215 (was moved to react-apollo before it was merged)

Intended outcome:
When an optimistic response is rolled back, all related hooks and components are rerendered to show the rolled-back state.

Actual outcome:
The cache seems to be updated, but the list is stuck with stale data until rerender of the component.

How to reproduce the issue:
Made a reduced reproduction of this here: https://codesandbox.io/s/optimistic-rollback-broadcast-ftmbj

The mutation I trigger in the sandbox does not exist on the server so it responds with 400.
The useQuery list displays the optimistic response, but never updates to show the rollback.
The button below reads the same query from cache to verify it has been rolled back.

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"

[jcane86]

To be fair, I just tested the read cache button on my sandbox by setting Chrome to Slow3G to read the cache while the operation was in transit, and I couldn't see the optimistic response there, so maybe I am reading the cache wrong there.

Remounting the component does fix the list, without a network operation, so I think the issue still stands

[georeith]

Related: #5215 was moved to react-apollo (incorrectly in my opinion)

I moved it prior to: 191065c

[jcane86]

Related: #5215 was moved to react-apollo (incorrectly in my opinion)

I moved it prior to: 191065c

fair enough, I wasn't having a go or anything. Sorry if it came across as rude, I'll remove that bit.

[georeith]

fair enough, I wasn't having a go or anything. Sorry if it came across as rude, I'll remove that bit.

Not at all I didn't think you were was just giving information about it. Was confused when I saw the hooks source in this repo today.

[hwillson]

Thanks for the reproduction @jcane86 - this definitely looks like a bug.

[3nvi]

any updates on that or ways we can help?

[benjamn]

@3nvi Check out #6221 when you have a chance!

[3nvi]

@3nvi Check out #6221 when you have a chance!

Will take a look within the next few hours and comment on the PR :)

[levidyrek]

Is there a workaround for this in the meantime?

[EirikFA]

I still experience issues in my own app and in @jcane86's reproduction (https://codesandbox.io/s/optimistic-rollback-broadcast-v59wo) after updating to beta 46. It seems the result is rolled back for the first mutation, but not for any following.

[WIStudent]

I am seeing a similar issue using watchQuery that is probably related. I have a demo here. Basically I am setting up a watchQuery and then triggering an optimistic mutation whose update function changes the watchQuery data using cache.modify. The watchQuery will be updated with the modified cache data caused by the optimistic response, but not with the rolled back cache data if the mutation failed. I tried this using beta 48.

[hwillson]

Just dropping some quick notes here - I've looked into this further and have verified that Apollo Client's optimistic response rollback code is all working properly. The problem here is caused by useQuery's memoization code. Even though the response is rolled back, useQuery doesn't pick the change up as it sees the render that should pick the change up as being the same as the last render, so it avoids running the query again. This isn't the first time we've had issues with useQuery's memoization approach, and there are still several open issues that are caused by it. I'm very tempted to remove our useDeepMemo use completely ...

[darkbasic]

I've tried beta.52 and what happens is really weird: as you said the query is rolled back, but it doesn't get reflected in the UI until you re-access it. If you do so and re-trigger the same mutation, this time it will get reflected in the UI without the need to re-access it.

[hwillson]

@benjamn and I have looked at this further, and have noticed that while removing useDeepMemo does fix the issue, it's masking the real issue that's happening at a lower level. We need to adjust the cache internals a bit (namely how maybeBroadcastWatch leverages caching via makeCacheKey while ensuring optimistic rollbacks are considered). We're working on a solution.

[hwillson]

Fixed - thanks!