Skip to content

events: expose CustomEvent on the global scope via a CLI flag - #43885

Merged
nodejs-github-bot merged 3 commits into
nodejs:mainfrom
daeyeon:main.customevent-220718.Mon.ddf1
Jul 23, 2022
Merged

events: expose CustomEvent on the global scope via a CLI flag#43885
nodejs-github-bot merged 3 commits into
nodejs:mainfrom
daeyeon:main.customevent-220718.Mon.ddf1

Conversation

@daeyeon

@daeyeon daeyeon commented Jul 18, 2022

Copy link
Copy Markdown
Member

Fixes: #40678
Refs: #43514

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

@nodejs-github-bot

nodejs-github-bot commented Jul 18, 2022

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/startup

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Jul 18, 2022
@daeyeon daeyeon added semver-minor PRs that contain new features and should be released in the next minor version. eventtarget Issues and PRs related to the EventTarget implementation. labels Jul 18, 2022
@targos targos added semver-major PRs that contain breaking changes and should be released in the next major version. and removed semver-minor PRs that contain new features and should be released in the next minor version. labels Jul 18, 2022
@targos

targos commented Jul 18, 2022

Copy link
Copy Markdown
Member

New globals are semver-major.

@aduh95

aduh95 commented Jul 18, 2022

Copy link
Copy Markdown
Contributor

New globals are semver-major.

It looks like it's behind a flag, so semver-minor PRs that contain new features and should be released in the next minor version. seems appropriate, no?

aduh95
aduh95 reviewed Jul 18, 2022
View reviewed changes
Comment thread test/parallel/test-global-customevent.js Resolved
Comment thread test/parallel/test-global-customevent.js Resolved
@targos

targos commented Jul 18, 2022

Copy link
Copy Markdown
Member

New globals are semver-major.

It looks like it's behind a flag, so semver-minor PRs that contain new features and should be released in the next minor version. seems appropriate, no?

I didn't look at the code. If it's behind a flag, the commit message should be clear about that.

@aduh95 aduh95 added semver-minor PRs that contain new features and should be released in the next minor version. and removed semver-major PRs that contain breaking changes and should be released in the next major version. labels Jul 18, 2022
daeyeon added 2 commits July 18, 2022 18:54
@daeyeon
events: expose CustomEvent on global with CLI flag
0c6cc6e 
Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com
@daeyeon
fixup: clarify the test
323ca0f 
Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com
@daeyeon
daeyeon force-pushed the main.customevent-220718.Mon.ddf1 branch from aa6de59 to 323ca0f Compare July 18, 2022 10:26
@daeyeon daeyeon changed the title events: expose CustomEvent on the global scope events: expose CustomEvent on the global scope via a CLI flag Jul 18, 2022
aduh95
aduh95 approved these changes Jul 18, 2022
View reviewed changes

@aduh95 aduh95 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We could enable this flag for WPT tests are remove a few failing cases e.g.

node/test/wpt/status/dom/events.json

Line 24 in 8dce6ad

"CustomEvent is not defined"

node/test/wpt/status/dom/events.json

Line 32 in 8dce6ad

"fail": "CustomEvent is not defined"

@daeyeon
fixup: update the wpt event test
6bc0655 
Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com
@daeyeon
daeyeon force-pushed the main.customevent-220718.Mon.ddf1 branch from eb4fa70 to 6bc0655 Compare July 18, 2022 14:14
@aduh95 aduh95 added author ready PRs with CI started, the required approvals, and no outstanding review comments. request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. labels Jul 18, 2022
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Jul 18, 2022
@nodejs-github-bot

nodejs-github-bot commented Jul 18, 2022

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-bot

nodejs-github-bot commented Jul 18, 2022

Copy link
Copy Markdown
Collaborator

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

@github-actions github-actions Bot mentioned this pull request Jul 19, 2022
Open
19 tasks
@nodejs-github-bot

nodejs-github-bot commented Jul 19, 2022

Copy link
Copy Markdown
Collaborator

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

@github-actions github-actions Bot mentioned this pull request Jul 20, 2022
Open
16 tasks
@aduh95 aduh95 mentioned this pull request Aug 1, 2022
Closed
@targos targos mentioned this pull request Aug 2, 2022
Merged
@daeyeon
daeyeon deleted the main.customevent-220718.Mon.ddf1 branch August 11, 2022 15:12
@daeyeon daeyeon mentioned this pull request Oct 2, 2022
Merged
@himself65 himself65 mentioned this pull request Mar 30, 2024
Closed
@skyclouds2001 skyclouds2001 mentioned this pull request Nov 5, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lpinca lpinca lpinca approved these changes
@aduh95 aduh95 aduh95 approved these changes
@RaisinTen RaisinTen RaisinTen approved these changes
+1 more reviewer
@Mesteery Mesteery Mesteery approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue-squash PRs the Commit Queue should land as one squashed commit. eventtarget Issues and PRs related to the EventTarget implementation. needs-ci PRs that need a full CI run. semver-minor PRs that contain new features and should be released in the next minor version.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Support CustomEvent

7 participants

@daeyeon @nodejs-github-bot @targos @aduh95 @lpinca @RaisinTen @Mesteery