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

Evicting the cache for a query with both @include and @nonreactive directives can cause errors if variables have been changed #11987

Open
jbachhardie opened this issue Aug 2, 2024 · 1 comment · May be fixed by #11990

Comments

@jbachhardie
Copy link

Issue Description

This one took quite a while to get a minimal reproduction for since a lot of things have to be true for it to happen so I'll try to explain it all step by step as well as attaching a Codesandbox. If you:

  1. have a query that uses @include with a variable that is set through a query-level default
  2. and which also uses @nonreactive somewhere in it
  3. and you're calling it with useQuery
  4. and you change the variables to something that doesn't include the variable that is used in the @include
  5. and you then evict a field of the query using cache.evict
  6. and you then call cache.gc
  7. then Apollo errors with a "Invalid variable referenced in @include directive" error, even though the variable has a valid default

This bubbles up as an unhandled error that can't be caught by either try/catch around the cache.gc() call or React error boundaries, which made it completely crash out our React Native app.

I know a little bit of what is happening here, observable.reobserve() in useResubscribeIfNecessary sets new variables on the observable but doesn't apply query defaults to them because that logic is in the QueryManager rather than the observable. This normally doesn't cause any problems AFAICT but if the query has a nonreactive directive then the Observable has to process the selection sets with the new variables, which are missing their defaults. I'm not super sure why this only happens if cache.gc is called, though, and I don't immediately know how I'd refactor things to fix this in a PR so I'm just reporting it for now.

Link to Reproduction

https://codesandbox.io/p/devbox/upbeat-bush-mwmqx6?workspaceId=03104bf9-16b8-430d-a030-08b693449298

Reproduction Steps

Click "Change variables" and then "Evict cache" in the CodeSandbox, see the uncaught error in the console

@apollo/client version

3.10.5

@phryneas
Copy link
Member

phryneas commented Aug 5, 2024

Hi Jae,
thank you for that thorough analysis and reproduction, this is great!

I believe there is no beautiful fix for this - we'll just have to add variable defaults everywhere they're missing.
I've started a PR over in #11990 - if you have the time, please try out the PR build and report back if it fixes the issue for you :)

@phryneas phryneas linked a pull request Aug 6, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants