Open johnnyoshika opened 4 years ago
This could be somewhat related: https://github.com/apollographql/react-apollo/issues/3333
@apollo/client 3.0.0-rc.0 Changed the behaviour of onCompleted so it must be memoized in the component. We are seeing crazy infinite behavior and are trying to figure out a way in @apollo/react-hoc to deal with that.
The answer is probably just to port all the files that do it. Otherwise crazy fixes would be needed.
https://github.com/apollographql/apollo-client/issues/6301
In the meantime we went back to beta 45
@thomassuckow Oh, good to know that I can go back to beta 45 to get around this problem for now.
I quickly switched this sample app to use beta 45 and everything worked as expected: https://codesandbox.io/s/continents-h0822
So it is indeed a problem that was introduced after that release.
I believe they are intending to keep it this way, which I can understand as it probably fixes a lot of edge cases. But it doesn't interact so well with react-hoc
@thomassuckow They're intending to keep the infinite loop??? Is that a strange decision?
They are intending to keep the requirement that onComplete, et. al needs to be memoized. Since Apollo Client 3 is react hook based, that requirement makes sense (Rules of Hooks). The problem is how the method was typically provided to the HOC does not lend itself to memoization.
@thomassuckow Interesting. Is there documentation whereonCompleted
and memoization is discussed? Can you point me to some sample code where a callback like that is memoized? Thanks in advance.
I cannot for the life of me find what I swear was a dev or a commit stating they intentionally changed the behavior.
Regardless, When using useQuery
the options object has onComplete
. Anywhere that is used, the callback would need to be wrapped in a useCallback
.
const f = useQuery(FOO, { onComplete: () => { doSomething(); doSomethingElse() } });
becomes
const callback = useCallback(() => { doSomething(); doSomethingElse() }, []);
const f = useQuery(FOO, { onComplete: callback });
@thomassuckow that works perfectly, thank you! So if I understand correctly, useQuery
re-requests the GraphQL request if any of the paramaters are different (using shallow comparison, I assume).
Yep. Which when the cache strategy is set to network-only causes an infinite loop if one of them isn't memoized.
I have two functions but same lazyloadquery in one page, one for loading by pagination and another for loading all data. Like this:
function useSomeBooksApi(){
const [fetch,{loading,data}] = useBooksLazyQuery({fetchPolicy: 'cache-and-network'})
const load = (groupId,page,size)=>{
fetch({variables:{groupId,page,size}})
}
return {
load,data,loading
}
}
function useAllBooksApi(){
const [fetch,{loading,data}] = useBooksLazyQuery({
fetchPolicy: 'network-only', // or no-cache
onCompleted(data){
downloadCsv(data.books)
}
})
return {
fetch,data,loading
}
}
When I use those in one page, useBooksLazyQuery calls infinitely. But different lazyloadquery with same usage works fine.
Info:
Intended outcome:
When using
useQuery
withonCompleted
andfetchPolicy
set tocache-and-network
, I expected the network request to be triggered only once.Actual outcome:
Resulted in an infinite render and infinite GraphQL requests.
Sample code:
How to reproduce the issue:
Bug can be reproduced here: https://codesandbox.io/s/continents-h0822
Specifically in this file: https://codesandbox.io/s/continents-h0822?file=/src/Continents.js
The console will show this and execute 4 GraphQL requests before execution will be forced to stop:
Codesandbox's page can be run here: https://h0822.csb.app/
Version
Also reproduced in codesandbox: https://codesandbox.io/s/continents-h0822