search

timolins / react-hot-toast

Smoking Hot React Notifications 🔥
https://react-hot-toast.com
MIT License
9.84k stars 331 forks source link

Remove state from dep array of useStore's useEffect #226

Closed antonisprovidakis closed 4 months ago

antonisprovidakis commented 2 years ago

I couldn't figure out any obvious reason for why state was added in the dep array of the useEffect used in useStore, since it's not used inside it.

I removed it and everything keeps working as before (i.e. unit tests pass, manual testing seems just fine).

Actually, things got a bit better since the useEffect does not run for every state update. It only runs once: on mount.

Before

The useEffect runs 7 times (for each subscriber) for the initial mount and all state transitions of a single toast.

https://user-images.githubusercontent.com/58694408/193424820-45e2c76f-29a0-4015-9c2b-11ca3fd46fb7.mp4

After

The useEffect runs 1 time (for each subscriber) only for the initial mount.

https://user-images.githubusercontent.com/58694408/193424823-b100a935-33f9-4d22-a3e2-d341b4844bb7.mp4

vercel[bot] commented 2 years ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
react-hot-toast ✅ Ready (Inspect) Visit Preview Oct 1, 2022 at 7:23PM (UTC)
antonisprovidakis commented 1 year ago

@timolins If this is not going to be merged for some reason, should I close it then?