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

Most upvoted comments

@alesmenzel destructuring can help here:

const { mutate } = useMutation(changeToggleStateInApi)

const handleChange = useCallback(
    (state) => {
        mutate(state)
    },
    [mutate]
)

taking this one step further, state => mutate(state) is the same as just mutate, so it would read:

const handleChange = useCallback(
    mutate,
    [mutate]
)

which in turn, is just mutate. Probably you simplified the example a bit 😃


that being said, the <Toggle> component will likely only call onChange when the user clicks the toggle, not when the functional identity of handleChange changes.

I guess the only reason to stick useCallback onto handleChange is if the Toggle component is wrapped in React.memo and 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:

const SomeComponent = () => {
       const { isLoading, data } = useQuery('toggle-state', fetchToggleStateFromApi)
	const mutation = useMutation(changeToggleStateInApi)

	const handleChange = useCallback(
		(state) => {
			mutation.mutate(state)
		},
		// [mutation, row] // DO NOT DO THIS even if eslint recommends you to add "mutation" there
		// eslint-disable-next-line
		[mutation.mutate]
	)
	
	return (<Toggle value={data} disabled={isLoading} onChange={handleChange}>)
}

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. If mutate/mutateAsync actually used this in their implementations, then this would break those functions. But because they actually don’t, we can use call/bind/apply as a way of declaring that fact to ESLint.

e.g.,

  const myMutation = useMutation(myAsyncFunction);

  useEffect(() => {
    myMutation.mutateAsync.call(undefined, "first argument");
  }, [myMutation.mutateAsync]);

The object returned from useMutation always changes, but mutate at least never changes. I’m not sure it’s expected behaviour that the object returned from useMutation never 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() or reset() - can not afford to call those when the state of the mutation changes. Opting for a local eslint override but with a comment.

  const closeModal = useCallback(() => {
    updateManyMutation.reset();
    createOneMutation.reset();
    updateOneMutation.reset();
    deleteOneMutation.reset();

    setGroupTemplate(null);
    setModal(null);
    // eslint-disable-next-line react-hooks/exhaustive-deps
  }, [
    // exclude mutations - linter prevents listing only reset functions
    modal,
  ]);

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 mutate and mutateAsync from the result though.