v5.89.0: onError, onSuccess and onSettled signature change breaks existing code

[stefanofa]

Describe the bug

After upgrading to @tanstack/react-query v5.89.0, my code using useMutation started failing because the onError callback now expects an extra parameter between variables and context (onMutateResult). This change came from PR #9615.

The same applies for onSuccess and onSettled.

Your minimal, reproducible example

https://www.typescriptlang.org/play/?#code/JYWwDg9gTgLgBAbzgVwM4FMCyyYEMbAQB2cAvnAGZQQhwBEAAnkangMYDWA9FOrmzAC0AR2TooATzoBuAFChIsODAlh0iOAFUM2PAWIB5MPpZlK1Wo2at+3XvyGjxUubJVq4u-ISJGTqAB4AEXxcOABeFCIOIggAdyIAGjgANVwoYFwAIwAbdFQIuAA3CGAAEwA+QoMQYBgA2TgtHRxvQ2MfQJC8ZIBRKGooZLSM7LzUCsTG+hBWkwAxIjpZCtd0AA9FeApkIgEfFB0JLxMACggO4lQAfgAuTzmfP06AkvLk1gyiAHMKgEpENNeDBkFASGgsI9iKcENMmrM9D5FvdcKgJHs4KcwOlcCBUADwlVYU0SXBgaCiHJSaQpqSAHQMi7+Wkk4j9QancSDZJFdKZXL5ZJsYgwDYwAHE0lwYAUTFc6B0kD5VC4b7qABk6rgTM61zpbIG0AlcKlOqu+qI7OgnMNQ2KfLGgrgwqIovW4qpUtIJpp01Ifzk3qAA

Steps to reproduce

Previously this compiled: onError(error, variables, context) { /* ... */ }
In v5.89.0 it now must be: onError(error, variables, onMutateResult, context) { /* ... */ }

Expected behavior

Was it expected to ship this as a patch even though it’s breaking? If yes, a doc/changelog note would probably help.

How often does this bug happen?

Every time

Screenshots or Videos

No response

Platform

Not relevant

Tanstack Query adapter

None

TanStack Query version

5.89.0

TypeScript version

5.9.2

Additional context

I am creating this issue also to help anyone confused after upgrading.

[TkDodo]

We added a new param to the end. The old context is now onMutateResult, and the new param at the end is the mutationFunction context. Sorry for the confusion but I don't think adding a new param at the end is breaking.

[fzaninotto]

I'd like to bring my 2c.

All react-admin apps using the latest version of react-query can't compile anymore. That's thousands of apps not working because of this change. It does seem like a bug to me...

[TkDodo]

@fzaninotto what do you mean by that? why cant’t they compile ?

also note that we’ve documented that type changes are not considered breaking:

Changes to types in this repository are considered non-breaking and are usually released as patch semver changes (otherwise every type enhancement would be a major version!).

But also note that this doesn’t mean we break types intentionally or on every release.

[sicin]

Same thing here, I'm surprised it's not a breaking change, although maybe I was using react query wrong. This is the code I used to have.

 const givePermissionMutation = useMutation({
      mutationFn: giveNotificationPermission,
      onMutate: () => {
          const toastId = toast.loading('');
          return { toastId };
      },
      onSuccess: (_, __, { toastId }) => {
          toast.success(t('notificationPermissionGiven'), { id: toastId });
      },
      onError: (e, __, context) => {
          if (context?.toastId) toast.error(t(e.message, { ns: 'httpErrors' }), { id: context.toastId });
      },
  });

Which now outputs the following error:

Type '(data: { pattern: NotificationPattern; output: Output; }, config?: AxiosRequestConfig) => Promise<void>' is not assignable to type 'MutationFunction<void, { pattern: NotificationPattern; output: Output; }>'.
  Types of parameters 'config' and 'context' are incompatible.
    Type 'MutationFunctionContext' has no properties in common with type 'AxiosRequestConfig<any>'.ts(2322)
hydration-iULCH7y8.d.ts(1193, 5): The expected type comes from property 'mutationFn' which is declared here on type 'UseMutationOptions<void, Error, { pattern: NotificationPattern; output: Output; }, { toastId: string; }>'

After changing mutationFn into:

      mutationFn: (data: { pattern: NotificationPattern; output: Output }) => giveNotificationPermission(data),

one error still remains with

Property 'toastId' does not exist on type '{ toastId: string; } | undefined'.ts(2339)
(parameter) toastId: any

I guess I can just add context?.toastId like with onError code, but it got me confused.

[jegork]

@TkDodo the bigger problem (at least for me, and from what I see for @sicin ) is not the addition of 4th parameter, but that the third parameter for onSuccess became from TContext to TOnMutateResult | undefined so all the code depending on the third parameter would need changing.

[TkDodo]

The rename is a bit confusing, sorry about that, but it's a positional parameter - you can name it however you want.

We widened the return type of onSuccess to include undefined but this might have been a mistake. If we can verify that onMutateResult is always defined in onSuccess we can revert it. But it needs to be undefined if there is no onMutate at all

[TkDodo]

I have a fix up here:

• fix(types): onMutateResult is always defined in onSuccess callback #9677

[jegork]

@TkDodo I wasn't explicit enough -- for me the issue was only in the type widening (renaming isn't a problem, at least for me)
thanks for the prompt PR!

[fzaninotto]

Thanks for the swift patch. I tried v5.90.2, but unfortunately it doesn't solve the problem.

Here is an example code snippet that breaks:

https://github.com/marmelab/react-admin/blob/8df4f871667ef0285a92675bf98a75b5998b249b/packages/ra-core/src/controller/create/useCreateController.ts#L97

Typescript complains with the following message:

src/controller/create/useCreateController.ts:97:24 - error TS2554: Expected 4 arguments, but got 3.

97                 return onSuccess(data, variables, context);
                          ~~~~~~~~~

  ../../node_modules/@tanstack/query-core/build/legacy/hydration-B0J2Tmyo.d.ts:1196:87
    1196     onSuccess?: (data: TData, variables: TVariables, onMutateResult: TOnMutateResult, context: MutationFunctionContext) => Promise<unknown> | unknown;
                                                                                               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    An argument for 'context' was not provided.

So it seems the fourth argument is compulsory, which breaks existing code.

[TkDodo]

which breaks existing code.

it only breaks existing code that relied on exactly 3 params being passed. If you build low-level abstractions on top of library code, you probably want:

onSuccess: async (...args) => {
  if (onSuccess) {
    return onSuccess(...args);
  }

or, you make your onSuccess be its own type rather than forwarding the one from react-query.

[fzaninotto]

We could use ...args, except in some places where we manually call the side effects (because we do optimistic and undoable updates), e.g. in

https://github.com/marmelab/react-admin/blob/8df4f871667ef0285a92675bf98a75b5998b249b/packages/ra-core/src/dataProvider/useCreate.ts#L409-L413

For these cases, we have no other way but to manually introduce the 4th argument, which will cause a compilation failure for previous versions of react-query. In other terms, we cannot support both the newer and the older tanstack query versions.

Besides, even if we use ...args and pass along the react-query types in react-admin, the user land code with custom onSuccess callbacks will fail compilation for the same reason (3 params instead of 4).

I keep scratching my head, I can't see any way around this new compulsory 4rth parameter.