Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

events: improve Event compatibility #43461

Merged

Conversation

daeyeon
Copy link
Member

@daeyeon daeyeon commented Jun 17, 2022

This fixes the Event constructor to improve Event Web API compatibility.

The test added was written by referring to wpt@dom/events/Event-constructors.any.js

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Jun 17, 2022
@aduh95 aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Jun 17, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 17, 2022
@nodejs-github-bot
Copy link
Collaborator

Copy link
Member

@lpinca lpinca left a comment

Choose a reason for hiding this comment

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

Nit: can you please rename test-event-whatwg-constructors.js to test-whatwg-event-constructors.js? For consistency with the existing test-whatwg-url-constructor.js.

@daeyeon
Copy link
Member Author

daeyeon commented Jun 18, 2022

It looks better for consistency. Applied the suggestion. Thanks!

Actually, it was named by following test-eventtarget-whatwg-xxx.jss. 😂 Maybe worth renaming such files to avoid confusion.

@daeyeon daeyeon force-pushed the main.web.event-220617.Fri.3c24 branch from 49682ce to a4b26cb Compare June 18, 2022 01:34
This fixes `Event` constructor to improve `Event Web API`
compatibility.

The test added was written by referring to
`wpt@dom/events/Event-constructors.any.js`.

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com
@daeyeon daeyeon force-pushed the main.web.event-220617.Fri.3c24 branch from a4b26cb to b9a0f37 Compare June 18, 2022 01:51
@daeyeon
Copy link
Member Author

daeyeon commented Jun 18, 2022

I squashed the fixup commits and added a commit to rename the files mentioned above.

@lpinca PTAL, and, if it's reasonable, could you remove the commit-queue-squash tag?

@lpinca
Copy link
Member

lpinca commented Jun 18, 2022

@daeyeon can you please move events: rename test-eventtarget-whatwg-xxx.js to a separate PR?

@daeyeon daeyeon force-pushed the main.web.event-220617.Fri.3c24 branch from b9a0f37 to 2d372b3 Compare June 18, 2022 05:54
@daeyeon
Copy link
Member Author

daeyeon commented Jun 18, 2022

@lpinca Thanks for the review, updated!

aduh95 pushed a commit that referenced this pull request Jun 18, 2022
This renames some test filenames to align with the existing
`parallel/test-whatwg-*.js` for consistency.

According to the comment in each file, they are seemingly manually
ported from the `wpt@dom/events` tests.

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: #43467
Refs: #43461
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
@aduh95 aduh95 added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 30, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 30, 2022
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@aduh95 aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 8, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 8, 2022
@nodejs-github-bot nodejs-github-bot merged commit 3f7f62f into nodejs:main Jul 8, 2022
@nodejs-github-bot
Copy link
Collaborator

Landed in 3f7f62f

targos pushed a commit that referenced this pull request Jul 12, 2022
This renames some test filenames to align with the existing
`parallel/test-whatwg-*.js` for consistency.

According to the comment in each file, they are seemingly manually
ported from the `wpt@dom/events` tests.

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: #43467
Refs: #43461
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
targos pushed a commit that referenced this pull request Jul 12, 2022
This fixes `Event` constructor to improve `Event Web API`
compatibility.

The test added was written by referring to
`wpt@dom/events/Event-constructors.any.js`.

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: #43461
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@daeyeon daeyeon deleted the main.web.event-220617.Fri.3c24 branch July 12, 2022 14:05
targos pushed a commit that referenced this pull request Jul 18, 2022
This renames some test filenames to align with the existing
`parallel/test-whatwg-*.js` for consistency.

According to the comment in each file, they are seemingly manually
ported from the `wpt@dom/events` tests.

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: #43467
Refs: #43461
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
targos pushed a commit that referenced this pull request Jul 31, 2022
This renames some test filenames to align with the existing
`parallel/test-whatwg-*.js` for consistency.

According to the comment in each file, they are seemingly manually
ported from the `wpt@dom/events` tests.

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: #43467
Refs: #43461
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
targos pushed a commit that referenced this pull request Jul 31, 2022
This fixes `Event` constructor to improve `Event Web API`
compatibility.

The test added was written by referring to
`wpt@dom/events/Event-constructors.any.js`.

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: #43461
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
This renames some test filenames to align with the existing
`parallel/test-whatwg-*.js` for consistency.

According to the comment in each file, they are seemingly manually
ported from the `wpt@dom/events` tests.

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: nodejs/node#43467
Refs: nodejs/node#43461
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
This fixes `Event` constructor to improve `Event Web API`
compatibility.

The test added was written by referring to
`wpt@dom/events/Event-constructors.any.js`.

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: nodejs/node#43461
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants