Skip to content

[DOM] Treat omitted Fragment Event listener options same as capture: false - #37251

Merged
eps1lon merged 2 commits into
react:mainfrom
teamleaderleo:repair/fragment-remove-unregistered-listener
Aug 26, 2026
Merged

[DOM] Treat omitted Fragment Event listener options same as capture: false#37251
eps1lon merged 2 commits into
react:mainfrom
teamleaderleo:repair/fragment-remove-unregistered-listener

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Summary

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.

How did you test this change?

  • added test characterizing the bug in the first commit

@meta-cla meta-cla Bot added the CLA Signed label Aug 8, 2026

@dianatofficial dianatofficial left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Solid PR. Tests cover the important paths.

teamleaderleo and others added 2 commits August 25, 2026 11:19
…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>
@eps1lon
eps1lon force-pushed the repair/fragment-remove-unregistered-listener branch from ad9a9c0 to def5abc Compare August 25, 2026 09:26
@github-actions

github-actions Bot commented Aug 25, 2026

Copy link
Copy Markdown

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

@eps1lon

eps1lon commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

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: FragmentInstance.removeEventListener now returns early when the listener was never registered, before touching child listeners or the registry, and the existing FragmentRef suite already has a regression test for that case. What remains is the options identity issue: omitted options normalize to '0' while false and {capture: false} normalize to 'c=0', so a listener added one way cannot be removed the other way even though EventTarget treats both as capture: false.

I also moved the test into the existing ReactDOMFragmentRefs-test.js suite (dropping the separate test file) and reduced it to the case that still reproduces.

@eps1lon eps1lon changed the title Fix Fragment ref event listener registry identity [DOM] Treat omitted Fragment Event listener options same as capture: false Aug 25, 2026
@eps1lon
eps1lon merged commit a112448 into react:main Aug 26, 2026
462 of 466 checks passed
github-actions Bot pushed a commit that referenced this pull request Aug 26, 2026
… 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)
github-actions Bot pushed a commit that referenced this pull request Aug 26, 2026
… 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)
github-actions Bot pushed a commit to code/lib-react that referenced this pull request Aug 26, 2026
… 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)
github-actions Bot pushed a commit to code/lib-react that referenced this pull request Aug 26, 2026
… 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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants