-
-
Notifications
You must be signed in to change notification settings - Fork 15.2k
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
Unsubscribe a listener inside a listener has side-effects on other listeners #461
Comments
Thanks! Would you please add a failing test? (And maybe you'd like to contribute a fix too? ;-) |
…the loop. Unsubscribing during a dispatch would change the listeners array. However that causes the next listener not to fire.
Fixes #461 - Copy listeners array, so subscribes can't affect the loop.
Fixed by #462. |
I have a question about this. The idea is the copy the array so subscribes can't affect the loop. But if the first listener unsubscribes a later listener, that won't be handled correctly. The unsubscribed listener will be called one more time after it was unsubscribed. Is that really the desired behavior? I agree it is a tricky issue and I'm not 100% certain how to address that. But before even trying to create a PR I wanted to take a step back and ask the "big picture" question of "When do we expect an unsubscribe to be applied?". Specifically, how should this case where one listener unsubscribes a later listener? (one of many potential corner cases worth considering) Although not strictly relevant, I think it might be worth nothing that this is an issue with React because you can get into a situation (this is how I got here, in fact) where a component gets unsubscribed when it is unmounted. But the listener is invoke on more time after the unsubscribe/unmounting and the listener calls Thoughts? |
I ran into a case where the subscription listener was called after the unsubscribe. This resulted in setState being called on unmounted components. So I introduced an extra flag to avoid this. But this is pretty complicated. A simpler solution would be to have this handled inside redux which is why I commented on this issue: reduxjs/redux#461
Unsubscribing a listener inside a listener, will splice the
listeners
array in the store ofcreateStore
, while looping over that array usingforEach
indispatch
.For example the following program fails:
countC
is 1, but it should be 2.A fix for this problem would be to copy the array before doing the
forEach
in thedispatch
function, for example:The text was updated successfully, but these errors were encountered: