Conversation
…re options per DOM spec (#36047) ## Summary `FragmentInstance.addEventListener` and `removeEventListener` fail to cross-match listeners when the `capture` option is passed as a **boolean** in one call and an **options object** in the other. This violates the [DOM Living Standard](https://dom.spec.whatwg.org/#dom-eventtarget-removeeventlistener), which states that `addEventListener(type, fn, true)` and `addEventListener(type, fn, {capture: true})` are identical. ### Root Cause In `ReactFiberConfigDOM.js`, the `normalizeListenerOptions` function generates a listener key string for deduplication. The boolean branch generates a **different format** than the object branch: ```js // Boolean branch (old) — produces "c=1" return `c=${opts ? '1' : '0'}`; // Object branch — produces "c=1&o=0&p=0" return `c=${opts.capture ? '1' : '0'}&o=${opts.once ? '1' : '0'}&p=${opts.passive ? '1' : '0'}`; ``` Because the keys differ, `indexOfEventListener` cannot match them — so `removeEventListener('click', fn, {capture: true})` silently fails to remove a listener registered with `addEventListener('click', fn, true)`, and vice versa. This causes a **memory leak and event listener accumulation** on all Fragment child DOM nodes. ### Fix Normalize the boolean branch to produce the same full key format: ```js // Boolean branch (fixed) — now produces "c=1&o=0&p=0" (matches object branch) return `c=${opts ? '1' : '0'}&o=0&p=0`; ``` This makes both forms produce an identical key, matching the DOM spec behavior. ### When Was This Introduced This bug has been present since `FragmentInstance` event listener tracking was first added. It became reachable in production as of [#36026](#36026) which enabled `enableFragmentRefs` + `enableFragmentRefsInstanceHandles` across all builds (merged 3 days ago). ### Tests Added two new regression tests to `ReactDOMFragmentRefs-test.js`: 1. `removes a capture listener registered with boolean when removed with options object` 2. `removes a capture listener registered with options object when removed with boolean` Both tests were failing before this fix and pass after. ## How did you test this change? Added two new automated tests covering both cross-form removal directions. Existing tests continue to pass. ## Changelog ### React DOM - **Fixed** `FragmentInstance.removeEventListener()` not removing capture-phase listeners when the `capture` option form (boolean vs options object) differs between `add` and `remove` calls.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )