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

v3.0.0-beta.16 mutating local state is not reflected in all components with useQuery #5733

Closed
dusty opened this issue Dec 27, 2019 · 6 comments

Comments

@dusty
Copy link

dusty commented Dec 27, 2019

Intended outcome:
Store local state in apollo cache and when updating the local state then all components that have a useQuery against that data will be updated.

Actual outcome:
It appears only the first subscriber to the useQuery receives updates.

How to reproduce the issue:
At the following code sandbox you can see some local state stored in apollo. There are three components that are rendering the result of the same useQuery query. When the Child component calls a mutation on that data, only the Parent component updates. Both the Child component and the OtherChild component do not update.

If the query is removed from the Parent component, then only the Child component updates.
If the query is removed from both the Parent and Child, then the OtherChild component updates.

https://codesandbox.io/s/apollo-client-prob-6ywxk

Versions

  • @apollo/client 3.0.0-beta.16
  • graphql 14.5.8
  • react 16.12.0
  • react-dom 16.12.0
  • react-scripts 3.0.1
@OlegLustenko
Copy link

OlegLustenko commented Dec 30, 2019

It seems related to #5644
I believe @benjamn could help

@co-l
Copy link

co-l commented Jan 2, 2020

This bug is currently eating my soul bit by bit.

To escape this nightmare, I'm currently duplicating my queries so they are unique to the component. Is there a better way?

@co-l
Copy link

co-l commented Jan 2, 2020

This bug is currently eating my soul bit by bit.

To escape this nightmare, I'm currently duplicating my queries so they are unique to the component. Is there a better way?

I had a "no-duh" moment and centralized my query from my components to the parent. Sanity restored.

@OlegLustenko
Copy link

@JohnBerlin, you can revert back that conditional - #5644 (comment)
But I believe there should be more clear way to support it.
You can just use version: @apollo/client 3.0.0-beta.14

@co-l
Copy link

co-l commented Jan 2, 2020

@JohnBerlin, you can revert back that conditional - #5644 (comment)
But I believe there should be more clear way to support it.
You can just use version: @apollo/client 3.0.0-beta.14

Thank you!

@benjamn benjamn self-assigned this Jan 3, 2020
@benjamn benjamn added this to the Release 3.0 milestone Jan 3, 2020
benjamn added a commit that referenced this issue Jan 3, 2020
Fixes #5733, which was caused by multiple watches having the same cache
key, so none except the first were ever re-broadcast, since the first
broadcast had the side effect of marking the later watches as clean,
before this.watches.forEach had a chance to visit them.

Background: PR #5644 reenabled an important optimization for the
InMemoryCache#broadcastWatches method, allowing it to skip watches whose
results have not changed. Unfortunately, while this optimization correctly
determined whether the result had changed, it did not account for the
possibility of multiple distinct consumers of the same result, which can
happen (for example) when multiple components use the same query and
variables via different useQuery calls.

Fortunately, the fix is straightforward (if not exactly obvious): in order
to assign distinct consumers different cache keys, it suffices to include
the provided watch.callback function in the cache key.

A more drastic way to fix #5733 would be to remove the caching of
maybeBroadcastWatch entirely, which would not be a huge loss because the
underlying cache.diff method is also cached. For now, though, with that
backup option in mind, I'd like to preserve this optimization unless it
causes any further problems.
@benjamn
Copy link
Member

benjamn commented Jan 3, 2020

Thanks very much to @dusty for opening this issue and providing a simple reproduction, and to @OlegLustenko for tracking down the probable cause!

Please try updating to @apollo/[email protected], which contains PR #5747, when you have a chance.

This bug is currently eating my soul bit by bit.

@JohnBerlin Is that because you were trying to diagnose the bug and fix it, and you found the internals of the cache hard to understand? If so, I can definitely empathize. Not everyone understands this system as well as I do, and it's not your responsibility to find solutions like #5747.

It is your responsibility, however, to remember that you're using beta software, and that we all share the same basic goals. Once you get into that mindset, I think you'll find words like "eating my soul" and "nightmare" no longer feel appropriate.

Take that advice, or leave it. I'll fix the bugs either way.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 1, 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