Repository navigation
fix(javascript): emit no event operation for a missing listener argument #8663
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
041a506
f1d30fe
4d36250
b6917aa
9617f1a
cb36de8
e9e604a
0bd6c11
c964e98
87b7be8
c9427ea
f038fdc
47e3b9b
ad13261
94dc27a
98316a1
c719fb1
88d5732
514a306
9a7b483
3c3684c
65d74b0
b6cc762
5944b69
676c01d
25cc26b
95cd68c
e118fc9
ba119f3
6b18b70
9b39943
09001af
be61d5c
048274c
486aade
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -177,9 +177,17 @@ const collectEvent = ( | |
| if (method === undefined) return; | ||
| const kind = eventKind(method); | ||
| const eventArgument = node.arguments[0]; | ||
| const listenerArgument = EVENT_LISTENER_METHODS.includes(method) | ||
| ? node.arguments[1] | ||
| : undefined; | ||
| const listenerMethod = EVENT_LISTENER_METHODS.includes(method); | ||
| const listenerArgument = listenerMethod ? node.arguments[1] : undefined; | ||
| // A listener method called without a listener argument passes `undefined` to | ||
| // Node's `checkListener`, which throws before it registers or removes | ||
| // anything. Only a spread argument can still reach that position. | ||
| if ( | ||
| listenerMethod && | ||
| listenerArgument === undefined && | ||
| !node.arguments.some((argument) => t.isSpreadElement(argument)) | ||
| ) | ||
| return; | ||
|
Comment on lines
+682
to
+687
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For an EventEmitter subclass defining AGENTS.md reference: AGENTS.md:L21-L21 Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Confirmed on the same root cause as the other thread, and the subclass case is now a regression test. The guard keyed on the property name, so it applied Node's I also kept the uncertainty explicit rather than resolving it toward absence: a receiver that cannot be proven keeps its operation as an unresolved candidate, matching the behavior before this PR. Measured on Node 22.23.3 — the subclass defaults the listener, registers |
||
| // A leading spread shifts the runtime argument positions, so argument 1 is not | ||
| // provably the listener: `e.on(...["x", fn], 1)` registers `fn` and ignores the | ||
| // trailing `1`. | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.