query: useMutation returns a new object on every call
Describe the bug
If I’m reading the code right, I think the useMutation function creates a new object on every call - so if this object is used directly in the dependency array for any further React hooks, it will trigger additional renders.
To Reproduce
function FooComponent() {
const callback = useCallback(() => Promise.resolve("done"), []);
const mutation = useMutation(callback);
const mutationRef = useRef(mutation);
useEffect(() => {
if (mutationRef.current !== mutation) {
console.log(`This should never get logged`);
}
})
return <></>
}
Expected behavior
The log line should never fire: the arguments to useMutation never change so I would expect the same reference back each time (I think this is similar to how useQuery works - at least, I don’t seem to have the same issue there).
Additional context
Looking at the current code would it be sufficient to memoise the returned object on currentResult and mutate? Or would that likely cause further problems?
About this issue
- Original URL
- State: closed
- Created 3 years ago
- Reactions: 9
- Comments: 18 (1 by maintainers)
Commits related to this issue
- fix(ui): react useEffect infinite loop (#941) rel: https://github.com/TanStack/query/issues/1858 — committed to recipeyak/recipeyak by sbdchd 2 years ago
@alesmenzel destructuring can help here:
taking this one step further,
state => mutate(state)is the same as justmutate, so it would read:which in turn, is just
mutate. Probably you simplified the example a bit 😃that being said, the
<Toggle>component will likely only callonChangewhen the user clicks the toggle, not when the functional identity ofhandleChangechanges.I guess the only reason to stick
useCallbackontohandleChangeis if theTogglecomponent is wrapped inReact.memoand you want to avoid an “unnecessary render” of that component. I don’t see how that can result in an api being called hundreds of times per second…This has been a real gotcha, calling some API hundreds of times per second is not fun. This is even more unfortunate because react hooks eslint plugin will warn users that they actually need tu put the whole mutation object into the dependency array, causing all the havoc.
Here is a minimal example of such busted behavior:
An alternative to destructuring is using Function.prototype.call (or bind or apply). This explicitly removes the function’s dependency on
this, so the ESLint rule can safely allow it. Ifmutate/mutateAsyncactually usedthisin their implementations, then this would break those functions. But because they actually don’t, we can usecall/bind/applyas a way of declaring that fact to ESLint.e.g.,
The object returned from
useMutationalways changes, butmutateat least never changes. I’m not sure it’s expected behaviour that the object returned fromuseMutationnever changes. I think most hooks work this way.Destruction does work but becomes verbose when you have multiple mutations in a single block and using multiple functions in other hooks.
To avoid rerunning my dependent hooks every frame I am wrapping the results in a forked version of https://github.com/kentcdodds/use-deep-compare-effect/blob/main/src/index.ts#L28-L43
But that is not enough to guard calls to
mutate()orreset()- can not afford to call those when the state of the mutation changes. Opting for a local eslint override but with a comment.okay weird, maybe it was just slow - the error is still there. sorry for the confusion. Seems like the only way to get around it is destructuring? I think it is safe to destruct
mutateandmutateAsyncfrom the result though.