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.

[MYKEU]

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 #6416 ?

[hwillson]

The problem is outlined and fixed in #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?

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

[pcunha29]

@TerrySlack i'm facing the same issue, tried to downgrade, useMemo, etc an still having infinite loop. What was your solution? did you had to create that webworker feature that you mentioned?

[brainkim]

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

[TerrySlack]

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

[pcunha29]

@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

[TerrySlack]

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

[pcunha29]

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

[TerrySlack]

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

[brainkim]

Reopening just so I can keep track of it 👀 Still looking for a reproduction against the latest version.

[brainkim]

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

[fernandatoledo]

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

[nivo33]

For me, the only thing that solved this was using useMemo on the variables provided to useQuery AND using useCallback for all the functions given (i.e onCompleted and onError).
On version 3.3.21

[AliRehman7141]

Hi @hwillson,
Kindly reopen it as we are still facing this issue?

[hwillson]

@AliRehman7141 as mentioned in #6301 (comment), can you provide a reproduction? If so we'll re-open. Thanks!

[iamkhalidbashir]

Reproduction is same as the initial comment of this thread
It doesn't work even without useCallback for 3.5.8

[jmtimko5]

I am seeing this as well on 3.5.10

[drager]

Just encountered this problem after we upgraded to React 18 and React Native 0.69.7. We have version 3.4.16 of @apollo/client. We fixed the problem in our components by wrapping onCompleted and onError in a useCallback the infinite requests stopped and it works like intended. I also tried updating @apollo/client to the latest version (3.7.5) without memoized onCompleted and onError and that worked as well. However, it feels like it's better to be safe than sorry and wrap them in useCallback even after the update.

Example:

// Problematic code
const {data} = useSomeQuery({onCompleted: () => console.log('Completed')});

// Memoized function (the fix)
const onCompleted = React.useCallback(() => console.log('Completed'), []);
const {data} = useSomeQuery({onCompleted});

[github-actions]

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.