Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mutate callbacks not called if component gets unmounted #5052

Closed
Kai-W opened this issue Feb 28, 2023 · 7 comments
Closed

mutate callbacks not called if component gets unmounted #5052

Kai-W opened this issue Feb 28, 2023 · 7 comments

Comments

@Kai-W
Copy link

Kai-W commented Feb 28, 2023

Describe the bug

Mutate Callbacks defined on the mutate call don't get called if the component gets unmounted. This was introduced by the fix for #4804 but is in some cases a not desired behavior.

I have Dropdown menus with actions that trigger mutations. These menus close when the action is clicked and calls a mutation. By the closing menu the Actions itself get unmounted. Callback Methods defined on the useMutation Hook get called but methods provided to the mutate call don't.

Your minimal, reproducible example

https://codesandbox.io/s/agitated-darkness-fbr1bn?file=/src/App.js

Steps to reproduce

Hiding a Component and calling a mutation with provided onSuccess/onError in the same Method does not calls the callbacks methods of the mutate call
e.g a Menu action that hides the Menu and calls mutate in the same onClick Function.

Expected behavior

Mutate Callbacks defined at the useMutation hook and the mutation call should behave the same. One getting called and one doesn't is very confusing.
Mutate Callbacks should be called even if the Mutation is unmounted. So task can be sheduled in Dropdown Menues,.
At least there must be an option to force the onSuccess/onError call even if the component gets unmounted.

How often does this bug happen?

Every time

Screenshots or Videos

No response

Platform

  • Windows 11
  • Chrome 110.0.5481.178

TanStack Query version

v4.24.10

TypeScript version

No response

Additional context

I don't want to move my user feedback in the OnSuccess/OnError method of the useMutation hook. I separated api changes from userFeedback so i have mutations i can reuse at different locations. Additionally I'm using the actions of MaterialUi-X Datagrid so i don't have control about the unmounting of the actions in the Dropdown Menu. The Feature to pass callbacks to the mutate call perfectly worked in my usecase until i updated to the latest version including the fix for #4804, witch broke my app.

@TkDodo
Copy link
Collaborator

TkDodo commented Mar 5, 2023

Callback Methods defined on the useMutation Hook get called but methods provided to the mutate call don't.

yeah this is on purpose. mutate callbacks are tied to the lifecycle of the component. The reason is mainly situations like the following:

  • you have a mutation, you invalidate in onSuccess of useMutation. That is fine because it should always happen
  • but you call things like router.push('/list') in onSuccess of mutate because it should only happen if the component is still mounted when the mutation finishes. If we were to call this callback unconditionally, you would get into situations where the user clicks a button to perform the mutation, it takes some time, so they navigate somewhere else. Then, when the mutation is done, they would be taken to the list, even though they are somewhere else already.

I separated api changes from userFeedback so i have mutations i can reuse at different locations.

You can still do this with a custom hook that gets an additional onSuccess handler passed in:

const useMyMutation = (onSuccess) => useMutation({
  mutationFn: updateSomething,
  onSuccess: async (data) => {
    await invalidateSomething(...)
    onSuccess?.(data)
  }
})

now you have an onSuccess callback that you can pass from your component to keep the separation, but it will always be executed.

@TkDodo TkDodo closed this as not planned Won't fix, can't repro, duplicate, stale Mar 5, 2023
@TkDodo
Copy link
Collaborator

TkDodo commented Mar 5, 2023

also, if I take your sandbox and execute it with an older version where #4848 was not yet available (e.g. 4.21.0), it still doesn't work. callbacks on mutate were never called when the component was already unmounted. What changed with that fix was that callbacks are not called if the mutation was started after the component has already unmounted.

@Kai-W
Copy link
Author

Kai-W commented Mar 6, 2023

well i have problems to reproduce the Problem with an older verison too. Maybe something else also changed that my code broke. So i will have to refactor to the solution you provided.
And i think this behaviour should be described in the docs:
https://react-query-v3.tanstack.com/reference/useMutation
Remaining options extend the same options described above in the useMutation hook. is the current decription of the parameters and they behave differently when the component gets unmounted

@TkDodo
Copy link
Collaborator

TkDodo commented Mar 6, 2023

@Kai-W
Copy link
Author

Kai-W commented Mar 7, 2023

Its mentioned in one sentence in the guides section. Its not mentioned in the API reference.

@TkDodo
Copy link
Collaborator

TkDodo commented Mar 7, 2023

imo the api reference should list the api, all options and what they are doing. it's not really the place to show how each and every option behaves under each and every situation. That's what we have the guides for.

for example, networkMode just says:

- networkMode: 'online' | 'always' | 'offlineFirst
  - optional
  - defaults to 'online'
  - see Network Mode for more information.

and then it points to the guide. It doesn't tell what each option is or does / behaves

@Kai-W
Copy link
Author

Kai-W commented Mar 7, 2023

I disagree. If i want to quickly check what a method does i check the api reference not a guide where somewhere maybe there is the information i am searching for. A section for Gides and concepts is great for initially gathering a basic understanding of the library but isn't great for quickly searching for information.
And the current api reference for the muteate.onSuccess is
This function will fire when the mutation is successful and will be passed the mutation's result. This is apparently not always the case

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants