From 3cba19c97725975c5a7d0d9aba00d97553158654 Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Tue, 11 Aug 2026 17:50:16 -0700 Subject: [PATCH 1/8] [DOM] Fix Fragment removeEventListener dropping tracked listeners (#37160) Removing an unregistered listener used splice(-1), which deleted the last tracked entry and left the real handler stuck on DOM children. --- .../src/client/ReactFiberConfigDOM.js | 33 ++++++++------- .../__tests__/ReactDOMFragmentRefs-test.js | 41 +++++++++++++++++++ 2 files changed, 57 insertions(+), 17 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index d959a1c6ae96..ffb9ed289f29 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -3073,24 +3073,23 @@ FragmentInstance.prototype.removeEventListener = function ( if (listeners === null) { return; } - if (typeof listeners !== 'undefined' && listeners.length > 0) { - traverseFragmentInstancesAndTextInstances( - this._fragmentFiber, - removeEventListenerFromChild, - type, - listener, - optionsOrUseCapture, - ); - const index = indexOfEventListener( - listeners, - type, - listener, - optionsOrUseCapture, - ); - if (this._eventListeners !== null) { - this._eventListeners.splice(index, 1); - } + const index = indexOfEventListener( + listeners, + type, + listener, + optionsOrUseCapture, + ); + if (index === -1) { + return; } + traverseFragmentInstancesAndTextInstances( + this._fragmentFiber, + removeEventListenerFromChild, + type, + listener, + optionsOrUseCapture, + ); + listeners.splice(index, 1); }; function removeEventListenerFromChild( child: Fiber, diff --git a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js index 5c55ceffafe0..ccf83c809a79 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js @@ -606,6 +606,47 @@ describe('FragmentRefs', () => { expect(logs).toEqual(['B']); }); + // @gate enableFragmentRefs + it('regression: does not detach a registered listener when removing an unregistered one', async () => { + const fragmentRef = React.createRef(); + const childRef = React.createRef(); + const root = ReactDOMClient.createRoot(container); + let logs = []; + + function registeredListener() { + logs.push('registered'); + } + + function unregisteredListener() { + logs.push('unregistered'); + } + + await act(() => { + root.render( + +
child
+
, + ); + }); + + fragmentRef.current.addEventListener('click', registeredListener); + childRef.current.click(); + expect(logs).toEqual(['registered']); + + // Regression: removing a listener that was never added must be a no-op. + // It must not detach registered listeners from fragmentInstance, + // causing them to stay attached to DOM even after removeEventListener. + fragmentRef.current.removeEventListener('click', unregisteredListener); + logs = []; + childRef.current.click(); + expect(logs).toEqual(['registered']); + + fragmentRef.current.removeEventListener('click', registeredListener); + logs = []; + childRef.current.click(); + expect(logs).toEqual([]); + }); + // @gate enableFragmentRefs it('adds and removes event listeners from children with multiple fragments', async () => { const fragmentRef = React.createRef(); From 278d318de1b40d6c96ff4e39ca0a2702ab133521 Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Tue, 11 Aug 2026 18:05:13 -0700 Subject: [PATCH 2/8] [DOM] Blur portaled Fragment focus targets (#37161) focus() passes through portals as it attempts focus down the fiber tree. blur() exited early based on a containment check, causing it to stop at portals. The result could be a focused element that cannot be blurred. Follow up to https://github.com/react/react/pull/37125, which made blur() apply recursively to be consistent with focus(). --- .../src/client/ReactFiberConfigDOM.js | 7 +--- .../__tests__/ReactDOMFragmentRefs-test.js | 35 +++++++++++++++++++ 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index ffb9ed289f29..326e332f3e78 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -3224,7 +3224,6 @@ function collectChildren(child: Fiber, collection: Array): boolean { } // $FlowFixMe[prop-missing] FragmentInstance.prototype.blur = function (this: FragmentInstanceType): void { - // Early exit if activeElement is not within the fragment's parent const parentHostFiber = getFragmentParentInstanceOrContainerFiber( this._fragmentFiber, ); @@ -3239,13 +3238,9 @@ FragmentInstance.prototype.blur = function (this: FragmentInstanceType): void { parentInstanceOrContainer, ); const activeElement = ownerDocument.activeElement; - if ( - activeElement === null || - !parentInstanceOrContainer.contains(activeElement) - ) { + if (activeElement === null) { return; } - traverseFragmentInstancesAndTextInstances( this._fragmentFiber, blurActiveElementWithinFragment, diff --git a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js index ccf83c809a79..dfcaaba26ad1 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js @@ -484,6 +484,41 @@ describe('FragmentRefs', () => { expect(document.activeElement).toEqual(document.body); }); + // @gate enableFragmentRefs + it('removes focus from a portaled element inside of the Fragment', async () => { + const fragmentRef = React.createRef(); + const root = ReactDOMClient.createRoot(container); + + function Test() { + return ( +
+ + {createPortal( +
+ +
, + document.body, + )} +
+
+ ); + } + + await act(() => { + root.render(); + }); + + await act(() => { + fragmentRef.current.focus(); + }); + expect(document.activeElement.id).toEqual('portaled-input'); + + await act(() => { + fragmentRef.current.blur(); + }); + expect(document.activeElement).toEqual(document.body); + }); + // @gate enableFragmentRefs it('does not remove focus from elements outside of the Fragment', async () => { const fragmentRefA = React.createRef(); From db4ee659e9671848c283eb5fbe08daa1e81caebc Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Tue, 11 Aug 2026 18:13:34 -0700 Subject: [PATCH 3/8] [DOM] Find host siblings for nested empty Fragments (#37162) Fixes a bug where `compareDocumentPosition` fiber traversal would stop searching after an empty Fragment fiber. Also affects `scrollIntoView` --- .../__tests__/ReactDOMFragmentRefs-test.js | 83 ++++++++++++++++++- .../src/ReactFiberTreeReflection.js | 19 +++-- packages/shared/ReactDOMFragmentRefShared.js | 9 +- 3 files changed, 96 insertions(+), 15 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js index dfcaaba26ad1..5f506b50e1b9 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js @@ -1924,6 +1924,51 @@ describe('FragmentRefs', () => { ); }); + // @gate enableFragmentRefs + it('handles empty fragments nested inside non-host wrappers', async () => { + const fragmentRef = React.createRef(); + const beforeRef = React.createRef(); + const afterRef = React.createRef(); + const root = ReactDOMClient.createRoot(container); + + function Test() { + return ( +
+
+ + + +
+
+ ); + } + + await act(() => root.render()); + + expectPosition( + fragmentRef.current.compareDocumentPosition(beforeRef.current), + { + preceding: true, + following: false, + contains: false, + containedBy: false, + disconnected: false, + implementationSpecific: true, + }, + ); + expectPosition( + fragmentRef.current.compareDocumentPosition(afterRef.current), + { + preceding: false, + following: true, + contains: false, + containedBy: false, + disconnected: false, + implementationSpecific: true, + }, + ); + }); + // @gate enableFragmentRefs it('handles nested children', async () => { const fragmentRef = React.createRef(); @@ -2368,8 +2413,8 @@ describe('FragmentRefs', () => { expectPosition( fragmentRef.current.compareDocumentPosition(childBRef.current), { - preceding: true, - following: false, + preceding: false, + following: true, contains: false, containedBy: false, disconnected: false, @@ -2640,6 +2685,40 @@ describe('FragmentRefs', () => { expect(siblingBRef.current.scrollIntoView).toHaveBeenCalledTimes(1); }); + // @gate enableFragmentRefs && enableFragmentRefsScrollIntoView + it('finds host siblings when the empty fragment is nested in a non-host wrapper', async () => { + const fragmentRef = React.createRef(); + const beforeRef = React.createRef(); + const afterRef = React.createRef(); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render( +
+
+ + + +
+
, + ); + }); + + beforeRef.current.scrollIntoView = jest.fn(); + afterRef.current.scrollIntoView = jest.fn(); + + // Default / alignToTop=true should use the following host sibling, + // even though the empty fragment's fiber.sibling is null. + fragmentRef.current.scrollIntoView(); + expect(beforeRef.current.scrollIntoView).toHaveBeenCalledTimes(0); + expect(afterRef.current.scrollIntoView).toHaveBeenCalledTimes(1); + + afterRef.current.scrollIntoView.mockClear(); + + fragmentRef.current.scrollIntoView(false); + expect(beforeRef.current.scrollIntoView).toHaveBeenCalledTimes(1); + expect(afterRef.current.scrollIntoView).toHaveBeenCalledTimes(0); + }); + // @gate enableFragmentRefs && enableFragmentRefsScrollIntoView it('calls scrollIntoView on the prev sibling if alignToTop is false', async () => { const fragmentRef = React.createRef(); diff --git a/packages/react-reconciler/src/ReactFiberTreeReflection.js b/packages/react-reconciler/src/ReactFiberTreeReflection.js index a3e8fc67b4ee..c40f56379781 100644 --- a/packages/react-reconciler/src/ReactFiberTreeReflection.js +++ b/packages/react-reconciler/src/ReactFiberTreeReflection.js @@ -472,34 +472,35 @@ export function getFragmentInstanceOrTextInstanceSiblings( result, fiber, parentHostFiber.child, + {foundSelf: false}, ); return result; } /** * Only collects HostText with enableFragmentRefsTextNodes enabled. Otherwise, only collects HostComponent. + * Returns true once the following host sibling has been found. */ function findFragmentInstanceOrTextInstanceSiblings( result: [Fiber | null, Fiber | null], self: Fiber, child: null | Fiber, - foundSelf: boolean = false, + state: {foundSelf: boolean}, ): boolean { while (child !== null) { if (child === self) { - foundSelf = true; - if (child.sibling) { - child = child.sibling; - } else { - return true; - } + // Shared across recursive calls so ancestors can keep scanning for + // following host siblings after a nested empty fragment. + state.foundSelf = true; + child = child.sibling; + continue; } if ( child.tag === HostComponent || child.tag === HostSingleton || (enableFragmentRefsTextNodes && child.tag === HostText) ) { - if (foundSelf) { + if (state.foundSelf) { result[1] = child; return true; } else { @@ -516,7 +517,7 @@ function findFragmentInstanceOrTextInstanceSiblings( result, self, child.child, - foundSelf, + state, ) ) { return true; diff --git a/packages/shared/ReactDOMFragmentRefShared.js b/packages/shared/ReactDOMFragmentRefShared.js index 6478c6be740c..b1c13b6a995a 100644 --- a/packages/shared/ReactDOMFragmentRefShared.js +++ b/packages/shared/ReactDOMFragmentRefShared.js @@ -11,7 +11,7 @@ import type {Fiber} from 'react-reconciler/src/ReactInternalTypes'; -import {getNextSiblingInstanceOrTextInstanceFiber} from 'react-reconciler/src/ReactFiberTreeReflection'; +import {getFragmentInstanceOrTextInstanceSiblings} from 'react-reconciler/src/ReactFiberTreeReflection'; export function compareDocumentPositionForEmptyFragment( fragmentFiber: Fiber, @@ -31,9 +31,10 @@ export function compareDocumentPositionForEmptyFragment( } else { if (parentResult & Node.DOCUMENT_POSITION_CONTAINED_BY) { // otherNode is one of the fragment's siblings. Use the next - // sibling to determine if its preceding or following. - const nextSiblingFiber = - getNextSiblingInstanceOrTextInstanceFiber(fragmentFiber); + // host sibling (via the parent tree, not fiber.sibling) to + // determine if its preceding or following. + const [, nextSiblingFiber] = + getFragmentInstanceOrTextInstanceSiblings(fragmentFiber); if (nextSiblingFiber === null) { result = Node.DOCUMENT_POSITION_PRECEDING; } else { From 809280d595b589981758f5372986b66eb9150f6c Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Tue, 11 Aug 2026 18:19:19 -0700 Subject: [PATCH 4/8] [DOM] Fix Fragment compareDocumentPosition for documentElement and empty portals (#37163) Accept documentElement in the CONTAINS fiber fallback when there is no React fiber, and position empty portaled fragments against the portal container instead of the React host parent. --- .../src/client/ReactFiberConfigDOM.js | 23 ++++++++-- .../__tests__/ReactDOMFragmentRefs-test.js | 46 ++++++++++++++++++- .../ReactDOMFragmentRefsDocument-test.js | 46 +++++++++++++++++++ .../src/ReactFiberTreeReflection.js | 18 ++++++++ 4 files changed, 128 insertions(+), 5 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index 326e332f3e78..2aacf8290f71 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -70,6 +70,7 @@ import { getFragmentInstanceOrTextInstanceSiblings, traverseFragmentInstancesAndTextInstancesDeeply, fiberIsPortaledIntoHost, + getFragmentPortalContainerInfo, isFiberContainedByFragment, isFragmentContainedByFiber, } from 'react-reconciler/src/ReactFiberTreeReflection'; @@ -3420,9 +3421,20 @@ FragmentInstance.prototype.compareDocumentPosition = function ( ); if (children.length === 0) { + // Match non-empty CDP: when portaled, position against the portal + // container rather than the React host parent. + let emptyParentHostInstance = parentHostInstance; + if (fiberIsPortaledIntoHost(this._fragmentFiber)) { + const portalContainer = getFragmentPortalContainerInfo( + this._fragmentFiber, + ); + if (portalContainer != null) { + emptyParentHostInstance = portalContainer; + } + } return compareDocumentPositionForEmptyFragment( this._fragmentFiber, - parentHostInstance, + emptyParentHostInstance, otherNode, getInstanceFromHostFiber, ); @@ -3525,10 +3537,13 @@ function validateDocumentPositionWithFiberTree( } if (documentPosition & Node.DOCUMENT_POSITION_CONTAINS) { if (otherFiber === null) { - // otherFiber could be null if its the document or body element + // otherFiber could be null if its the document, documentElement, or body const ownerDocument = otherNode.ownerDocument; - // $FlowFixMe[invalid-compare] - return otherNode === ownerDocument || otherNode === ownerDocument.body; + return ( + (otherNode as Instance | Document) === ownerDocument || + otherNode === ownerDocument.documentElement || + otherNode === ownerDocument.body + ); } return isFragmentContainedByFiber(fragmentFiber, otherFiber); } diff --git a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js index 5f506b50e1b9..b2cdb032af66 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js @@ -2391,7 +2391,7 @@ describe('FragmentRefs', () => { expectPosition( fragmentRef.current.compareDocumentPosition(document.body), { - preceding: true, + preceding: false, following: false, contains: true, containedBy: false, @@ -2422,6 +2422,50 @@ describe('FragmentRefs', () => { }, ); }); + + // @gate enableFragmentRefs + it('positions empty portaled fragments against the portal container', async () => { + const fragmentRef = React.createRef(); + const reactParentRef = React.createRef(); + const portalTarget = document.createElement('div'); + portalTarget.id = 'portal-target'; + document.body.appendChild(portalTarget); + const root = ReactDOMClient.createRoot(container); + + function Test() { + return ( +
+ {createPortal(, portalTarget)} +
+ ); + } + + await act(() => root.render()); + + // Empty CDP must use the portal container as parent + expectPosition( + fragmentRef.current.compareDocumentPosition(portalTarget), + { + preceding: false, + following: false, + contains: true, + containedBy: false, + disconnected: false, + implementationSpecific: true, + }, + ); + expectPosition( + fragmentRef.current.compareDocumentPosition(reactParentRef.current), + { + preceding: true, + following: false, + contains: false, + containedBy: false, + disconnected: false, + implementationSpecific: true, + }, + ); + }); }); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMFragmentRefsDocument-test.js b/packages/react-dom/src/__tests__/ReactDOMFragmentRefsDocument-test.js index 8157b43f3766..4feb74e1c7d4 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFragmentRefsDocument-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFragmentRefsDocument-test.js @@ -16,6 +16,7 @@ let ReactDOMClient; let act; let document; let Fragment; +let Node; describe('FragmentRefs', () => { beforeEach(() => { @@ -28,10 +29,12 @@ describe('FragmentRefs', () => { const jsdom = new JSDOM.JSDOM(''); document = jsdom.window.document; + Node = jsdom.window.Node; global.window = jsdom.window; global.document = global.window.document; global.navigator = global.window.navigator; global.Event = global.window.Event; + global.Node = Node; }); describe('focus methods', () => { @@ -252,4 +255,47 @@ describe('FragmentRefs', () => { expect(fragmentRef.current.getClientRects()).toEqual(['html-rect']); }); }); + + describe('compareDocumentPosition', () => { + function expectPosition(position, spec) { + const positionResult = { + following: (position & Node.DOCUMENT_POSITION_FOLLOWING) !== 0, + preceding: (position & Node.DOCUMENT_POSITION_PRECEDING) !== 0, + contains: (position & Node.DOCUMENT_POSITION_CONTAINS) !== 0, + containedBy: (position & Node.DOCUMENT_POSITION_CONTAINED_BY) !== 0, + disconnected: (position & Node.DOCUMENT_POSITION_DISCONNECTED) !== 0, + implementationSpecific: + (position & Node.DOCUMENT_POSITION_IMPLEMENTATION_SPECIFIC) !== 0, + }; + expect(positionResult).toEqual(spec); + } + + // @gate enableFragmentRefs + it('treats documentElement as containing the fragment', async () => { + const fragmentRef = React.createRef(); + const container = document.createElement('div'); + document.body.appendChild(container); + const root = ReactDOMClient.createRoot(container); + + await act(() => { + root.render( + +
+ , + ); + }); + + expectPosition( + fragmentRef.current.compareDocumentPosition(document.documentElement), + { + preceding: true, + following: false, + contains: true, + containedBy: false, + disconnected: false, + implementationSpecific: false, + }, + ); + }); + }); }); diff --git a/packages/react-reconciler/src/ReactFiberTreeReflection.js b/packages/react-reconciler/src/ReactFiberTreeReflection.js index c40f56379781..e9fac1f336a9 100644 --- a/packages/react-reconciler/src/ReactFiberTreeReflection.js +++ b/packages/react-reconciler/src/ReactFiberTreeReflection.js @@ -459,6 +459,24 @@ export function fiberIsPortaledIntoHost(fiber: Fiber): boolean { return foundPortalParent; } +export function getFragmentPortalContainerInfo(fiber: Fiber): null | Container { + let parent = fiber.return; + while (parent !== null) { + if (parent.tag === HostPortal) { + return parent.stateNode.containerInfo as Container; + } + if ( + parent.tag === HostRoot || + parent.tag === HostComponent || + parent.tag === HostSingleton + ) { + break; + } + parent = parent.return; + } + return null; +} + export function getFragmentInstanceOrTextInstanceSiblings( fiber: Fiber, ): [Fiber | null, Fiber | null] { From 18c30e7a5e8467fff8c51267dff37c59404c8ce2 Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Tue, 11 Aug 2026 18:25:13 -0700 Subject: [PATCH 5/8] [DOM] Attach Fragment event listeners to committed text children (#37164) Commit and delete previously skipped TEXT_NODE, so listeners added before a text child mounted never reached it. Match addEventListener. --- .../src/client/ReactFiberConfigDOM.js | 21 ++++----- .../__tests__/ReactDOMFragmentRefs-test.js | 43 +++++++++++++++++++ 2 files changed, 54 insertions(+), 10 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index 2aacf8290f71..adf6f2d6f77f 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -3720,17 +3720,18 @@ export function commitNewChildToFragmentInstance( childInstance: InstanceWithFragmentHandles | Text, fragmentInstance: FragmentInstanceType, ): void { - if (childInstance.nodeType === TEXT_NODE) { - return; - } - const instance: InstanceWithFragmentHandles = childInstance as any; const eventListeners = fragmentInstance._eventListeners; if (eventListeners !== null) { for (let i = 0; i < eventListeners.length; i++) { const {type, listener, optionsOrUseCapture} = eventListeners[i]; - instance.addEventListener(type, listener, optionsOrUseCapture); + childInstance.addEventListener(type, listener, optionsOrUseCapture); } } + // Observers and fragment handles only apply to element children. + if (childInstance.nodeType === TEXT_NODE) { + return; + } + const instance: InstanceWithFragmentHandles = childInstance as any; if (fragmentInstance._observers !== null) { fragmentInstance._observers.forEach(observer => { observer.observe(instance); @@ -3745,17 +3746,17 @@ export function deleteChildFromFragmentInstance( childInstance: InstanceWithFragmentHandles | Text, fragmentInstance: FragmentInstanceType, ): void { - if (childInstance.nodeType === TEXT_NODE) { - return; - } - const instance: InstanceWithFragmentHandles = childInstance as any; const eventListeners = fragmentInstance._eventListeners; if (eventListeners !== null) { for (let i = 0; i < eventListeners.length; i++) { const {type, listener, optionsOrUseCapture} = eventListeners[i]; - instance.removeEventListener(type, listener, optionsOrUseCapture); + childInstance.removeEventListener(type, listener, optionsOrUseCapture); } } + if (childInstance.nodeType === TEXT_NODE) { + return; + } + const instance: InstanceWithFragmentHandles = childInstance as any; if (enableFragmentRefsInstanceHandles) { if (instance.reactFragments != null) { instance.reactFragments.delete(fragmentInstance); diff --git a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js index b2cdb032af66..0da72382f9c6 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js @@ -809,6 +809,49 @@ describe('FragmentRefs', () => { expect(hasClicked).toBe(true); }); + // @gate enableFragmentRefs && enableFragmentRefsTextNodes + it('adds an event listener to a newly added text child', async () => { + const fragmentRef = React.createRef(); + const parentRef = React.createRef(); + const root = ReactDOMClient.createRoot(container); + let showText; + + function Component() { + const [shouldShowText, setShouldShowText] = React.useState(false); + showText = () => { + setShouldShowText(true); + }; + + return ( +
+ + {shouldShowText ? 'Hello' : null} + +
+ ); + } + + await act(() => { + root.render(); + }); + + const logs = []; + fragmentRef.current.addEventListener('click', () => { + logs.push('fragment'); + }); + + await act(() => { + showText(); + }); + + const textNode = Array.from(parentRef.current.childNodes).find( + node => node.nodeType === 3, + ); + expect(textNode).not.toBe(undefined); + textNode.dispatchEvent(new MouseEvent('click', {bubbles: true})); + expect(logs).toEqual(['fragment']); + }); + // @gate enableFragmentRefs it('applies event listeners to host children nested within non-host children', async () => { const fragmentRef = React.createRef(); From 305feb9058f38363e25c61d69985b9393b16cc2a Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Tue, 11 Aug 2026 18:32:39 -0700 Subject: [PATCH 6/8] [DOM] Fix Fragment dispatchEvent when the container is a Document (#37165) dispatchEvent appends a temporary Text node to the fragment's nearest host parent, but a Document can't contain Text, so createRoot(document) threw HierarchyRequestError whenever the fragment had a listener or the event didn't bubble. Use a Comment node for Document containers: it is a legal document child and sits at the fragment's own position, unlike documentElement, which would put the target inside the fragment and fire its listeners twice. --- .../src/client/ReactFiberConfigDOM.js | 9 +- .../ReactDOMFragmentRefsDocument-test.js | 101 ++++++++++++++++++ 2 files changed, 109 insertions(+), 1 deletion(-) diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index adf6f2d6f77f..d55db9471be2 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -3156,7 +3156,14 @@ FragmentInstance.prototype.dispatchEvent = function ( (eventListeners !== null && eventListeners.length > 0) || !event.bubbles ) { - const temp = document.createTextNode(''); + // The temporary node stands in for the fragment's position so that its own + // listeners fire before the event propagates to the parent. A Document can + // only hold comments and processing instructions alongside its + // documentElement, so a Text node would be an invalid child there. + const temp = + parentHostInstance.nodeType === DOCUMENT_NODE + ? (parentHostInstance as any as Document).createComment('') + : document.createTextNode(''); if (eventListeners) { for (let i = 0; i < eventListeners.length; i++) { const {type, listener, optionsOrUseCapture} = eventListeners[i]; diff --git a/packages/react-dom/src/__tests__/ReactDOMFragmentRefsDocument-test.js b/packages/react-dom/src/__tests__/ReactDOMFragmentRefsDocument-test.js index 4feb74e1c7d4..c24ca631a411 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFragmentRefsDocument-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFragmentRefsDocument-test.js @@ -34,6 +34,7 @@ describe('FragmentRefs', () => { global.document = global.window.document; global.navigator = global.window.navigator; global.Event = global.window.Event; + global.MouseEvent = global.window.MouseEvent; global.Node = Node; }); @@ -103,6 +104,106 @@ describe('FragmentRefs', () => { expect(fragmentListener).toHaveBeenCalledTimes(1); expect(bodyListener).toHaveBeenCalledTimes(1); }); + + // @gate enableFragmentRefs + it('dispatches to its own listeners when the container is a Document', async () => { + const fragmentRef = React.createRef(); + const root = ReactDOMClient.createRoot(document); + const logs = []; + + await act(() => { + root.render( + <> + + + +
+ + + , + ); + }); + + fragmentRef.current.addEventListener('click', () => { + logs.push('fragment'); + }); + document.addEventListener('click', () => { + logs.push('document'); + }); + + const isCancelable = !fragmentRef.current.dispatchEvent( + new MouseEvent('click', {bubbles: true}), + ); + + expect(logs).toEqual(['fragment', 'document']); + expect(isCancelable).toBe(false); + }); + + // @gate enableFragmentRefs + it('does not propagate through its own children when wrapping documentElement', async () => { + const fragmentRef = React.createRef(); + const root = ReactDOMClient.createRoot(document); + const logs = []; + + await act(() => { + root.render( + + + +
+ + + , + ); + }); + + // This also registers the listener on the child. Because the + // fragment's position is a sibling of , the event must not + // propagate through it and fire the listener a second time. + fragmentRef.current.addEventListener('click', () => { + logs.push('fragment'); + }); + document.addEventListener('click', () => { + logs.push('document'); + }); + + fragmentRef.current.dispatchEvent( + new MouseEvent('click', {bubbles: true}), + ); + + expect(logs).toEqual(['fragment', 'document']); + }); + + // @gate enableFragmentRefs + it('dispatches non-bubbling events when the container is a Document', async () => { + const fragmentRef = React.createRef(); + const root = ReactDOMClient.createRoot(document); + const logs = []; + + await act(() => { + root.render( + <> + + + +
+ + + , + ); + }); + + document.addEventListener('click', () => { + logs.push('document'); + }); + + const isCancelable = !fragmentRef.current.dispatchEvent( + new MouseEvent('click', {bubbles: false}), + ); + + expect(logs).toEqual([]); + expect(isCancelable).toBe(false); + }); }); describe('addEventListener()', () => { From fdaa617ce526e359f2cfa0d800646bdbd07b7123 Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Tue, 11 Aug 2026 18:38:18 -0700 Subject: [PATCH 7/8] [DOM] Apply Fragment listeners to children inserted into portals later (#37166) Commit-time fragment ancestry stopped at HostPortal via isHostParent, so a child added after listeners were registered never received them even though existing portal siblings did. Collect fragment parents past portals and other non-HostComponent host parents so commit bookkeeping matches HostComponent/HostRoot fragment ancestry. --- .../__tests__/ReactDOMFragmentRefs-test.js | 50 +++++++++++++++++++ .../src/ReactFiberCommitHostEffects.js | 48 ++++++++++-------- 2 files changed, 77 insertions(+), 21 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js index 0da72382f9c6..86c1021db657 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js @@ -1075,6 +1075,56 @@ describe('FragmentRefs', () => { expect(logs).toEqual(['child-b']); }); + // @gate enableFragmentRefs + it('applies event listeners to children portaled in after registration', async () => { + const fragmentRef = React.createRef(); + const childARef = React.createRef(); + const childBRef = React.createRef(); + const root = ReactDOMClient.createRoot(container); + let showChildB; + + function Test() { + const [shouldShowChildB, setShouldShowChildB] = React.useState(false); + showChildB = () => { + setShouldShowChildB(true); + }; + + return ( + + {createPortal( + <> +
+ {shouldShowChildB &&
} + , + document.body, + )} + + ); + } + + await act(() => { + root.render(); + }); + + const logs = []; + fragmentRef.current.addEventListener('click', e => { + logs.push(e.target.id); + }); + + childARef.current.click(); + expect(logs).toEqual(['child-a']); + + // child-b is inserted into the same portal after the listener was + // registered, so it should be treated like its sibling child-a. + await act(() => { + showChildB(); + }); + + logs.length = 0; + childBRef.current.click(); + expect(logs).toEqual(['child-b']); + }); + describe('with activity', () => { // @gate enableFragmentRefs it('does not apply event listeners to hidden trees', async () => { diff --git a/packages/react-reconciler/src/ReactFiberCommitHostEffects.js b/packages/react-reconciler/src/ReactFiberCommitHostEffects.js index c734882e9c6d..42a09fb1a1d9 100644 --- a/packages/react-reconciler/src/ReactFiberCommitHostEffects.js +++ b/packages/react-reconciler/src/ReactFiberCommitHostEffects.js @@ -284,7 +284,7 @@ export function commitFragmentInstanceInsertionEffects(fiber: Fiber): void { commitNewChildToFragmentInstance(fiber.stateNode, fragmentInstance); } - if (isFragmentInstanceHostParent(parent)) { + if (isFragmentInstanceHostBoundary(parent)) { return; } @@ -300,7 +300,7 @@ export function commitFragmentInstanceDeletionEffects(fiber: Fiber): void { deleteChildFromFragmentInstance(fiber.stateNode, fragmentInstance); } - if (isFragmentInstanceHostParent(parent)) { + if (isFragmentInstanceHostBoundary(parent)) { return; } @@ -322,23 +322,24 @@ function isHostParent(fiber: Fiber): boolean { ); } -function isFragmentInstanceParent(fiber: Fiber): boolean { - return fiber && fiber.tag === Fragment && fiber.stateNode !== null; -} - -// Fragments collect HostSingleton children regardless of whether the -// singleton is a scope for placement, so their host parent boundary is -// wider than `isHostParent`. -function isFragmentInstanceHostParent(fiber: Fiber): boolean { +// HostPortal / HostHoistable are host parents for placement, but not for +// fragment instance ancestry — commit bookkeeping walks past them so it +// matches getFragmentParentInstanceOrContainerFiber. HostSingleton is a +// fragment host boundary (and a collected child) even when it is not a +// placement scope. +function isFragmentInstanceHostBoundary(fiber: Fiber): boolean { return ( fiber.tag === HostComponent || - // $FlowFixMe[constant-condition] - (supportsSingletons ? fiber.tag === HostSingleton : false) || fiber.tag === HostRoot || - fiber.tag === HostPortal + // $FlowFixMe[constant-condition] + (supportsSingletons ? fiber.tag === HostSingleton : false) ); } +function isFragmentInstanceParent(fiber: Fiber): boolean { + return fiber && fiber.tag === Fragment && fiber.stateNode !== null; +} + function getHostSibling(fiber: Fiber): ?Instance { // We're going to search forward into the tree until we find a sibling host // node. Unfortunately, if multiple insertions are done in a row we have to @@ -535,16 +536,21 @@ function commitPlacement(finishedWork: Fiber): void { parentFragmentInstances.push(fragmentInstance); } } - if (collectFragmentInstances && isFragmentInstanceHostParent(parentFiber)) { - // Fragments collect children only down to the nearest host fiber. - // The search for the placement parent can continue past host fibers - // that are not valid placement parents, like HostSingletons outside - // a singleton scope, but fragments above them own that host fiber - // as a child, not the placed node. + if (hostParentFiber === undefined && isHostParent(parentFiber)) { + // Nearest host parent for placement. Portals still win here so + // children insert into the portal container. + hostParentFiber = parentFiber; + } + if ( + collectFragmentInstances && + isFragmentInstanceHostBoundary(parentFiber) + ) { + // Stop collecting at HostComponent / HostRoot / HostSingleton. + // Placement can continue past non-scope singletons to HostRoot, and + // past portals (already recorded above) to fragment ancestors. collectFragmentInstances = false; } - if (isHostParent(parentFiber)) { - hostParentFiber = parentFiber; + if (hostParentFiber !== undefined && !collectFragmentInstances) { break; } parentFiber = parentFiber.return; From 22e4f993c7ce9373c95aef093b003f84249e2045 Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Tue, 11 Aug 2026 18:46:13 -0700 Subject: [PATCH 8/8] [Fiber] Extract Fragment instance commit helpers into their own module (#37167) Just a small refactor to consolidate FragmentInstance helpers into one module --- .../src/ReactFiberCommitHostEffects.js | 110 ++-------------- .../src/ReactFiberCommitWork.js | 4 +- .../src/ReactFiberFragmentInstance.js | 117 ++++++++++++++++++ 3 files changed, 130 insertions(+), 101 deletions(-) create mode 100644 packages/react-reconciler/src/ReactFiberFragmentInstance.js diff --git a/packages/react-reconciler/src/ReactFiberCommitHostEffects.js b/packages/react-reconciler/src/ReactFiberCommitHostEffects.js index 42a09fb1a1d9..bbcf51ddcf1a 100644 --- a/packages/react-reconciler/src/ReactFiberCommitHostEffects.js +++ b/packages/react-reconciler/src/ReactFiberCommitHostEffects.js @@ -26,7 +26,6 @@ import { HostText, HostPortal, DehydratedFragment, - Fragment, } from './ReactWorkTags'; import {ContentReset, Placement} from './ReactFiberFlags'; import { @@ -57,17 +56,16 @@ import { acquireSingletonInstance, releaseSingletonInstance, isSingletonScope, - commitNewChildToFragmentInstance, - deleteChildFromFragmentInstance, } from './ReactFiberConfig'; import {captureCommitPhaseError} from './ReactFiberWorkLoop'; import {trackHostMutation} from './ReactFiberMutationTracking'; import {runWithFiberInDEV} from './ReactCurrentFiber'; +import {enableFragmentRefs} from 'shared/ReactFeatureFlags'; import { - enableFragmentRefs, - enableFragmentRefsTextNodes, -} from 'shared/ReactFeatureFlags'; + commitNewChildToFragmentInstances, + getParentFragmentInstances, +} from './ReactFiberFragmentInstance'; export function commitHostMount(finishedWork: Fiber) { const type = finishedWork.type; @@ -256,58 +254,6 @@ export function commitShowHideHostTextInstance(node: Fiber, isHidden: boolean) { } } -export function commitNewChildToFragmentInstances( - fiber: Fiber, - parentFragmentInstances: null | Array, -): void { - if ( - (fiber.tag !== HostComponent && - fiber.tag !== HostSingleton && - !(enableFragmentRefsTextNodes && fiber.tag === HostText)) || - // Only run fragment insertion effects for initial insertions - fiber.alternate !== null || - parentFragmentInstances === null - ) { - return; - } - for (let i = 0; i < parentFragmentInstances.length; i++) { - const fragmentInstance = parentFragmentInstances[i]; - commitNewChildToFragmentInstance(fiber.stateNode, fragmentInstance); - } -} - -export function commitFragmentInstanceInsertionEffects(fiber: Fiber): void { - let parent = fiber.return; - while (parent !== null) { - if (isFragmentInstanceParent(parent)) { - const fragmentInstance: FragmentInstanceType = parent.stateNode; - commitNewChildToFragmentInstance(fiber.stateNode, fragmentInstance); - } - - if (isFragmentInstanceHostBoundary(parent)) { - return; - } - - parent = parent.return; - } -} - -export function commitFragmentInstanceDeletionEffects(fiber: Fiber): void { - let parent = fiber.return; - while (parent !== null) { - if (isFragmentInstanceParent(parent)) { - const fragmentInstance: FragmentInstanceType = parent.stateNode; - deleteChildFromFragmentInstance(fiber.stateNode, fragmentInstance); - } - - if (isFragmentInstanceHostBoundary(parent)) { - return; - } - - parent = parent.return; - } -} - function isHostParent(fiber: Fiber): boolean { return ( fiber.tag === HostComponent || @@ -322,24 +268,6 @@ function isHostParent(fiber: Fiber): boolean { ); } -// HostPortal / HostHoistable are host parents for placement, but not for -// fragment instance ancestry — commit bookkeeping walks past them so it -// matches getFragmentParentInstanceOrContainerFiber. HostSingleton is a -// fragment host boundary (and a collected child) even when it is not a -// placement scope. -function isFragmentInstanceHostBoundary(fiber: Fiber): boolean { - return ( - fiber.tag === HostComponent || - fiber.tag === HostRoot || - // $FlowFixMe[constant-condition] - (supportsSingletons ? fiber.tag === HostSingleton : false) - ); -} - -function isFragmentInstanceParent(fiber: Fiber): boolean { - return fiber && fiber.tag === Fragment && fiber.stateNode !== null; -} - function getHostSibling(fiber: Fiber): ?Instance { // We're going to search forward into the tree until we find a sibling host // node. Unfortunately, if multiple insertions are done in a row we have to @@ -524,37 +452,19 @@ function insertOrAppendPlacementNode( function commitPlacement(finishedWork: Fiber): void { // Recursively insert all host nodes into the parent. let hostParentFiber; - let parentFragmentInstances = null; - let collectFragmentInstances = enableFragmentRefs; let parentFiber = finishedWork.return; while (parentFiber !== null) { - if (collectFragmentInstances && isFragmentInstanceParent(parentFiber)) { - const fragmentInstance: FragmentInstanceType = parentFiber.stateNode; - if (parentFragmentInstances === null) { - parentFragmentInstances = [fragmentInstance]; - } else { - parentFragmentInstances.push(fragmentInstance); - } - } - if (hostParentFiber === undefined && isHostParent(parentFiber)) { - // Nearest host parent for placement. Portals still win here so - // children insert into the portal container. + if (isHostParent(parentFiber)) { hostParentFiber = parentFiber; - } - if ( - collectFragmentInstances && - isFragmentInstanceHostBoundary(parentFiber) - ) { - // Stop collecting at HostComponent / HostRoot / HostSingleton. - // Placement can continue past non-scope singletons to HostRoot, and - // past portals (already recorded above) to fragment ancestors. - collectFragmentInstances = false; - } - if (hostParentFiber !== undefined && !collectFragmentInstances) { break; } parentFiber = parentFiber.return; } + // Fragment ancestry is collected separately so portals remain placement + // parents while fragment bookkeeping still walks past them to ancestors. + const parentFragmentInstances = enableFragmentRefs + ? getParentFragmentInstances(finishedWork) + : null; // $FlowFixMe[constant-condition] if (!supportsMutation) { diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 4faf501fb449..415fecf8713f 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -255,9 +255,11 @@ import { commitHostRemoveChild, commitHostSingletonAcquisition, commitHostSingletonRelease, +} from './ReactFiberCommitHostEffects'; +import { commitFragmentInstanceDeletionEffects, commitFragmentInstanceInsertionEffects, -} from './ReactFiberCommitHostEffects'; +} from './ReactFiberFragmentInstance'; import { trackEnterViewTransitions, commitEnterViewTransitions, diff --git a/packages/react-reconciler/src/ReactFiberFragmentInstance.js b/packages/react-reconciler/src/ReactFiberFragmentInstance.js new file mode 100644 index 000000000000..7b76f2434082 --- /dev/null +++ b/packages/react-reconciler/src/ReactFiberFragmentInstance.js @@ -0,0 +1,117 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +import type {FragmentInstanceType} from './ReactFiberConfig'; +import type {Fiber} from './ReactInternalTypes'; + +import { + HostRoot, + HostComponent, + HostSingleton, + HostText, + Fragment, +} from './ReactWorkTags'; +import { + supportsSingletons, + commitNewChildToFragmentInstance, + deleteChildFromFragmentInstance, +} from './ReactFiberConfig'; +import {enableFragmentRefsTextNodes} from 'shared/ReactFeatureFlags'; + +export function commitNewChildToFragmentInstances( + fiber: Fiber, + parentFragmentInstances: null | Array, +): void { + if ( + (fiber.tag !== HostComponent && + fiber.tag !== HostSingleton && + !(enableFragmentRefsTextNodes && fiber.tag === HostText)) || + // Only run fragment insertion effects for initial insertions + fiber.alternate !== null || + parentFragmentInstances === null + ) { + return; + } + for (let i = 0; i < parentFragmentInstances.length; i++) { + const fragmentInstance = parentFragmentInstances[i]; + commitNewChildToFragmentInstance(fiber.stateNode, fragmentInstance); + } +} + +export function commitFragmentInstanceInsertionEffects(fiber: Fiber): void { + let parent = fiber.return; + while (parent !== null) { + if (isFragmentInstanceParent(parent)) { + const fragmentInstance: FragmentInstanceType = parent.stateNode; + commitNewChildToFragmentInstance(fiber.stateNode, fragmentInstance); + } + + if (isFragmentInstanceHostBoundary(parent)) { + return; + } + + parent = parent.return; + } +} + +export function commitFragmentInstanceDeletionEffects(fiber: Fiber): void { + let parent = fiber.return; + while (parent !== null) { + if (isFragmentInstanceParent(parent)) { + const fragmentInstance: FragmentInstanceType = parent.stateNode; + deleteChildFromFragmentInstance(fiber.stateNode, fragmentInstance); + } + + if (isFragmentInstanceHostBoundary(parent)) { + return; + } + + parent = parent.return; + } +} + +export function getParentFragmentInstances( + fiber: Fiber, +): null | Array { + let parentFragmentInstances = null; + let parent = fiber.return; + while (parent !== null) { + if (isFragmentInstanceParent(parent)) { + const fragmentInstance: FragmentInstanceType = parent.stateNode; + if (parentFragmentInstances === null) { + parentFragmentInstances = [fragmentInstance]; + } else { + parentFragmentInstances.push(fragmentInstance); + } + } + if (isFragmentInstanceHostBoundary(parent)) { + break; + } + parent = parent.return; + } + return parentFragmentInstances; +} + +// HostPortal / HostHoistable are host parents for placement, but not for +// fragment instance ancestry — commit bookkeeping walks past them so it +// matches getFragmentParentInstanceOrContainerFiber. HostSingleton is a +// fragment host boundary (and a collected child) even when it is not a +// placement scope. +function isFragmentInstanceHostBoundary(fiber: Fiber): boolean { + return ( + fiber.tag === HostComponent || + fiber.tag === HostRoot || + // $FlowFixMe[constant-condition] + (supportsSingletons ? fiber.tag === HostSingleton : false) + ); +} + +function isFragmentInstanceParent(fiber: Fiber): boolean { + return fiber && fiber.tag === Fragment && fiber.stateNode !== null; +}