FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

events: replace NodeCustomEvent with CustomEvent by F3n67u · Pull Request #43876 · nodejs/node · GitHub

/ node Public

events: replace NodeCustomEvent with CustomEvent - #43876

Merged
aduh95 merged 1 commit into
nodejs:mainfrom
F3n67u:node-custom-event
May 11, 2024
ConversationCommits (1)ChecksFiles changed
Merged

events: replace NodeCustomEvent with CustomEvent#43876
aduh95 merged 1 commit into
nodejs:mainfrom
F3n67u:node-custom-event

Conversation

F3n67u commented Jul 17, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

CustomEvent is implemented at #43514, NodeCustomEvent is the same as CustomEvent, so I replace NodeCustomEvent with CustomEvent in favor of web API. cc @daeyeon

Refs: #43514 (comment)

nodejs-github-bot added the needs-ci PRs that need a full CI run. label Jul 17, 2022
F3n67u requested a review from benjamingr July 17, 2022 11:53
F3n67u added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 17, 2022
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 17, 2022

nodejs-github-bot commented Jul 17, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/45487/

F3n67u added the events Issues and PRs related to the events subsystem / EventEmitter. label Jul 17, 2022

benjamingr commented Jul 18, 2022

Copy link
Copy Markdown
Member

@addaleax

addaleax approved these changes Jul 19, 2022
View reviewed changes
Comment thread lib/internal/event_target.js Outdated

addaleax Jul 19, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

If you keep this as class NodeCustomEvent extends CustomEvent {}, that might even be a bit nicer since it’s a way to differentiate between events coming from a user and events coming from a Node.js EventEmitter .emit() call?

F3n67u Jul 21, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Could you help me understand what specific usercase do we need to support? I don't understand in which scenario we would need to differentiate between events coming from a user and events coming from a Node.js EventEmitter .emit() call.

aduh95 force-pushed the node-custom-event branch from 4098a39 to d0560a8 Compare May 11, 2024 09:37
aduh95 added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label May 11, 2024

nodejs-github-bot commented May 11, 2024

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/59094/

aduh95 merged commit bd323b6 into nodejs:main May 11, 2024

aduh95 commented May 11, 2024

Copy link
Copy Markdown
Contributor

Landed in bd323b6

targos pushed a commit that referenced this pull request May 12, 2024
events: replace NodeCustomEvent with CustomEvent
5ee6924 
PR-URL: #43876
Refs: #43514
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos mentioned this pull request May 13, 2024
Merged
marco-ippolito mentioned this pull request Jun 17, 2024
Merged
marco-ippolito pushed a commit that referenced this pull request Jun 17, 2024
events: replace NodeCustomEvent with CustomEvent
75dd009 
PR-URL: #43876
Refs: #43514
Reviewed-By: Anna Henningsen <anna@addaleax.net>
soophoo pushed a commit to soophoo/node that referenced this pull request Jun 20, 2024
events: replace NodeCustomEvent with CustomEvent
2714746 
PR-URL: nodejs#43876
Refs: nodejs#43514
Reviewed-By: Anna Henningsen <anna@addaleax.net>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

addaleax addaleax approved these changes
benjamingr Awaiting requested review from benjamingr

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. events Issues and PRs related to the events subsystem / EventEmitter. needs-ci PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL