search

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.37k stars 2.66k forks source link

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

Open jbachhardie opened 2 months ago

jbachhardie commented 2 months ago

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 commented 2 months ago

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 :)