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

Apollo v3.6.8 and above throw `Uncaught TypeError: Cannot read properties of undefined (reading 'shift')` when running multiple writeFragment calls in a loop #10262

Closed mogzol closed 8 months ago

mogzol commented 1 year ago

Intended outcome:

This issue occurs when my app makes multiple calls to client.writeFragment in a loop, like this:

for (const asset of result.model) {
  apolloClient.writeFragment({
    id: asset.id,
    fragment: UPDATE_ASSET_FRAGMENT,
    fragmentName: "UpdateAssetFields",
    data: asset,
  });
}

I would expect this to work fine, and it does work fine on all versions of Apollo prior to v3.6.8

Actual outcome:

On v3.6.8 of Apollo and later, on the second iteration of that loop (so the second writeFragment call), the following error is thrown:

Uncaught TypeError: Cannot read properties of undefined (reading 'shift')
    at Object.complete (Concast.js:42:1)
    at Concast.removeObserver (Concast.js:113:1)
    at ObservableQuery.tearDownQuery (ObservableQuery.js:458:1)
    at ObservableQuery.js:33:1
    at cleanupSubscription (module.js:88:1)
    at Subscription.unsubscribe (module.js:203:1)
    at useQuery.js:115:34
    at HTMLUnknownElement.callCallback (react-dom.development.js:3945:1)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:3994:1)
    at invokeGuardedCallback (react-dom.development.js:4056:1)

How to reproduce the issue:

Unfortunately I'm not entirely sure how to reproduce this (EDIT: see first comment below for reproduction). I tried creating a new basic Apollo project with a single component and query and then running multiple writeFragments, but couldn't get the error to happen. I can tell you that it does happen reliably in my own app, and the change that caused it was this MR: https://github.com/apollographql/apollo-client/pull/9791

Specifically the change to ObservableQuery.ts on line 839:

- this.concast.removeObserver(this.observer, true);
+ this.concast.removeObserver(this.observer); 

If I add the second true argument back in, then the error is no longer thrown and everything works properly.

Versions

  System:
    OS: macOS 13.0
  Binaries:
    Node: 16.18.0 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 8.19.2 - /usr/local/bin/npm
  Browsers:
    Chrome: 107.0.5304.87
    Firefox: 106.0.2
  npmPackages:
    @apollo/client: 3.6.8 => 3.6.8 
mogzol commented 1 year ago

I've been investigating this more and have managed to reproduce the issue, see here:

https://codesandbox.io/s/apollo-typeerror-demo-qygfem?file=/src/App.jsx

The issue seems to occur when there are any observed queries that use the @export directive, which my app relies heavily on. This issue means that this whole use case described in the Apollo documentation does not work on recent Apollo versions.

I've had to downgrade back to Apollo 3.6.7 for the time being, hopefully this issue can be resolved soon so I can upgrade back to the latest version.

It's also worth noting that on React 18 this issue seems to be even more pronounced, it happens immediately, without needing any calls to writeFragment. I'm not sure if it's the exact same issue though, as it also happens on versions older than 3.6.8. Removing the @export directive does still fix it. React 18 demo.

Camsteack commented 1 year ago

Any update on this ? It's basically not possible to use the @export directive with React 18 ?

mogzol commented 1 year ago

I would also appreciate an update on this. This is still preventing me from updating beyond Apollo 3.6.7 or upgrading my app to React 18. I've had a few other issues caused by the @export directive as well, it seems like it isn't a very well tested feature.

benjamn commented 1 year ago

@mogzol @Camsteack Fix incoming! https://github.com/apollographql/apollo-client/pull/10526

jerelmiller commented 8 months ago

Hey all 👋

It looks like this issue was fixed with 3.7.8 via https://github.com/apollographql/apollo-client/pull/10526. I've confirmed this by upgrading the reproduction and verifying the error has gone away. As such, I'm going to go ahead and close this issue. Thanks!

github-actions[bot] commented 8 months ago

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo Client usage and allow us to serve you better.

github-actions[bot] commented 7 months ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. For general questions, we recommend using StackOverflow or our discord server.