fix(eventEmitter)!: improve event handling and listener management#2277
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## next #2277 +/- ##
===========================================
+ Coverage 76.66% 88.75% +12.08%
===========================================
Files 81 117 +36
Lines 2083 3281 +1198
Branches 532 966 +434
===========================================
+ Hits 1597 2912 +1315
+ Misses 375 331 -44
+ Partials 111 38 -73 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Hello and thanks for the PR!
Firstly, may I ask what's the motivation for the PR?
I'd say it can be merged, but for v6 - the change of behavior, while subtle, is a breaking change. Also the removal of removeListener is a breaking change - it should be added back.
Thank you :)
Hi, I noticed this behavior when experimenting with my own implementations in React. I think this behavior improves stability, even in the worst case. I think this change can be merged into version 6. Not urgent. |
|
@yslpn could you please rebase this PR onto the |
|
@yslpn is attempting to deploy a commit to the Crowdin Team on Vercel. A member of the Team first needs to authorize it. |
Done. Could you change the target branch to next? Either I don't have the necessary permissions or I can't find the button :) |
There was a problem hiding this comment.
Pull request overview
Refactors the core EventEmitter implementation to improve listener management (deduping) and to make listener invocation resilient to modifications during an emit cycle, with accompanying tests to lock in the new behaviors.
Changes:
- Switch internal listener storage from
ArraytoSetto avoid duplicate listener registrations. - Snapshot listeners during
emitto prevent iteration from being affected by add/remove operations inside handlers. - Add tests covering “remove during emit” behavior and duplicate listener prevention.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
packages/core/src/eventEmitter.ts |
Refactors listener storage to Set and updates emit iteration to use a snapshot. |
packages/core/src/eventEmitter.test.ts |
Adds tests for emit-cycle mutation safety and listener deduplication behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
vonovak
left a comment
There was a problem hiding this comment.
thanks for the PR, see my other comment
Description
Types of changes
Refactored EventEmitter + add two new tests
Checklist