fix(EventStack): fix erroneous removal of non-empty EventPool #2992
Conversation
|
💖 Thanks for opening this pull request! 💖 Here is a list of things that will help get it across the finish line:
We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can. |
Codecov Report
@@ Coverage Diff @@
## master #2992 +/- ##
=======================================
Coverage 99.96% 99.96%
=======================================
Files 163 163
Lines 2743 2743
=======================================
Hits 2742 2742
Misses 1 1Continue to review full report at Codecov.
|
|
I will review it today, need to test it. |
|
@mic4ael does this fix an any existing issue? |
|
@layershifter yes, Closes #2905 |
|
@mic4ael very cool ❤️ |
Signed-off-by: Oleksandr Fediashov <[email protected]>
Signed-off-by: Oleksandr Fediashov <[email protected]>
| if (handlerSet) return handlerSet.hasHandlers() | ||
| return false | ||
| hasHandlers() { | ||
| return this.handlerSets.size > 0 | ||
| } |
There was a problem hiding this comment.
EventPool has handlers if it has any EventHandlerSet.
| this.pools.delete(poolName) | ||
| } | ||
|
|
||
| this.removeTargetHandler(eventType) |
There was a problem hiding this comment.
We will need always to call removeTargetHandler().
| this.pools.set(poolName, newPool) | ||
| } else { | ||
| this.removeTargetHandler(eventType) | ||
| this.pools.delete(poolName) |
There was a problem hiding this comment.
EventPool should not added if it's empty
|
@mic4ael I added some changes, can you please confirm that everything is okay? |
layershifter
changed the title
layershifter
changed the title
|
@layershifter yup, works fine ;) |
|
@layershifter what is going on with the codecov/patch job? It has been stuck for quite some time |
|
Sometimes it fails 🤔 Levi will merge this PR when he will have time. |
|
Congrats on merging your first pull request! 🎉🎉🎉 |
|
Really appreciate this fix. I know it took some real effort to grok the problem and solve the issue ❤️ |
|
Released in |
That fix avoids destruction of an
EventPoolin some cases which results in loss of some of the registered handlers.Fixes #2905.
Fixes #2921.