[DOM] Treat omitted Fragment Event listener options same as capture: false - #37251
Conversation
dianatofficial
left a comment
There was a problem hiding this comment.
Solid PR. Tests cover the important paths.
…ions
Adds a test to the FragmentRefs event listener suite that captures how `FragmentInstance` currently matches listeners when the options argument differs between `addEventListener` and `removeEventListener`. A listener added with omitted options is not matched by a removal passing an explicit `false`, and a listener added with `{capture: false}` is not matched by a removal omitting options, so both listeners keep firing. This contradicts the `EventTarget` contract, where listener identity is the tuple of type, callback, and capture flag, with capture defaulting to `false`.
Co-Authored-By: Claude Code (kimi-k3[1m]) <noreply@anthropic.com>
…dentity
`normalizeListenerOptions` mapped omitted options to `'0'` while explicit `false` and `{capture: false}` mapped to `'c=0'`, so a Fragment listener added without an options argument could not be removed with an explicit capture-false value (or the reverse), even though `EventTarget.removeEventListener` treats both as `capture: false`. Omitted options now normalize to the same `'c=0'` identity, and the characterization test is updated to assert that such removals actually detach the listener.
Co-Authored-By: Claude Code (kimi-k3[1m]) <noreply@anthropic.com>
ad9a9c0 to
def5abc
Compare
|
The build for this commit did not complete, so there is no size report. See the workflow run for details. Generated by sizebot against def5abc |
|
I rebased this onto main and split it into two commits: the first characterizes the current behavior, the second contains the fix and the updated assertions. Two of the three issues were already fixed on main in the meantime: I also moved the test into the existing |
capture: false
… false` (#37251) `FragmentInstance` tracks its event listeners so they can be applied to children added later, and matches them by a normalized options identity. Omitted options currently normalize to a different identity than an explicit `false` or `{capture: false}`, even though both mean `capture: false` per the `EventTarget` contract, where listener identity is the tuple of type, callback, and capture flag. As a result, a listener added without an options argument cannot be removed with an explicit capture-false value (or the reverse). This change normalizes omitted options to the same capture-false identity as `false` and `{capture: false}`. The first commit adds a test to the FragmentRef suite characterizing the current behavior; the second commit contains the fix and the updated assertions. --------- Co-authored-by: Claude Code (kimi-k3[1m]) <noreply@anthropic.com> DiffTrain build for [a112448](a112448)
… false` (#37251) `FragmentInstance` tracks its event listeners so they can be applied to children added later, and matches them by a normalized options identity. Omitted options currently normalize to a different identity than an explicit `false` or `{capture: false}`, even though both mean `capture: false` per the `EventTarget` contract, where listener identity is the tuple of type, callback, and capture flag. As a result, a listener added without an options argument cannot be removed with an explicit capture-false value (or the reverse). This change normalizes omitted options to the same capture-false identity as `false` and `{capture: false}`. The first commit adds a test to the FragmentRef suite characterizing the current behavior; the second commit contains the fix and the updated assertions. --------- Co-authored-by: Claude Code (kimi-k3[1m]) <noreply@anthropic.com> DiffTrain build for [a112448](a112448)
… false` (react#37251) `FragmentInstance` tracks its event listeners so they can be applied to children added later, and matches them by a normalized options identity. Omitted options currently normalize to a different identity than an explicit `false` or `{capture: false}`, even though both mean `capture: false` per the `EventTarget` contract, where listener identity is the tuple of type, callback, and capture flag. As a result, a listener added without an options argument cannot be removed with an explicit capture-false value (or the reverse). This change normalizes omitted options to the same capture-false identity as `false` and `{capture: false}`. The first commit adds a test to the FragmentRef suite characterizing the current behavior; the second commit contains the fix and the updated assertions. --------- Co-authored-by: Claude Code (kimi-k3[1m]) <noreply@anthropic.com> DiffTrain build for [a112448](react@a112448)
… false` (react#37251) `FragmentInstance` tracks its event listeners so they can be applied to children added later, and matches them by a normalized options identity. Omitted options currently normalize to a different identity than an explicit `false` or `{capture: false}`, even though both mean `capture: false` per the `EventTarget` contract, where listener identity is the tuple of type, callback, and capture flag. As a result, a listener added without an options argument cannot be removed with an explicit capture-false value (or the reverse). This change normalizes omitted options to the same capture-false identity as `false` and `{capture: false}`. The first commit adds a test to the FragmentRef suite characterizing the current behavior; the second commit contains the fix and the updated assertions. --------- Co-authored-by: Claude Code (kimi-k3[1m]) <noreply@anthropic.com> DiffTrain build for [a112448](react@a112448)
Summary
FragmentInstancetracks its event listeners so they can be applied to children added later, and matches them by a normalized options identity. Omitted options currently normalize to a different identity than an explicitfalseor{capture: false}, even though both meancapture: falseper theEventTargetcontract, where listener identity is the tuple of type, callback, and capture flag. As a result, a listener added without an options argument cannot be removed with an explicit capture-false value (or the reverse).This change normalizes omitted options to the same capture-false identity as
falseand{capture: false}. The first commit adds a test to the FragmentRef suite characterizing the current behavior; the second commit contains the fix and the updated assertions.How did you test this change?