Skip to content

fix: Make EventTarget methods enumerable#57247

Open
retyui wants to merge 1 commit into
react:mainfrom
retyui:fix-event-target
Open

fix: Make EventTarget methods enumerable#57247
retyui wants to merge 1 commit into
react:mainfrom
retyui:fix-event-target

Conversation

@retyui

@retyui retyui commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Summary:

By spec EventTarget should have 3 enumerable methods (addEventListener, removeEventListener and dispatchEvent)

Screenshot 2026-06-17 at 12 50 15

Changelog:

[GENERAL] [CHANGED] - Make EventTarget methods enumerable by spec

Test Plan:

Create aRN app and run code with/without commented code

// Object.defineProperties(EventTarget.prototype, {
//   addEventListener: { enumerable: true },
//   removeEventListener: { enumerable: true },
//   dispatchEvent: { enumerable: true },
// });

console.log('Start');
for (const key in new EventTarget()) {
  console.log({ key });
}
console.log('End');

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 17, 2026
@facebook-github-tools facebook-github-tools Bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Jun 17, 2026

@Abbondanzo Abbondanzo 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.

This isn't something we do anywhere else in the project. Can you explain the motivation or the spec you're referencing? Even Chrome doesn't do this. You can still call Object.keys(EventTarget.prototype) without this change

@retyui

retyui commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

@Abbondanzo

Can you explain the motivation or the spec you're referencing?

I want to align EventTarget implementation to be compatible with spec.

--

Even Chrome doesn't do this.

How you can prove it? As I see Chrome, Firefox, Safari, Node.js: all of them implements EventTarget with methods as enumerable: true

Screenshot 2026-06-17 at 17 38 02 Screenshot 2026-06-17 at 17 38 27 Screenshot 2026-06-17 at 17 39 47 Screenshot 2026-06-17 at 17 46 20

@retyui retyui requested a review from Abbondanzo June 17, 2026 15:47
@Abbondanzo

Abbondanzo commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Lazily, I was only trying Object.keys(new EventTarget()) and not attempting to turn that into an iterator, so my mistake. This is a good starting point. For the sake of consistency, do you plan to make additional changes to other WebIDL classes (like IntersectionObserver, Performance, CustomEvent, etc.)?

@meta-codesync

meta-codesync Bot commented Jun 17, 2026

Copy link
Copy Markdown

@Abbondanzo has imported this pull request. If you are a Meta employee, you can view this in D108908236.

@retyui retyui force-pushed the fix-event-target branch from 078997f to 991a8ec Compare June 17, 2026 19:31
@retyui

retyui commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

do you plan to make additional changes to other WebIDL classes (like IntersectionObserver, Performance, CustomEvent, etc.)?

@Abbondanzo I hope to back to them after merging abort-controller PR #57230

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

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Callstack Partner: Callstack Partner Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants