The onSuccess callback has confusing behavior when specified on useMutation vs. on mutate

When I use the useMutation hook and then call the mutate function multiple times, the onSuccess callback has a confusing behavior for which I can't see a good reason.

When I specify onSuccess globally, as useMutation option, and then do 4 mutations, the callback is called 4 times:

function MutateGlobal() {
  const { mutate } = useMutation(
    async ({ id }) => {
      await new Promise((r) => setTimeout(r, 1000));
    },
    {
      onSuccess: (_, { id }) => {
        console.log("global onSuccess:", id);
      },
    }
  );

  React.useEffect(() => {
    for (let i = 0; i < 4; i++) {
      mutate({ id: i });
    }
  }, []);

  return null;
}

This logs to the console:

global onSuccess: 0
global onSuccess: 1
global onSuccess: 2
global onSuccess: 3

However, when I specify a local onSuccess on each mutate call:

function MutateLocal() {
  const { mutate } = useMutation(async ({ id }) => {
    await new Promise((r) => setTimeout(r, 1000));
  });

  React.useEffect(() => {
    for (let i = 0; i < 4; i++) {
      mutate(
        { id: i },
        {
          onSuccess: (_, { id }) => {
            console.log("local onSuccess:", id);
          },
        }
      );
    }
  }, []);

  return null;
}

only the last mutation is logged:

local onSuccess: 3

I know this is documented and probably intentional, but still I don't understand how it can possibly make sense? A global callback is diligently called for all mutations, but local, supposedly independent callbacks influence and cancel each other? If anything, it should be the other way around. But the ideal behavior I'd expect is to call onSuccess on each mutation, no matter how it's defined.

onError and onSettled has the same behavior.

Tested on React Query 3.34.0.

as you said, its intentional. I see that we've only documented it in the api reference, but it likely also should be mentioned here under additional callbacks.

I think the reason behind it is separation of concerns. useMutation callbacks will always fire because they are used for things required for the mutation to work properly, like optimistic updates and query invalidation.

the callbacks on onSuccess are mostly for component-specific side effects. They will not be called at all if the component unmounts before the mutation finishes, for example. They are useful for triggering redirects or showing error notifications.

You can also get more fine-grained control with mutateAsync, but don't forget error handling, as mutateAsync returns the Promise:

  React.useEffect(() => {
    for (let i = 0; i < 4; i++) {
      mutateAsync({ id: i }).then(() => console.log("local onSuccess:", i));
    }
  }, [mutateAsync]);

the callbacks on onSuccess are mostly for component-specific side effects.

That explains why it's convenient that the local onSuccess is cancelled on unmount: if the intended use case is to update the component UI with some setState, it saves me from doing the is-mounted check myself.

But calling it only for the last mutate call? I don't see a good rationale for that. Imagine I'm rendering a todo list where each item has a trash icon, and clicking on that icon triggers a mutate({ todoId }) call, and I use local onSuccess to do a local UI effect on successful deletion, like showing some notice or "undo" button. Then, as I user, suppose I delete two items quickly after each other, before the first request finishes. Then I get only one onSuccess call. No UI update for the first deletion.

At this moment, my takeaway would be to avoid mutate({ onSuccess }), because its behavior can be erratic, and always do mutateAsync().then(onSuccess) or useMutation({ onSuccess }) instead.

But calling it only for the last mutate call? I don't see a good rationale for that. Imagine I'm rendering a todo list where each item has a trash icon, and clicking on that icon triggers a mutate({ todoId }) call, and I use local onSuccess to do a local UI effect on successful deletion, like showing some notice or "undo" button. Then, as I user, suppose I delete two items quickly after each other, before the first request finishes. Then I get only one onSuccess call. No UI update for the first deletion.

For this specific use-case, I would move the mutation down to each item and give every item their own mutation. I haven't really fired the same mutation multiple times to be honest :)

const Item = ({ id }) => {
  const { mutate } = useMyDeleteMutation(id)
  return <Button onClick={() => mutate({ onSuccess: () => ... )}) />

I haven't really fired the same mutation multiple times to be honest

Our specific use case, the one where we discovered this issue during review, was "deleting selected items":

const { mutate } = useDeleteMutation();
const [selectedIds, setSelectedIds] = useState([]);
const deleteSelected = () => selectedIds.forEach((id) => mutate({ id }, { onSuccess }));
<button onClick={ deleteSelected }>Delete selected</button>

I guess it's always possible to refactor away from the "multiple mutate calls with a local onSuccess callback" pattern, but ... even if a bug has a convenient workaround, it still continues to be a bug doesn't it?

I know, the mutate docs document this:

If you make multiple requests, onSuccess will fire only after the latest call you've made.

but it still feels to me like a documented bug rather than a reasonable behavior.

I see how that can happen with the above code. Looking through the code, all of it is intentional though. We only have one observer, running mutate again will unsubscribe the current observer and create a new one, which is equivalent of unmounting the component. So without a large change, we can't really have the one case we want (only call callbacks if the component is still mounted) without having the second case (only call it for the latest one)

@TkDodo

Thanks @TkDodo for your time looking at this issue. I think I now understand the pros and cons of the onSuccess callback, and that calling mutate multiple times can have some weird results. After all, the isSuccess field has a similar trait -- after:

const { mutate, isSuccess } = useMutation( ... );

and calling mutate multiple times, the isSuccess value will come from the observer for the last mutation, and the previous ones are kind of forgotten.

Navigation Menu

Repository navigation

The onSuccess callback has confusing behavior when specified on useMutation vs. on mutate #3040

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

The onSuccess callback has confusing behavior when specified on useMutation vs. on mutate #3040

Description

Activity

TkDodo commented on Dec 1, 2021

jsnajdr commented on Dec 1, 2021

TkDodo commented on Dec 1, 2021

jsnajdr commented on Dec 1, 2021

TkDodo commented on Dec 1, 2021

jsnajdr commented on Dec 1, 2021

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions