apollo-client: Not memoize onCompleted and onError never resolves the query

Heads up: we’re not quite ready to publish @apollo/client@3.1.0 yet, but I’ve published an @apollo/client@3.1.0-pre.0 release that includes #6588, if you want to try that in the meantime. 🙏

@pcunha29 I put the project on hold, but I will look into it tomorrow and see if I can get it finished for end of next week. Sorry for the delay. I feel your pain and have completely moved away from Apollo.

@danReynolds You are right, the issue points out that the solution is to memoize the onCompleted and onError. However what the issue is reporting is that the behaviour has changed without being noted as a breaking change (let me know if this was added to the changelog as a breaking change 😅 ).

If the change of behaviour is intended, that’s cool but just need to be documented otherwise I can only see this behaviour change as an unintended bug.

I’m having the same infinite loop but using cache-and-network. It seems this can be reproduced in the original codesandbox by changing the fetch policy to cache-and-network and reloading the page.

I’m using version 3.3.20 and still having the same issue, added useCallback to the onCompleted and onError method and still the onCompleted gets fired infinitely

Never mind I’m leaving it closed but I have my eye on it. Sorry!

@pcunha29 I tried that. No luck. All good though. We have high hopes for the worker. I put it on hold, got distracted and am glad to have gotten a ping from you today. Time to dust it off and finish this.

@pcunha29 That’s good news. I had no luck with useCallback at all. Really strange. I tried updating 3 different projects. The updates worked in 1 project and the other two had the infinite rendering.

Still, I plan on finishing my web worker based one and hope to put it out there as an npm package.

Thanks for the suggestion to memoize with useCallback. After some time spent scouring the open issues on this repo, that’s got me over a major stumbling block in a production app.

Forking the above-linked CodeSandbox, I can still reproduce this with the latest @apollo/client@3.0.0-rc.9. Setting fetchPolicy to any of "network-only", "cache-and-network", or "no-cache" surfaces the issue, and memoizing the onCompleted/onError functions works to fix it in all of these cases.

As discussed in apollographql/react-apollo#4008, is it deliberate that Apollo Client will now effectively require those callbacks to be memoized? It’s not obvious to me that they should be included in the equality check as mentioned above, although I’m not familiar enough with the internals of the client code to comment more decisively. From a “developer experience” point of view though, it’s certainly non-intuitive to have to memoize those functions, particularly when this was not the case prior to 3.0.0-beta.45.

@hwillson I’ve encountered this issue as well. The result is an infinite number of API calls which means the same as our own frontend doing a DDoS attack on our API. We’ve reverted back to version 3.0.0-beta.44 which seems to resolve the issue for now. However, with such serious consequences, I think this should be somewhat high priority bug to fix.

I’ve narrowed the issue with our setup to be exactly the non-memoized onError and onCompleted handlers which, for some reason, seem to be included in making the decision to re-fetch the query.

This is what our component could look like:

function MyComponent() {
  const { data } = useQuery(MY_QUERY, {
    ssr: false,
    onError: () => { /* ... */ }
  });

  return <p>Hello</p>;
}

If I comment the onError handler out, the issue goes away. We had another issue with caching which is not related to this but it resulted for us to apply the following policies for the client.

client.defaultOptions = {
  watchQuery: {
    fetchPolicy: "no-cache",
    errorPolicy: "ignore",
  },
  query: {
    fetchPolicy: "no-cache",
    errorPolicy: "none",
  },
  mutate: {
    errorPolicy: "none",
  },
};

Which, as far as I know, should essentially disable caching altogether. Now, if I comment out the watchQuery policy, the issue with onError handler seems to go away but then, in some specific places, I encounter #6307.

So, for me, the infinite query fetching seems to result from two different configurations:

1. watchQuery.fetchPolicy = "no-cache"
2. Non-memoized onError handler

@TerrySlack i’ll leave this here, in case that helps you. That’s how I have it working now

const [getData] = useLazyQuery(GET_DATA_QUERY, { fetchPolicy: ‘no-cache’, onError: useCallback((err: any) => { …code }, []), onCompleted: useCallback((data: any) => { …code }, []) })

@apollo/client@3.2.5

@brainkim and @TerrySlack thanks for your response! ✋

in the meantime, i was able to solve this with useCallback. I’m using useLazyQuery instead of useQuery, I have no idea if that can affect anything related to other solutions that didn’t work out on my situation

@pcunha29 I’m sorry to hear that. Do you have a runnable example? 👉👈 I keep trying to context-switch and recreate all these issues on my own and then I end up finding out it’s already been fixed. I’m happy to reopen the issue if can create a minimal example!

Just to add, we’re now in a code freeze as we prep for the 3.0 launch (tomorrow 🎉), which means the fix for this won’t make it in for 3.0. We’ll have it out in a 3.0.1 shortly after the 3.0 launch. Thanks for your patience all!

Looks more like a bug and is not documented… is anyone who actual written the code and understand the right behavior in this chat? The repo owners?

It’s because of this equality check on the options: https://github.com/apollographql/apollo-client/blob/master/src/react/data/QueryData.ts#L243:L243. It makes sense, I mean if you pass new options to the query then you probably want it to re-run like if you switched variables it should query with. The issue is with inline function since they’ll be newly created each render. Assuming this is intended behavior, the solve is to memoize your functions.

Currently trying to migrate to v3 but got hit with similar issue (using cache-and-network)