Did anything related to timings and/or callbacks change in React Query v4?

[jonicious]

Hey everyone,

really enjoying React Query, so thanks everyone being involved here.

Our team maintains an AngularJS app (yup, the very first one 🥲) which we are migrating to React. We use ngReact to render React components as AngularJS directives. Because of this, we can not upgrade to React 18 and need to stay on React 17 for now. Eventually, we will upgrade but this probably requires a fork of ngReact.

I am currently upgrading from React Query v3 to v4. While upgrading, I noticed some "React state updates on an unmounted component" errors, similar to #3804. (I know, the React error was removed in React 18 but we can not upgrade at the moment.) I then re-read this blog post which says:

You can have callbacks on useMutation as well as on mutate itself. It is important to know that the callbacks on useMutation fire before the callbacks on mutate. Further, the callbacks on mutate might not fire at all if the component unmounts before the mutation has finished.

In some cases, moving the onSuccess callbacks from useMutation to the mutate function already solved the error. However, some errors were not that easy to fix.

We still use angular-ui-router for navigation. With some workarounds, we are able to use inside our React components.

In certain cases, we are navigating in onSuccess callbacks. In these cases, we still saw the unmounted errors. Finally, I was able to work around them by wrapping the $state.go call in a simple setTimeout even without defining the milliseconds. Sounds a bit related to #3966.

I would have loved to create an example on how to reproduce it but our use case is so specific and would need a whole AngularJS app including React. I am fine with keeping the setTimeout for now but I am just curious why it is happening:

• Did something related to timings change in React Query v4?
• Could it be related to useSyncExternalStore/shim?

I know that our case is special and I do not expect a fix from React Query side, this is why I just started a discussion. Very thankful for any hints or thoughts!

[TkDodo]

I think it's possible that something timing related has changed. Specifically, we've had problems with useSyncExternalStore and our own microtask batching, so we had to change that slightly. For React17, we use the shim for useSyncExternalStore and I'm not sure how that works in detail.

The warning itself is not crucial. We removed our isMounted ref check because that workaround is worse than the warning (which, as you know, was removed in v18)

[jonicious]

Thanks @TkDodo, makes sense! Do you think, it might be worth mentioning this in the migration docs that some applications could be affected? Then again, it's probably only a small fraction of affected applications.