This repository has been archived by the owner on Apr 3, 2024. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Noticed this issue happen in a real app scenario.
Sometimes, dispatching
flushEffectsSymbol
causesentitiesToStop
to be cleared out from the state, without the cleanup ever happening.This was puzzling me at first, because you'd think that if there were any
entitiesToStop
, then they would be stopped before theflushEffectsSymbol
ever gets a chance to be dispatched. But without this patch, callingfireEvent.click(goodbyeButton);
twice right after another causes 2 "goodbye" messages to be logged instead of just the one.In particular, I think what's going on is that when the 2nd goodbye click happens, react batches the dispatch
START
and dispatchflushEffectsSymbol
together before executing the cleanup effect. The sequence is the following:The fix makes sure we never lose any
entitiesToStop
, and that we do clear them from the state when they have actually been stopped.