fkhadra / react-toastify

React notification made easy 🚀 !
https://fkhadra.github.io/react-toastify/introduction
MIT License
12.34k stars 676 forks source link

Duplicated Notifications by Race Condition #946

Closed H4ad closed 8 months ago

H4ad commented 1 year ago

Do you want to request a feature or report a bug?

Bug

What is the current behavior?

I added just once the notification but it has been displayed twice.

From what I looked, the problem is with these lines:

https://github.com/fkhadra/react-toastify/blob/97d9afc48c6e2ef0f6240842700251b8b6e64cd9/src/hooks/useToastContainer.ts#L68-L81

Because I'm using StrictMode, this function is called twice but WillMount is not properly cleaned because we call cancelEmit, which makes the cleanup function never be called since we call events using setTimeout:

https://github.com/fkhadra/react-toastify/blob/97d9afc48c6e2ef0f6240842700251b8b6e64cd9/src/core/toast.ts#L325-L334

If the toast is called very close to the last notification, react will join these two calls in a single batch (my guess), and then, the buildToast function cannot verify if the toast is duplicated because it was not added yet to the toastToRender by appendToast.

https://github.com/fkhadra/react-toastify/blob/97d9afc48c6e2ef0f6240842700251b8b6e64cd9/src/hooks/useToastContainer.ts#L218-L233

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem.

https://codesandbox.io/s/amazing-cray-9f19y2

What is the expected behavior?

To not duplicate the notifications.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

H4ad commented 1 year ago

I could fix locally by adding another safe guard condition on buildToast on this line:

if (!canBeRendered(content) || isNotValid(options) || instance.queue.some(t => t.toastProps.toastId === options.toastId)) return;
gudlyf commented 1 year ago

I believe I had a similar issue and resolved it by setting the toastId:

toast.success('Success message here', {
  toastId: new Date().getTime().toString() + Math.random()
});
fkhadra commented 8 months ago

I'll close the issue given it can be solved using a custom id. In v10 it won't happen as the library internal implementation has switched to useSyncExternalStore