ctrlplusb / easy-peasy

Vegetarian friendly state for React
https://easy-peasy.dev
MIT License
5.03k stars 190 forks source link

useLocalStore leaks change subscription #913

Closed yard closed 3 months ago

yard commented 4 months ago

Description Seems that https://github.com/ctrlplusb/easy-peasy/blob/master/src/use-local-store.js#L31 lacks some kind of unsubcription in case the local store is re-created (e.g. due to dependency array change). At the moment, changing the dependency array and thus re-creating the store leads to the new store receiving updates if there is any long-running operation touching the previous instance.

Reproduction I have crafted a simple reproduction at https://codesandbox.io/s/nice-field-6dsmgx.

Clicking on "Start timer" will start updating the counter on the page. Clicking on "Reset store" will briefly reset the counter to 0 (as the new store gets placed into the component's state), but then it will follow with updating the state as the timer from the previous store is still active.

I do appreciate that in most cases we have to cancel all background operations etc, but it is not always possible/easy to do and worst comes to worst, the unmount store should not try updating the state anyway.

Happy to submit a PR to fix this issue if it proves to be valid.

jmyrland commented 4 months ago

Hey @yard !

Tried to check out the sandbox, but it is stuck in "Loading..." 🫤

At the moment, changing the dependency array and thus re-creating the store leads to the new store receiving updates if there is any long-running operation touching the previous instance.

So, if I understand correctly, the "new instance" will get updates from the "old instance"?

A PR would be appreciated!

yard commented 4 months ago

Hi @jmyrland !

Sorry that the sandbox is not working for you, it's not exactly reliable.

Your understanding is absolutely correct – please see the PR with a corresponding test case and a fix (well, quite a word for a single return statement added 😅).

yard commented 3 months ago

Hi @jmyrland ! Did you have a chance to check the PR?

jmyrland commented 3 months ago

Hi @jmyrland ! Did you have a chance to check the PR?

Merged! Will push a new release shortly 👍

jmyrland commented 3 months ago

@yard new version is live now: easy-peasy@6.0.5