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

[jinshin1013]

Intended outcome:
The following query should log either Completed or Error to the console.

useQuery(QUERY, {
  fetchPolicy: 'network-only',
  onCompleted: () => console.log('Completed'),
  onError: () => console.log('Error'),
})

Actual outcome:
The query is never resolved and the component re-renders infinitely.

Is it now recommended to make sure to memoize the query/mutation callbacks (onCompleted, onError, update, etc) when used with fetchPolicy: 'network-only'? The version, @apollo/[email protected] resolved the callbacks fine without having to memoize them.

How to reproduce the issue:
I've created a reproduction on the Code Sandbox: https://codesandbox.io/s/sleepy-mendeleev-1q9jf?file=/src/App.js

Open up the console and you will see loading being logged in the console infinitely. If you however comment the non-memoized onCompleted and onError and uncomment the memoized version and refresh the browser (as below), it will successfully resolve the query and show the data.

const { loading, data } = useQuery(ALL_PEOPLE, {
    fetchPolicy: "network-only",
    // onCompleted: () => console.log("Hello"),
    // onError: () => console.log("error")
    onCompleted: useCallback(() => console.log("Hello"), []),
    onError: useCallback(() => console.log("error"), [])
  });

Versions
Reproducible from @apollo/client: 3.0.0-beta.46

[markusylisiurunen]

@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

[dusty]

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.

[mikeyshing88]

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

[danReynolds]

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.

[jinshin1013]

@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.

[bengal75]

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/[email protected]. 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.

[unutoiul]

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?

[hatchli]

Any update to this? It's really not documented nor an intuitive solution to have to memoize onCompleted/onError...

[bwhitty]

Is this possibly related to https://github.com/apollographql/apollo-client/issues/6416 ?

[hwillson]

The problem is outlined and fixed in https://github.com/apollographql/apollo-client/pull/6588. That PR hasn't been merged yet however, as we aren't entirely happy with the solution. We're considering migrating parts of QueryData (useQuery) into the ObservableQuery class, to handle this a bit differently.

[hwillson]

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!

[eduleite]

Will this fix leave this with the same behaviour of version 2.6? In that version, these callbacks are updated when the component rerenders. Will they be memoized and never be updated in your fix?

Edit: I just found the commit and according to the code, seens like it will just work like version 2.6. Can you confirm this?

[benjamn]

Heads up: we're not quite ready to publish @apollo/[email protected] yet, but I've published an @apollo/[email protected] release that includes #6588, if you want to try that in the meantime. 🙏

[TerrySlack]

I'm still experiencing this behaviour, even with onCompleted and onError functions memoized.
Just non stop rendering with:
"@apollo/client": "^3.2.5",

I'm attempting to migrate from react-apollo and am not having any luck.

Is a fix coming for this?
Where in the official documentation can I read about what actually works and/or provides a fix for this issue?

Edit: I have also started removing the onCompleted and onError functions altogether, and just checking the loading, error and data properties returned from useQuery and useMutation. But still have the same issue. Infinite rendering

[eduleite]

Did you try 3.2.4?
It was really a problem that was fixed. May be a regression in 3.2.5. I'm using 3.2.4 right now and it is working as expected.

[TerrySlack]

@eduleite Just downgraded now and I still get the infinite render.
I have the onCompleted and onError methods back, and memoized, and no luck.

[VFC-elindgren]

@eduleite I have tried on 3.2.2, 3.2.4, and 3.2.5, I have pared component down to

const NewCCWithFlexMicroform = (props: BaseProps) => { const flexResponse = useCybersourceContextQuery({ fetchPolicy: "network-only" }); return <div>Looping?</div>; }

And even this minimal example loops forever. Am trying to find any version >= 3.0.0 that doesn't have this bug. Just upgraded from 2.6.4 and it was a beast, would love not to have to revert those commits

[lauyilouis]

@eduleite Just downgraded now and I still get the infinite render.
I have the onCompleted and onError methods back, and memoized, and no luck.

@TerrySlack

I was facing the same problem until I realised my useCallback is missing the dependency list.
It is:
onCompleted: useCallback(() => {}, [])
instead of:
onCompleted: useCallback(() => {})

This workaround works at the latest stable version 3.2.5. Please try and good luck.

[TerrySlack]

@lauyilouis I still have no luck. I have my onCompleted as you do onCompleted: useCallback(() => {}, [])

[TerrySlack]

@lauyilouis That being said, I'm working on a feature where all api calls will be made through a webworker. So far the prototype works, now going to try it on a branch and see if we can replace all our api calls with it and remove Apollo. This indefinite render has left us stuck on our current version and that's not going to work for us.