Skip to content

lib: add ability to add separate event name to defineEventHandler - #45032

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
jasnell:defineeventtarget-name
Oct 19, 2022
Merged

lib: add ability to add separate event name to defineEventHandler#45032
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
jasnell:defineeventtarget-name

Conversation

@jasnell

@jasnelljasnell commented Oct 16, 2022

Copy link
Copy Markdown
Member

Adds an optional third argument to defineEventHandler() to specify an event name that is separate from the name used for the property. This will be used, for instance, by the quic implementation to support generating the on... events where the event name itself is hyphenated. For instance defineEventHandler(target, 'streamreset', 'stream-reset')

Separated out from #44325

This is an internal only api so no documentation change is needed.

Signed-off-by: James M Snell jasnell@gmail.com

Adds an optional third argument to `defineEventHandler()` to specify
an event name that is separate from the name used for the property.
This will be used, for instance, by the quic implementation to support
generating the `on...` events where the event name itself is hyphenated.
For instance `defineEventHandler(target, 'streamreset', 'stream-reset')`
Separated out from nodejs#44325
Signed-off-by: James M Snell <jasnell@gmail.com>
@nodejs-github-botnodejs-github-bot added the needs-ci PRs that need a full CI run. label Oct 16, 2022
@jasnelljasnell mentioned this pull request Oct 16, 2022
@anonriganonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 17, 2022

@mcollinamcollina left a comment

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.

lgmt

@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 17, 2022
@nodejs-github-bot

This comment was marked as outdated.

@aduh95aduh95 added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 17, 2022
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

nodejs-github-bot commented Oct 17, 2022

Copy link
Copy Markdown
Collaborator

@jasnelljasnell removed the needs-ci PRs that need a full CI run. label Oct 17, 2022
@jasnelljasnell added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 19, 2022
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 19, 2022
@nodejs-github-bot
nodejs-github-bot merged commit 1f7e8d6 into nodejs:mainOct 19, 2022
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 1f7e8d6

RafaelGSS pushed a commit that referenced this pull request Nov 1, 2022
Adds an optional third argument to `defineEventHandler()` to specify
an event name that is separate from the name used for the property.
This will be used, for instance, by the quic implementation to support
generating the `on...` events where the event name itself is hyphenated.
For instance `defineEventHandler(target, 'streamreset', 'stream-reset')`
Separated out from #44325
Signed-off-by: James M Snell <jasnell@gmail.com>
PR-URL: #45032
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@RafaelGSSRafaelGSS mentioned this pull request Nov 1, 2022
RafaelGSS pushed a commit that referenced this pull request Nov 10, 2022
Adds an optional third argument to `defineEventHandler()` to specify
an event name that is separate from the name used for the property.
This will be used, for instance, by the quic implementation to support
generating the `on...` events where the event name itself is hyphenated.
For instance `defineEventHandler(target, 'streamreset', 'stream-reset')`
Separated out from #44325
Signed-off-by: James M Snell <jasnell@gmail.com>
PR-URL: #45032
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@danielleadams

Copy link
Copy Markdown
Contributor

@jasnell this didn't land cleanly on v18.x. Do you mind backporting this? Thank you.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@jasnell@nodejs-github-bot@danielleadams@mcollina@anonrig@aduh95