From 416914f90990e6941a15c20d16b1ef29b90f61ed Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Thu, 16 Jan 2025 15:27:47 -0500 Subject: [PATCH 01/21] Add enableFragmentRefs feature flag --- packages/shared/ReactFeatureFlags.js | 5 +++-- packages/shared/forks/ReactFeatureFlags.native-fb.js | 1 + packages/shared/forks/ReactFeatureFlags.native-oss.js | 2 ++ packages/shared/forks/ReactFeatureFlags.test-renderer.js | 2 ++ packages/shared/forks/ReactFeatureFlags.test-renderer.www.js | 2 ++ packages/shared/forks/ReactFeatureFlags.www.js | 2 ++ 6 files changed, 12 insertions(+), 2 deletions(-) diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 5588004bdf278..bb087a899b22d 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -138,7 +138,7 @@ export const enablePersistedModeClonedFlag = false; export const enableShallowPropDiffing = false; -export const enableSiblingPrerendering = true; +export const enableSiblingPrerendering = false; /** * Enables an expiration time for retry lanes to avoid starvation. @@ -160,9 +160,10 @@ export const enableInfiniteRenderLoopDetection = false; export const enableUseEffectCRUDOverload = false; export const enableFastAddPropertiesInDiffing = true; - export const enableLazyPublicInstanceInFabric = false; +export const enableFragmentRefs = false; + // ----------------------------------------------------------------------------- // Ready for next major. // diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index f996274b86a18..f49f972e59981 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -83,6 +83,7 @@ export const enableThrottledScheduling = false; export const enableViewTransition = false; export const enableSwipeTransition = false; export const enableScrollEndPolyfill = true; +export const enableFragmentRefs = false; // Flow magic to verify the exports of this file match the original version. ((((null: any): ExportsType): FeatureFlagsType): ExportsType); diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 9b878754736ce..baeef0b56483d 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -76,6 +76,8 @@ export const enableFastAddPropertiesInDiffing = false; export const enableLazyPublicInstanceInFabric = false; export const enableScrollEndPolyfill = true; +export const enableFragmentRefs = false; + // Profiling Only export const enableProfilerTimer = __PROFILE__; export const enableProfilerCommitHooks = __PROFILE__; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index f5deddae739be..1e3eedc9e2255 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -76,6 +76,8 @@ export const enableFastAddPropertiesInDiffing = true; export const enableLazyPublicInstanceInFabric = false; export const enableScrollEndPolyfill = true; +export const enableFragmentRefs = false; + // TODO: This must be in sync with the main ReactFeatureFlags file because // the Test Renderer's value must be the same as the one used by the // react package. diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index b15f54484b029..2342b959f99f8 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -87,5 +87,7 @@ export const enableFastAddPropertiesInDiffing = false; export const enableLazyPublicInstanceInFabric = false; export const enableScrollEndPolyfill = true; +export const enableFragmentRefs = false; + // Flow magic to verify the exports of this file match the original version. ((((null: any): ExportsType): FeatureFlagsType): ExportsType); diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 07e7e1f51aa17..a8b0b4e100b18 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -113,5 +113,7 @@ export const enableLazyPublicInstanceInFabric = false; export const enableSwipeTransition = false; +export const enableFragmentRefs = false; + // Flow magic to verify the exports of this file match the original version. ((((null: any): ExportsType): FeatureFlagsType): ExportsType); From 415795eca20b90b0faf90c92d28367a3d89c2e38 Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Fri, 17 Jan 2025 15:50:41 -0500 Subject: [PATCH 02/21] Create and attach fragment instances --- packages/react-art/src/ReactFiberConfigART.js | 18 + .../src/client/ReactFiberConfigDOM.js | 165 ++++++++ .../__tests__/ReactDOMFragmentRefs-test.js | 393 ++++++++++++++++++ .../src/ReactFiberConfigFabric.js | 26 ++ .../src/ReactFiberConfigNative.js | 26 ++ .../src/createReactNoop.js | 12 + .../react-reconciler/src/ReactChildFiber.js | 55 ++- packages/react-reconciler/src/ReactFiber.js | 17 +- .../src/ReactFiberBeginWork.js | 8 + .../src/ReactFiberCommitEffects.js | 35 +- .../src/ReactFiberCommitHostEffects.js | 70 +++- .../src/ReactFiberCommitWork.js | 34 +- .../src/ReactFiberFragmentComponent.js | 16 + .../src/forks/ReactFiberConfig.custom.js | 10 + .../src/ReactFiberConfigTestHost.js | 26 ++ .../ReactElementValidator-test.internal.js | 10 +- .../ReactJSXElementValidator-test.js | 24 +- packages/shared/ReactFeatureFlags.js | 2 +- ...actFeatureFlags.test-renderer.native-fb.js | 1 + .../forks/ReactFeatureFlags.www-dynamic.js | 1 + .../shared/forks/ReactFeatureFlags.www.js | 3 +- 21 files changed, 920 insertions(+), 32 deletions(-) create mode 100644 packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js create mode 100644 packages/react-reconciler/src/ReactFiberFragmentComponent.js diff --git a/packages/react-art/src/ReactFiberConfigART.js b/packages/react-art/src/ReactFiberConfigART.js index 1ca506d022f4e..048b5c3e5adfc 100644 --- a/packages/react-art/src/ReactFiberConfigART.js +++ b/packages/react-art/src/ReactFiberConfigART.js @@ -318,6 +318,24 @@ export function cloneMutableTextInstance(textInstance) { return textInstance; } +export type FragmentInstance = null | { + appendChild: (child: any) => void, + parentInstance: any, + ... +}; + +export function createFragmentInstance(parentInstance): null { + return null; +} + +export function appendChildToFragmentInstance(child, fragmentInstance): void { + // Noop +} + +export function removeChildFromFragmentInstance(child, fragmentInstance): void { + // Noop +} + export function finalizeInitialChildren(domElement, type, props) { return false; } diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index 47d6a4cb24806..9a2cc5f1b7b67 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -2076,6 +2076,171 @@ export function subscribeToGestureDirection( } } +type EventListenerOptionsOrUseCapture = + | boolean + | { + capture?: boolean, + once?: boolean, + passive?: boolean, + signal?: AbortSignal, + ... + }; + +type StoredEventListener = { + type: string, + listener: EventListener, + optionsOrUseCapture: void | EventListenerOptionsOrUseCapture, +}; + +export type FragmentInstance = { + _children: Set, + _eventListeners: Array, + parentInstance: Instance | Container, + appendChild(child: Element): void, + removeChild(child: Element): void, + addEventListener( + type: string, + listener: EventListener, + optionsOrUseCapture?: EventListenerOptionsOrUseCapture, + ): void, + removeEventListener( + type: string, + listener: EventListener, + optionsOrUseCapture?: EventListenerOptionsOrUseCapture, + ): void, +}; + +function FragmentInstancePseudoElement( + this: FragmentInstance, + parentInstance: Container, +) { + this.parentInstance = parentInstance || document.documentElement; + if (this.parentInstance === null) { + throw new Error( + 'React expected a Fragment Ref to have a parent HTMLElement or to fallback to an element ' + + '(document.documentElement). But no parent or element was found. React never removes the ' + + 'documentElement for any Document it renders into so the cause is likely in some other script ' + + 'running on this page.', + ); + } + this._children = new Set(); + this._eventListeners = []; +} +// $FlowFixMe[prop-missing] +FragmentInstancePseudoElement.prototype.appendChild = function ( + this: FragmentInstance, + child: Element, +): void { + this._children.add(child); + for (let i = 0; i < this._eventListeners.length; i++) { + const {type, listener, optionsOrUseCapture} = this._eventListeners[i]; + child.addEventListener(type, listener, optionsOrUseCapture); + } +}; +// $FlowFixMe[prop-missing] +FragmentInstancePseudoElement.prototype.removeChild = function ( + this: FragmentInstance, + child: Element, +): void { + this._children.delete(child); + for (let i = 0; i < this._eventListeners.length; i++) { + const {type, listener, optionsOrUseCapture} = this._eventListeners[i]; + child.removeEventListener(type, listener, optionsOrUseCapture); + } +}; +// $FlowFixMe[prop-missing] +FragmentInstancePseudoElement.prototype.addEventListener = function ( + this: FragmentInstance, + type: string, + listener: EventListener, + optionsOrUseCapture?: EventListenerOptionsOrUseCapture, +): void { + // Element.addEventListener will only apply uniquely new event listeners by default. Since we + // need to collect the listeners to apply to appended children, we track them ourselves and use + // custom equality check for the options. + const isNewEventListener = + indexOfEventListener(this._eventListeners, { + type, + listener, + optionsOrUseCapture, + }) === -1; + if (isNewEventListener) { + this._eventListeners.push({type, listener, optionsOrUseCapture}); + this._children.forEach(child => { + child.addEventListener(type, listener, optionsOrUseCapture); + }); + } +}; +// $FlowFixMe[prop-missing] +FragmentInstancePseudoElement.prototype.removeEventListener = function ( + this: FragmentInstance, + type: string, + listener: EventListener, + optionsOrUseCapture?: EventListenerOptionsOrUseCapture, +): void { + this._children.forEach(child => { + child.removeEventListener(type, listener, optionsOrUseCapture); + }); + const index = indexOfEventListener(this._eventListeners, { + type, + listener, + optionsOrUseCapture, + }); + this._eventListeners.splice(index, 1); +}; + +function normalizeListenerOptions( + opts: ?EventListenerOptionsOrUseCapture, +): string { + if (opts == null) { + return '0'; + } + + if (typeof opts === 'boolean') { + return `c=${opts ? '1' : '0'}`; + } + + return `c=${opts.capture ? '1' : '0'}&o=${opts.once ? '1' : '0'}&p=${opts.passive ? '1' : '0'}`; +} + +function indexOfEventListener( + eventListeners: Array, + eventListenerToFind: StoredEventListener, +): number { + for (let i = 0; i < eventListeners.length; i++) { + const item = eventListeners[i]; + if ( + item.type === eventListenerToFind.type && + item.listener === eventListenerToFind.listener && + normalizeListenerOptions(item.optionsOrUseCapture) === + normalizeListenerOptions(eventListenerToFind.optionsOrUseCapture) + ) { + return i; + } + } + return -1; +} + +export function createFragmentInstance( + parentInstance: Instance | Container, +): FragmentInstance { + return new (FragmentInstancePseudoElement: any)(parentInstance); +} + +export function appendChildToFragmentInstance( + childElement: Element, + fragmentInstance: FragmentInstance, +): void { + fragmentInstance.appendChild(childElement); +} + +export function removeChildFromFragmentInstance( + childElement: Element, + fragmentInstance: FragmentInstance, +): void { + fragmentInstance.removeChild(childElement); +} + export function clearContainer(container: Container): void { const nodeType = container.nodeType; if (nodeType === DOCUMENT_NODE) { diff --git a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js new file mode 100644 index 0000000000000..b592a06032297 --- /dev/null +++ b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js @@ -0,0 +1,393 @@ +/** + * 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. + * + * @emails reactcore + */ + +'use strict'; + +let React; +let ReactDOMClient; +let act; +let container; + +describe('FragmentRefs', () => { + beforeEach(() => { + jest.resetModules(); + React = require('react'); + ReactDOMClient = require('react-dom/client'); + act = require('internal-test-utils').act; + container = document.createElement('div'); + }); + + // @gate enableFragmentRefs + it('attaches a ref to Fragment', async () => { + const fragmentRef = React.createRef(); + const root = ReactDOMClient.createRoot(container); + + await act(() => + root.render( +
+ +
Hi
+
+
, + ), + ); + expect(container.innerHTML).toEqual( + '
Hi
', + ); + + expect(fragmentRef.current).not.toBe(null); + }); + + // @gate enableFragmentRefs + it('tracks added and removed children', async () => { + let addChild; + let removeChild; + const fragmentRef = React.createRef(); + + function Test() { + const [extraChildCount, setExtraChildCount] = React.useState(0); + addChild = () => { + setExtraChildCount(prev => prev + 1); + }; + removeChild = () => { + setExtraChildCount(prev => prev - 1); + }; + + return ( +
+ + {Array.from({length: extraChildCount}).map((_, index) => ( +
+ Extra Child {index} +
+ ))} +
+
+ ); + } + + const root = ReactDOMClient.createRoot(container); + await act(() => root.render()); + expect(fragmentRef.current._children.size).toBe(0); + await act(() => addChild()); + expect(fragmentRef.current._children.size).toBe(1); + await act(() => removeChild()); + expect(fragmentRef.current._children.size).toBe(0); + }); + + // @gate enableFragmentRefs + it('tracks nested host children', async () => { + const fragmentRef = React.createRef(); + const root = ReactDOMClient.createRoot(container); + + function Wrapper({children}) { + return children; + } + + await act(() => { + root.render( + + +
+ + +
+ +
+
+
+ + + +
+ +
+ , + ); + }); + + expect(fragmentRef.current._children.size).toBe(5); + }); + + // @gate enableFragmentRefs + it('handles empty children', async () => { + const fragmentRef = React.createRef(); + const root = ReactDOMClient.createRoot(container); + + await act(() => { + root.render(); + }); + + expect(fragmentRef.current._children.size).toBe(0); + }); + + // @gate enableFragmentRefs + it('accepts a ref callback', async () => { + let fragmentRef; + const root = ReactDOMClient.createRoot(container); + + await act(() => { + root.render( + (fragmentRef = ref)}> +
Hi
+
, + ); + }); + + expect(fragmentRef._children.size).toBe(1); + }); + + // @gate enableFragmentRefs + it('is available in effects', async () => { + function Test() { + const fragmentRef = React.useRef(null); + React.useLayoutEffect(() => { + expect(fragmentRef.current).not.toBe(null); + }); + React.useEffect(() => { + expect(fragmentRef.current).not.toBe(null); + }); + return ( + +
+ + ); + } + + const root = ReactDOMClient.createRoot(container); + await act(() => root.render()); + }); + + describe('event listeners', () => { + // @gate enableFragmentRefs + it('adds and removes event listeners from children', async () => { + const parentRef = React.createRef(); + const fragmentRef = React.createRef(); + const childARef = React.createRef(); + const childBRef = React.createRef(); + const root = ReactDOMClient.createRoot(container); + + let logs = []; + + function handleFragmentRefClicks() { + logs.push('fragmentRef'); + } + + function Test() { + React.useEffect(() => { + fragmentRef.current.addEventListener( + 'click', + handleFragmentRefClicks, + ); + + return () => { + // TODO: This is a noop here because the tracked child nodes were deleted and they + // remove their own event listeners. Decide if we want to change the timing of this. + // instance._children is currently empty at this point. + fragmentRef.current.removeEventListener( + 'click', + handleFragmentRefClicks, + ); + }; + }, []); + return ( +
+ +
A
+
B
+
+
+ ); + } + + await act(() => { + root.render(); + }); + + childARef.current.addEventListener('click', () => { + logs.push('A'); + }); + + childBRef.current.addEventListener('click', () => { + logs.push('B'); + }); + + // Clicking on the parent should not trigger any listeners + parentRef.current.click(); + expect(logs).toEqual([]); + + // Clicking a child triggers its own listeners and the Fragment's + childARef.current.click(); + expect(logs).toEqual(['fragmentRef', 'A']); + + logs = []; + + childBRef.current.click(); + expect(logs).toEqual(['fragmentRef', 'B']); + + logs = []; + + fragmentRef.current.removeEventListener('click', handleFragmentRefClicks); + + childARef.current.click(); + expect(logs).toEqual(['A']); + + logs = []; + + childBRef.current.click(); + expect(logs).toEqual(['B']); + }); + + // @gate enableFragmentRefs + it('adds and removes event listeners from children with multiple fragments', async () => { + const fragmentRef = React.createRef(); + const nestedFragmentRef = React.createRef(); + const childARef = React.createRef(); + const childBRef = React.createRef(); + const root = ReactDOMClient.createRoot(container); + + await act(() => { + root.render( +
+ +
A
+
+ +
B
+
+
+
+
, + ); + }); + + let logs = []; + + function handleFragmentRefClicks() { + logs.push('fragmentRef'); + } + + function handleNestedFragmentRefClicks() { + logs.push('nestedFragmentRef'); + } + + fragmentRef.current.addEventListener('click', handleFragmentRefClicks); + nestedFragmentRef.current.addEventListener( + 'click', + handleNestedFragmentRefClicks, + ); + + childBRef.current.click(); + // Event bubbles to the parent fragment + expect(logs).toEqual(['nestedFragmentRef', 'fragmentRef']); + + logs = []; + + childARef.current.click(); + expect(logs).toEqual(['fragmentRef']); + + logs = []; + + fragmentRef.current.removeEventListener('click', handleFragmentRefClicks); + nestedFragmentRef.current.removeEventListener( + 'click', + handleNestedFragmentRefClicks, + ); + childBRef.current.click(); + expect(logs).toEqual([]); + }); + + // @gate enableFragmentRefs + it('adds an event listener to a newly added child', async () => { + const fragmentRef = React.createRef(); + const childRef = React.createRef(); + const root = ReactDOMClient.createRoot(container); + let showChild; + + function Component() { + const [shouldShowChild, setShouldShowChild] = React.useState(false); + showChild = () => { + setShouldShowChild(true); + }; + + return ( +
+ +
A
+ {shouldShowChild && ( +
+ B +
+ )} +
+
+ ); + } + + await act(() => { + root.render(); + }); + + expect(fragmentRef.current).not.toBe(null); + expect(childRef.current).toBe(null); + + let hasClicked = false; + fragmentRef.current.addEventListener('click', () => { + hasClicked = true; + }); + + await act(() => { + showChild(); + }); + expect(childRef.current).not.toBe(null); + + childRef.current.click(); + expect(hasClicked).toBe(true); + }); + + // @gate enableFragmentRefs + it('applies event listeners to host children nested within non-host children', async () => { + const fragmentRef = React.createRef(); + const childRef = React.createRef(); + const nestedChildRef = React.createRef(); + const root = ReactDOMClient.createRoot(container); + + function Wrapper({children}) { + return children; + } + + await act(() => { + root.render( +
+ +
Host A
+ + + +
Host B
+
+
+
+
+
, + ); + }); + const logs = []; + fragmentRef.current.addEventListener('click', e => { + logs.push(e.target.textContent); + }); + + expect(logs).toEqual([]); + childRef.current.click(); + expect(logs).toEqual(['Host A']); + nestedChildRef.current.click(); + expect(logs).toEqual(['Host A', 'Host B']); + }); + }); +}); diff --git a/packages/react-native-renderer/src/ReactFiberConfigFabric.js b/packages/react-native-renderer/src/ReactFiberConfigFabric.js index 6db223e4b4dc5..9ba26aee8ef95 100644 --- a/packages/react-native-renderer/src/ReactFiberConfigFabric.js +++ b/packages/react-native-renderer/src/ReactFiberConfigFabric.js @@ -591,6 +591,32 @@ export function waitForCommitToBeReady(): null { return null; } +export type FragmentInstance = null | { + appendChild: (child: Instance) => void, + parentInstance: Instance, + ... +}; + +export function createFragmentInstance( + parentInstance: Instance, +): FragmentInstance { + return null; +} + +export function appendChildToFragmentInstance( + child: Instance, + fragmentInstance: FragmentInstance, +): void { + // Noop +} + +export function removeChildFromFragmentInstance( + child: Instance, + fragmentInstance: FragmentInstance, +): void { + // Noop +} + export const NotPendingTransition: TransitionStatus = null; export const HostTransitionContext: ReactContext = { $$typeof: REACT_CONTEXT_TYPE, diff --git a/packages/react-native-renderer/src/ReactFiberConfigNative.js b/packages/react-native-renderer/src/ReactFiberConfigNative.js index 5f1b5583a18d9..b7bb29b6c9ee7 100644 --- a/packages/react-native-renderer/src/ReactFiberConfigNative.js +++ b/packages/react-native-renderer/src/ReactFiberConfigNative.js @@ -202,6 +202,32 @@ export function cloneMutableTextInstance( throw new Error('Not yet implemented.'); } +export type FragmentInstance = null | { + appendChild: (child: Instance) => void, + parentInstance: Instance, + ... +}; + +export function createFragmentInstance( + parentInstance: Instance, +): FragmentInstance { + return null; +} + +export function appendChildToFragmentInstance( + child: Instance, + fragmentInstance: FragmentInstance, +): void { + // Noop +} + +export function removeChildFromFragmentInstance( + child: Instance, + fragmentInstance: FragmentInstance, +): void { + // Noop +} + export function finalizeInitialChildren( parentInstance: Instance, type: string, diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index 4978dccf2a03c..c014a6f862daf 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -512,6 +512,18 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { throw new Error('Not yet implemented.'); }, + createFragmentInstance(parentInstance) { + return null; + }, + + appendChildToFragmentInstance(child, fragmentInstance) { + // Noop + }, + + removeChildFromFragmentInstance(child, fragmentInstance) { + // Noop + }, + scheduleTimeout: setTimeout, cancelTimeout: clearTimeout, noTimeout: -1, diff --git a/packages/react-reconciler/src/ReactChildFiber.js b/packages/react-reconciler/src/ReactChildFiber.js index 9ced8912b977b..b40ebb60405b4 100644 --- a/packages/react-reconciler/src/ReactChildFiber.js +++ b/packages/react-reconciler/src/ReactChildFiber.js @@ -47,6 +47,7 @@ import isArray from 'shared/isArray'; import { enableAsyncIterableChildren, disableLegacyMode, + enableFragmentRefs, } from 'shared/ReactFeatureFlags'; import { @@ -214,10 +215,14 @@ function validateFragmentProps( const keys = Object.keys(element.props); for (let i = 0; i < keys.length; i++) { const key = keys[i]; - if (key !== 'children' && key !== 'key') { + if ( + key !== 'children' && + key !== 'key' && + (enableFragmentRefs ? key !== 'ref' : true) + ) { if (fiber === null) { - // For unkeyed root fragments there's no Fiber. We create a fake one just for - // error stack handling. + // For unkeyed root fragments without refs (enableFragmentRefs), + // there's no Fiber. We create a fake one just for error stack handling. fiber = createFiberFromElement(element, returnFiber.mode, 0); if (__DEV__) { fiber._debugInfo = currentDebugInfo; @@ -227,11 +232,19 @@ function validateFragmentProps( runWithFiberInDEV( fiber, erroredKey => { - console.error( - 'Invalid prop `%s` supplied to `React.Fragment`. ' + - 'React.Fragment can only have `key` and `children` props.', - erroredKey, - ); + if (enableFragmentRefs) { + console.error( + 'Invalid prop `%s` supplied to `React.Fragment`. ' + + 'React.Fragment can only have `key`, `ref`, and `children` props.', + erroredKey, + ); + } else { + console.error( + 'Invalid prop `%s` supplied to `React.Fragment`. ' + + 'React.Fragment can only have `key` and `children` props.', + erroredKey, + ); + } }, key, ); @@ -517,6 +530,9 @@ function createChildReconciler( lanes, element.key, ); + if (enableFragmentRefs) { + coerceRef(updated, element); + } validateFragmentProps(element, updated, returnFiber); return updated; } @@ -601,6 +617,9 @@ function createChildReconciler( returnFiber.mode, lanes, key, + enableFragmentRefs && + returnFiber.stateNode !== null && + returnFiber.stateNode.ref !== undefined, ); created.return = returnFiber; if (__DEV__) { @@ -706,6 +725,7 @@ function createChildReconciler( returnFiber.mode, lanes, null, + false, ); created.return = returnFiber; if (__DEV__) { @@ -1619,6 +1639,9 @@ function createChildReconciler( if (child.tag === Fragment) { deleteRemainingChildren(returnFiber, child.sibling); const existing = useFiber(child, element.props.children); + if (enableFragmentRefs) { + coerceRef(existing, element); + } existing.return = returnFiber; if (__DEV__) { existing._debugOwner = element._owner; @@ -1669,7 +1692,11 @@ function createChildReconciler( returnFiber.mode, lanes, element.key, + enableFragmentRefs && element.props.ref !== undefined, ); + if (enableFragmentRefs) { + coerceRef(created, element); + } created.return = returnFiber; if (__DEV__) { // We treat the parent as the owner for stack purposes. @@ -1742,17 +1769,19 @@ function createChildReconciler( // not as a fragment. Nested arrays on the other hand will be treated as // fragment nodes. Recursion happens at the normal flow. - // Handle top level unkeyed fragments as if they were arrays. - // This leads to an ambiguity between <>{[...]} and <>.... + // Handle top level unkeyed fragments without refs (enableFragmentRefs) + // as if they were arrays. This leads to an ambiguity between <>{[...]} and <>.... // We treat the ambiguous cases above the same. // We don't use recursion here because a fragment inside a fragment // is no longer considered "top level" for these purposes. - const isUnkeyedTopLevelFragment = + const isUnkeyedUnrefedTopLevelFragment = typeof newChild === 'object' && newChild !== null && newChild.type === REACT_FRAGMENT_TYPE && - newChild.key === null; - if (isUnkeyedTopLevelFragment) { + newChild.key === null && + (enableFragmentRefs ? newChild.props.ref === undefined : true); + + if (isUnkeyedUnrefedTopLevelFragment) { validateFragmentProps(newChild, null, returnFiber); newChild = newChild.props.children; } diff --git a/packages/react-reconciler/src/ReactFiber.js b/packages/react-reconciler/src/ReactFiber.js index 70cb3ed9de023..70af89f7e21d5 100644 --- a/packages/react-reconciler/src/ReactFiber.js +++ b/packages/react-reconciler/src/ReactFiber.js @@ -24,6 +24,7 @@ import type { ViewTransitionState, } from './ReactFiberViewTransitionComponent'; import type {TracingMarkerInstance} from './ReactFiberTracingMarkerComponent'; +import type {FragmentState} from './ReactFiberFragmentComponent'; import { supportsResources, @@ -41,6 +42,7 @@ import { disableLegacyMode, enableObjectFiber, enableViewTransition, + enableFragmentRefs, } from 'shared/ReactFeatureFlags'; import {NoFlags, Placement, StaticMask} from './ReactFiberFlags'; import {ConcurrentRoot} from './ReactRootTags'; @@ -589,7 +591,13 @@ export function createFiberFromTypeAndProps( } else { getTag: switch (type) { case REACT_FRAGMENT_TYPE: - return createFiberFromFragment(pendingProps.children, mode, lanes, key); + return createFiberFromFragment( + pendingProps.children, + mode, + lanes, + key, + enableFragmentRefs && pendingProps.ref !== undefined, + ); case REACT_STRICT_MODE_TYPE: fiberTag = Mode; mode |= StrictLegacyMode; @@ -770,8 +778,15 @@ export function createFiberFromFragment( mode: TypeOfMode, lanes: Lanes, key: null | string, + hasRef: boolean, ): Fiber { const fiber = createFiber(Fragment, elements, key, mode); + if (enableFragmentRefs && hasRef) { + const instance: FragmentState = { + ref: null, + }; + fiber.stateNode = instance; + } fiber.lanes = lanes; return fiber; } diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 9c88a5de870cf..9b817b11590ba 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -116,6 +116,7 @@ import { disableDefaultPropsExceptForClasses, enableHydrationLaneScheduling, enableViewTransition, + enableFragmentRefs, } from 'shared/ReactFeatureFlags'; import isArray from 'shared/isArray'; import shallowEqual from 'shared/shallowEqual'; @@ -987,6 +988,9 @@ function updateFragment( renderLanes: Lanes, ) { const nextChildren = workInProgress.pendingProps; + if (enableFragmentRefs) { + markRef(current, workInProgress); + } reconcileChildren(current, workInProgress, nextChildren, renderLanes); return workInProgress.child; } @@ -2355,6 +2359,7 @@ function mountSuspenseFallbackChildren( mode, renderLanes, null, + false, ); } else { primaryChildFragment = mountWorkInProgressOffscreenFiber( @@ -2367,6 +2372,7 @@ function mountSuspenseFallbackChildren( mode, renderLanes, null, + false, ); } @@ -2509,6 +2515,7 @@ function updateSuspenseFallbackChildren( mode, renderLanes, null, + false, ); // Needs a placement effect because the parent (the Suspense boundary) already // mounted but this is a new fiber. @@ -2573,6 +2580,7 @@ function mountSuspenseFallbackAfterRetryWithoutHydrating( fiberMode, renderLanes, null, + false, ); // Needs a placement effect because the parent (the Suspense // boundary) already mounted but this is a new fiber. diff --git a/packages/react-reconciler/src/ReactFiberCommitEffects.js b/packages/react-reconciler/src/ReactFiberCommitEffects.js index 05cf17a34273e..69e7f823c2f1f 100644 --- a/packages/react-reconciler/src/ReactFiberCommitEffects.js +++ b/packages/react-reconciler/src/ReactFiberCommitEffects.js @@ -24,11 +24,14 @@ import { enableSchedulingProfiler, enableUseEffectCRUDOverload, enableViewTransition, + enableFragmentRefs, } from 'shared/ReactFeatureFlags'; import { ClassComponent, + Fragment, HostComponent, HostHoistable, + HostRoot, HostSingleton, ViewTransitionComponent, } from './ReactWorkTags'; @@ -48,6 +51,7 @@ import { import { getPublicInstance, createViewTransitionInstance, + createFragmentInstance, } from './ReactFiberConfig'; import { captureCommitPhaseError, @@ -84,6 +88,11 @@ import { ResourceEffectIdentityKind, ResourceEffectUpdateKind, } from './ReactFiberHooks'; +import { + appendHostChildrenToFragmentInstance, + getHostParentFiber, +} from './ReactFiberCommitHostEffects'; +import type {FragmentState} from './ReactFiberFragmentComponent'; function shouldProfile(current: Fiber): boolean { return ( @@ -877,7 +886,7 @@ function commitAttachRef(finishedWork: Fiber) { case HostComponent: instanceToUse = getPublicInstance(finishedWork.stateNode); break; - case ViewTransitionComponent: + case ViewTransitionComponent: { if (enableViewTransition) { const instance: ViewTransitionState = finishedWork.stateNode; const props: ViewTransitionProps = finishedWork.memoizedProps; @@ -888,6 +897,30 @@ function commitAttachRef(finishedWork: Fiber) { instanceToUse = instance.ref; break; } + instanceToUse = finishedWork.stateNode; + break; + } + case Fragment: + if (enableFragmentRefs) { + const instance: FragmentState = finishedWork.stateNode; + const hostParentFiber = getHostParentFiber(finishedWork); + const hostParentInstance = + hostParentFiber.tag === HostRoot + ? hostParentFiber.stateNode.containerInfo + : hostParentFiber.stateNode; + if ( + instance.ref === null || + instance.ref.parentInstance !== hostParentInstance + ) { + instance.ref = createFragmentInstance(hostParentInstance); + appendHostChildrenToFragmentInstance( + finishedWork.child, + instance.ref, + ); + } + instanceToUse = instance.ref; + break; + } // Fallthrough default: instanceToUse = finishedWork.stateNode; diff --git a/packages/react-reconciler/src/ReactFiberCommitHostEffects.js b/packages/react-reconciler/src/ReactFiberCommitHostEffects.js index c104c2a8464b5..1ffe8cdd929d7 100644 --- a/packages/react-reconciler/src/ReactFiberCommitHostEffects.js +++ b/packages/react-reconciler/src/ReactFiberCommitHostEffects.js @@ -24,6 +24,7 @@ import { HostText, HostPortal, DehydratedFragment, + Fragment, } from './ReactWorkTags'; import {ContentReset, Placement} from './ReactFiberFlags'; import { @@ -50,11 +51,14 @@ import { acquireSingletonInstance, releaseSingletonInstance, isSingletonScope, + appendChildToFragmentInstance, } from './ReactFiberConfig'; import {captureCommitPhaseError} from './ReactFiberWorkLoop'; import {trackHostMutation} from './ReactFiberMutationTracking'; import {runWithFiberInDEV} from './ReactCurrentFiber'; +import {enableFragmentRefs} from 'shared/ReactFeatureFlags'; +import type {FragmentInstance} from './ReactFiberFragmentComponent'; export function commitHostMount(finishedWork: Fiber) { const type = finishedWork.type; @@ -199,7 +203,7 @@ export function commitShowHideHostTextInstance(node: Fiber, isHidden: boolean) { } } -function getHostParentFiber(fiber: Fiber): Fiber { +export function getHostParentFiber(fiber: Fiber): Fiber { let parent = fiber.return; while (parent !== null) { if (isHostParent(parent)) { @@ -214,6 +218,25 @@ function getHostParentFiber(fiber: Fiber): Fiber { ); } +export function getFragmentInstanceParent( + fiber: Fiber, +): null | FragmentInstance { + let parent = fiber.return; + while (parent !== null) { + if (isFragmentInstanceParent(parent)) { + return parent.stateNode.ref; + } + + if (isHostParent(parent)) { + return null; + } + + parent = parent.return; + } + + return null; +} + function isHostParent(fiber: Fiber): boolean { return ( fiber.tag === HostComponent || @@ -226,6 +249,39 @@ function isHostParent(fiber: Fiber): boolean { ); } +function isFragmentInstanceParent(fiber: Fiber): boolean { + return ( + fiber && + fiber.tag === Fragment && + fiber.stateNode !== null && + fiber.stateNode.ref !== null + ); +} + +export function appendHostChildrenToFragmentInstance( + child: Fiber | null, + instance: FragmentInstance, +): void { + if (child === null) { + return; + } + + if (instance === null) { + return; + } + + if (child.sibling !== null) { + appendHostChildrenToFragmentInstance(child.sibling, instance); + } + + if (child.tag === HostComponent) { + instance.appendChild(child.stateNode); + return; + } + + appendHostChildrenToFragmentInstance(child.child, instance); +} + 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 @@ -299,6 +355,12 @@ function insertOrAppendPlacementNodeIntoContainer( appendChildToContainer(parent, stateNode); } trackHostMutation(); + if (enableFragmentRefs && tag === HostComponent) { + const parentFragmentInstance = getFragmentInstanceParent(node); + if (parentFragmentInstance !== null) { + appendChildToFragmentInstance(node.stateNode, parentFragmentInstance); + } + } return; } else if (tag === HostPortal) { // If the insertion itself is a portal, then we don't want to traverse @@ -343,6 +405,12 @@ function insertOrAppendPlacementNode( appendChild(parent, stateNode); } trackHostMutation(); + if (enableFragmentRefs && tag === HostComponent) { + const parentFragmentInstance = getFragmentInstanceParent(node); + if (parentFragmentInstance !== null) { + appendChildToFragmentInstance(node.stateNode, parentFragmentInstance); + } + } return; } else if (tag === HostPortal) { // If the insertion itself is a portal, then we don't want to traverse diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index b9ea97bf56d8c..4eaa033e5aef7 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -61,6 +61,7 @@ import { disableLegacyMode, enableComponentPerformanceTrack, enableViewTransition, + enableFragmentRefs, } from 'shared/ReactFeatureFlags'; import { FunctionComponent, @@ -85,6 +86,7 @@ import { CacheComponent, TracingMarkerComponent, ViewTransitionComponent, + Fragment, } from './ReactWorkTags'; import { NoFlags, @@ -163,7 +165,9 @@ import { cancelViewTransitionName, cancelRootViewTransitionName, restoreRootViewTransitionName, + getPublicInstance, isSingletonScope, + removeChildFromFragmentInstance, } from './ReactFiberConfig'; import { captureCommitPhaseError, @@ -235,6 +239,7 @@ import { commitHostRemoveChild, commitHostSingletonAcquisition, commitHostSingletonRelease, + getFragmentInstanceParent, } from './ReactFiberCommitHostEffects'; import { commitEnterViewTransitions, @@ -769,6 +774,13 @@ function commitLayoutEffectOnFiber( } // Fallthrough } + case Fragment: + if (enableFragmentRefs) { + if (flags & Ref) { + safelyAttachRef(finishedWork, finishedWork.return); + } + } + // Fallthrough default: { recursivelyTraverseLayoutEffects( finishedRoot, @@ -1353,6 +1365,16 @@ function commitDeletionEffectsOnFiber( if (!offscreenSubtreeWasHidden) { safelyDetachRef(deletedFiber, nearestMountedAncestor); } + + if (enableFragmentRefs) { + const parentFragmentInstance = getFragmentInstanceParent(deletedFiber); + if (parentFragmentInstance !== null) { + removeChildFromFragmentInstance( + getPublicInstance(deletedFiber.stateNode), + parentFragmentInstance, + ); + } + } // Intentional fallthrough to next branch } case HostText: { @@ -2270,7 +2292,7 @@ function commitMutationEffectsOnFiber( } break; } - case ViewTransitionComponent: + case ViewTransitionComponent: { if (enableViewTransition) { if (flags & Ref) { if (!offscreenSubtreeWasHidden && current !== null) { @@ -2298,7 +2320,8 @@ function commitMutationEffectsOnFiber( popMutationContext(prevMutationContext); break; } - // Fallthrough + break; + } case ScopeComponent: { if (enableScopeAPI) { recursivelyTraverseMutationEffects(root, finishedWork, lanes); @@ -2321,6 +2344,13 @@ function commitMutationEffectsOnFiber( } break; } + case Fragment: + if (enableFragmentRefs) { + if (flags & Ref) { + safelyAttachRef(finishedWork, finishedWork.return); + } + } + // Fallthrough default: { recursivelyTraverseMutationEffects(root, finishedWork, lanes); commitReconciliationEffects(finishedWork, lanes); diff --git a/packages/react-reconciler/src/ReactFiberFragmentComponent.js b/packages/react-reconciler/src/ReactFiberFragmentComponent.js new file mode 100644 index 0000000000000..cfbb8500be523 --- /dev/null +++ b/packages/react-reconciler/src/ReactFiberFragmentComponent.js @@ -0,0 +1,16 @@ +/** + * 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 {FragmentInstance as _FragmentInstance} from './ReactFiberConfig'; + +export type FragmentInstance = _FragmentInstance; + +export type FragmentState = { + ref: null | FragmentInstance, +}; diff --git a/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js b/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js index e978249e185fe..008b1843e1c56 100644 --- a/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js +++ b/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js @@ -45,6 +45,11 @@ export type ViewTransitionInstance = null | {name: string, ...}; export opaque type InstanceMeasurement = mixed; export type EventResponder = any; export type GestureTimeline = any; +export type FragmentInstance = null | { + appendChild: (child: Instance) => void, + parentInstance: Instance, + ... +}; export const rendererVersion = $$$config.rendererVersion; export const rendererPackageName = $$$config.rendererPackageName; @@ -160,6 +165,11 @@ export const subscribeToGestureDirection = export const createViewTransitionInstance = $$$config.createViewTransitionInstance; export const clearContainer = $$$config.clearContainer; +export const createFragmentInstance = $$$config.createFragmentInstance; +export const appendChildToFragmentInstance = + $$$config.appendChildToFragmentInstance; +export const removeChildFromFragmentInstance = + $$$config.removeChildFromFragmentInstance; // ------------------- // Persistence diff --git a/packages/react-test-renderer/src/ReactFiberConfigTestHost.js b/packages/react-test-renderer/src/ReactFiberConfigTestHost.js index bb18e44e5580d..d46ad3036f5d5 100644 --- a/packages/react-test-renderer/src/ReactFiberConfigTestHost.js +++ b/packages/react-test-renderer/src/ReactFiberConfigTestHost.js @@ -447,6 +447,32 @@ export function createViewTransitionInstance( return null; } +export type FragmentInstance = null | { + appendChild: (child: Instance) => void, + parentInstance: Instance, + ... +}; + +export function createFragmentInstance( + parentInstance: Instance | Container, +): FragmentInstance { + return null; +} + +export function appendChildToFragmentInstance( + child: Instance, + fragmentInstance: FragmentInstance, +): void { + // noop +} + +export function removeChildFromFragmentInstance( + child: Instance, + fragmentInstance: FragmentInstance, +): void { + // Noop +} + export function getInstanceFromNode(mockNode: Object): Object | null { const instance = nodeToInstanceMap.get(mockNode); if (instance !== undefined) { diff --git a/packages/react/src/__tests__/ReactElementValidator-test.internal.js b/packages/react/src/__tests__/ReactElementValidator-test.internal.js index bbb06411b03e2..f1ee7026346ec 100644 --- a/packages/react/src/__tests__/ReactElementValidator-test.internal.js +++ b/packages/react/src/__tests__/ReactElementValidator-test.internal.js @@ -427,9 +427,13 @@ describe('ReactElementValidator', () => { const root = ReactDOMClient.createRoot(document.createElement('div')); await act(() => root.render(React.createElement(Foo))); assertConsoleErrorDev([ - 'Invalid prop `a` supplied to `React.Fragment`. React.Fragment ' + - 'can only have `key` and `children` props.\n' + - ' in Foo (at **)', + gate('enableFragmentRefs') + ? 'Invalid prop `a` supplied to `React.Fragment`. React.Fragment ' + + 'can only have `key`, `ref`, and `children` props.\n' + + ' in Foo (at **)' + : 'Invalid prop `a` supplied to `React.Fragment`. React.Fragment ' + + 'can only have `key` and `children` props.\n' + + ' in Foo (at **)', ]); }); diff --git a/packages/react/src/__tests__/ReactJSXElementValidator-test.js b/packages/react/src/__tests__/ReactJSXElementValidator-test.js index 041191e2ab5fd..41ee720478ae1 100644 --- a/packages/react/src/__tests__/ReactJSXElementValidator-test.js +++ b/packages/react/src/__tests__/ReactJSXElementValidator-test.js @@ -221,9 +221,13 @@ describe('ReactJSXElementValidator', () => { root.render(); }); assertConsoleErrorDev([ - 'Invalid prop `a` supplied to `React.Fragment`. React.Fragment ' + - 'can only have `key` and `children` props.\n' + - ' in Foo (at **)', + gate('enableFragmentRefs') + ? 'Invalid prop `a` supplied to `React.Fragment`. React.Fragment ' + + 'can only have `key`, `ref`, and `children` props.\n' + + ' in Foo (at **)' + : 'Invalid prop `a` supplied to `React.Fragment`. React.Fragment ' + + 'can only have `key` and `children` props.\n' + + ' in Foo (at **)', ]); }); @@ -246,11 +250,15 @@ describe('ReactJSXElementValidator', () => { await act(() => { root.render(); }); - assertConsoleErrorDev([ - 'Invalid prop `ref` supplied to `React.Fragment`.' + - ' React.Fragment can only have `key` and `children` props.\n' + - ' in Foo (at **)', - ]); + assertConsoleErrorDev( + gate('enableFragmentRefs') + ? [] + : [ + 'Invalid prop `ref` supplied to `React.Fragment`.' + + ' React.Fragment can only have `key` and `children` props.\n' + + ' in Foo (at **)', + ], + ); }); it('does not warn for fragments of multiple elements without keys', async () => { diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index bb087a899b22d..d50965f13e949 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -138,7 +138,7 @@ export const enablePersistedModeClonedFlag = false; export const enableShallowPropDiffing = false; -export const enableSiblingPrerendering = false; +export const enableSiblingPrerendering = true; /** * Enables an expiration time for retry lanes to avoid starvation. diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js index 4df92a5c90ef8..f2fa119800fa8 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js @@ -71,6 +71,7 @@ export const enableSwipeTransition = false; export const enableFastAddPropertiesInDiffing = false; export const enableLazyPublicInstanceInFabric = false; export const enableScrollEndPolyfill = true; +export const enableFragmentRefs = false; // Flow magic to verify the exports of this file match the original version. ((((null: any): ExportsType): FeatureFlagsType): ExportsType); diff --git a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js index 3fe3b7a0a3e29..0116e160b643e 100644 --- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js @@ -41,6 +41,7 @@ export const enableLazyPublicInstanceInFabric = false; export const enableViewTransition = __VARIANT__; export const enableComponentPerformanceTrack = __VARIANT__; export const enableScrollEndPolyfill = __VARIANT__; +export const enableFragmentRefs = __VARIANT__; // TODO: These flags are hard-coded to the default values used in open source. // Update the tests so that they pass in either mode, then set these diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index a8b0b4e100b18..0584af1f81826 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -39,6 +39,7 @@ export const { enableViewTransition, enableComponentPerformanceTrack, enableScrollEndPolyfill, + enableFragmentRefs, } = dynamicFeatureFlags; // On WWW, __EXPERIMENTAL__ is used for a new modern build. @@ -113,7 +114,5 @@ export const enableLazyPublicInstanceInFabric = false; export const enableSwipeTransition = false; -export const enableFragmentRefs = false; - // Flow magic to verify the exports of this file match the original version. ((((null: any): ExportsType): FeatureFlagsType): ExportsType); From d3052037b2af6428331df146264bc32864efd49c Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Thu, 20 Feb 2025 11:59:27 -0500 Subject: [PATCH 03/21] Remove public append/remove child APIs --- .../src/client/ReactFiberConfigDOM.js | 38 ++++++------------- .../src/ReactFiberCommitHostEffects.js | 2 +- 2 files changed, 13 insertions(+), 27 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index 9a2cc5f1b7b67..13048e77ccc35 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -2096,8 +2096,6 @@ export type FragmentInstance = { _children: Set, _eventListeners: Array, parentInstance: Instance | Container, - appendChild(child: Element): void, - removeChild(child: Element): void, addEventListener( type: string, listener: EventListener, @@ -2127,28 +2125,6 @@ function FragmentInstancePseudoElement( this._eventListeners = []; } // $FlowFixMe[prop-missing] -FragmentInstancePseudoElement.prototype.appendChild = function ( - this: FragmentInstance, - child: Element, -): void { - this._children.add(child); - for (let i = 0; i < this._eventListeners.length; i++) { - const {type, listener, optionsOrUseCapture} = this._eventListeners[i]; - child.addEventListener(type, listener, optionsOrUseCapture); - } -}; -// $FlowFixMe[prop-missing] -FragmentInstancePseudoElement.prototype.removeChild = function ( - this: FragmentInstance, - child: Element, -): void { - this._children.delete(child); - for (let i = 0; i < this._eventListeners.length; i++) { - const {type, listener, optionsOrUseCapture} = this._eventListeners[i]; - child.removeEventListener(type, listener, optionsOrUseCapture); - } -}; -// $FlowFixMe[prop-missing] FragmentInstancePseudoElement.prototype.addEventListener = function ( this: FragmentInstance, type: string, @@ -2231,14 +2207,24 @@ export function appendChildToFragmentInstance( childElement: Element, fragmentInstance: FragmentInstance, ): void { - fragmentInstance.appendChild(childElement); + fragmentInstance._children.add(childElement); + for (let i = 0; i < fragmentInstance._eventListeners.length; i++) { + const {type, listener, optionsOrUseCapture} = + fragmentInstance._eventListeners[i]; + childElement.addEventListener(type, listener, optionsOrUseCapture); + } } export function removeChildFromFragmentInstance( childElement: Element, fragmentInstance: FragmentInstance, ): void { - fragmentInstance.removeChild(childElement); + fragmentInstance._children.delete(childElement); + for (let i = 0; i < fragmentInstance._eventListeners.length; i++) { + const {type, listener, optionsOrUseCapture} = + fragmentInstance._eventListeners[i]; + childElement.removeEventListener(type, listener, optionsOrUseCapture); + } } export function clearContainer(container: Container): void { diff --git a/packages/react-reconciler/src/ReactFiberCommitHostEffects.js b/packages/react-reconciler/src/ReactFiberCommitHostEffects.js index 1ffe8cdd929d7..5adc847ce6317 100644 --- a/packages/react-reconciler/src/ReactFiberCommitHostEffects.js +++ b/packages/react-reconciler/src/ReactFiberCommitHostEffects.js @@ -275,7 +275,7 @@ export function appendHostChildrenToFragmentInstance( } if (child.tag === HostComponent) { - instance.appendChild(child.stateNode); + appendChildToFragmentInstance(child.stateNode, instance); return; } From 4ff4ec3cc67bf8fd9f8b6ed0f34d98e7a89938cf Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Thu, 20 Feb 2025 14:46:27 -0500 Subject: [PATCH 04/21] Handle children with multiple fragment parents --- .../__tests__/ReactDOMFragmentRefs-test.js | 34 +++++++++++++++++++ .../src/ReactFiberCommitHostEffects.js | 34 ++++++++++++------- .../src/ReactFiberCommitWork.js | 18 ++++++---- 3 files changed, 67 insertions(+), 19 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js index b592a06032297..139e95084898b 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js @@ -115,6 +115,40 @@ describe('FragmentRefs', () => { expect(fragmentRef.current._children.size).toBe(5); }); + // @gate enableFragmentRefs + it('can share tracked children with nested fragment instances', async () => { + const fragmentRefA = React.createRef(); + const fragmentRefB = React.createRef(); + const root = ReactDOMClient.createRoot(container); + + function Test({showB}) { + return ( + +
+ +
+ {showB &&
} + + + ); + } + + await act(() => root.render()); + + expect(fragmentRefA.current._children.size).toBe(2); + expect(fragmentRefB.current._children.size).toBe(1); + + await act(() => root.render()); + + expect(fragmentRefA.current._children.size).toBe(3); + expect(fragmentRefB.current._children.size).toBe(2); + + await act(() => root.render()); + + expect(fragmentRefA.current._children.size).toBe(2); + expect(fragmentRefB.current._children.size).toBe(1); + }); + // @gate enableFragmentRefs it('handles empty children', async () => { const fragmentRef = React.createRef(); diff --git a/packages/react-reconciler/src/ReactFiberCommitHostEffects.js b/packages/react-reconciler/src/ReactFiberCommitHostEffects.js index 5adc847ce6317..934d5e3291917 100644 --- a/packages/react-reconciler/src/ReactFiberCommitHostEffects.js +++ b/packages/react-reconciler/src/ReactFiberCommitHostEffects.js @@ -218,23 +218,27 @@ export function getHostParentFiber(fiber: Fiber): Fiber { ); } -export function getFragmentInstanceParent( +export function getFragmentInstanceParents( fiber: Fiber, -): null | FragmentInstance { +): null | Array { let parent = fiber.return; + let fragmentParents: null | Array = null; while (parent !== null) { if (isFragmentInstanceParent(parent)) { - return parent.stateNode.ref; + if (fragmentParents === null) { + fragmentParents = []; + } + const fragmentInstance: FragmentInstance = parent.stateNode.ref; + fragmentParents.push(fragmentInstance); } if (isHostParent(parent)) { - return null; + return fragmentParents; } parent = parent.return; } - - return null; + return fragmentParents; } function isHostParent(fiber: Fiber): boolean { @@ -356,9 +360,12 @@ function insertOrAppendPlacementNodeIntoContainer( } trackHostMutation(); if (enableFragmentRefs && tag === HostComponent) { - const parentFragmentInstance = getFragmentInstanceParent(node); - if (parentFragmentInstance !== null) { - appendChildToFragmentInstance(node.stateNode, parentFragmentInstance); + const parentFragmentInstances = getFragmentInstanceParents(node); + if (parentFragmentInstances !== null) { + for (let i = 0; i < parentFragmentInstances.length; i++) { + const instance = parentFragmentInstances[i]; + appendChildToFragmentInstance(node.stateNode, instance); + } } } return; @@ -406,9 +413,12 @@ function insertOrAppendPlacementNode( } trackHostMutation(); if (enableFragmentRefs && tag === HostComponent) { - const parentFragmentInstance = getFragmentInstanceParent(node); - if (parentFragmentInstance !== null) { - appendChildToFragmentInstance(node.stateNode, parentFragmentInstance); + const parentFragmentInstances = getFragmentInstanceParents(node); + if (parentFragmentInstances !== null) { + for (let i = 0; i < parentFragmentInstances.length; i++) { + const instance = parentFragmentInstances[i]; + appendChildToFragmentInstance(node.stateNode, instance); + } } } return; diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 4eaa033e5aef7..6602d127a698d 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -239,7 +239,7 @@ import { commitHostRemoveChild, commitHostSingletonAcquisition, commitHostSingletonRelease, - getFragmentInstanceParent, + getFragmentInstanceParents, } from './ReactFiberCommitHostEffects'; import { commitEnterViewTransitions, @@ -1367,12 +1367,16 @@ function commitDeletionEffectsOnFiber( } if (enableFragmentRefs) { - const parentFragmentInstance = getFragmentInstanceParent(deletedFiber); - if (parentFragmentInstance !== null) { - removeChildFromFragmentInstance( - getPublicInstance(deletedFiber.stateNode), - parentFragmentInstance, - ); + const parentFragmentInstances = + getFragmentInstanceParents(deletedFiber); + if (parentFragmentInstances !== null) { + const element = getPublicInstance(deletedFiber.stateNode); + for (let i = 0; i < parentFragmentInstances.length; i++) { + removeChildFromFragmentInstance( + element, + parentFragmentInstances[i], + ); + } } } // Intentional fallthrough to next branch From 3b0e2b27f79db19baf88f532e063e496482e4599 Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Fri, 21 Feb 2025 14:10:46 -0500 Subject: [PATCH 05/21] Add basic order preservation test --- .../src/client/ReactFiberConfigDOM.js | 11 ++++ .../__tests__/ReactDOMFragmentRefs-test.js | 50 +++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index 13048e77ccc35..32a826c888d37 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -2106,6 +2106,7 @@ export type FragmentInstance = { listener: EventListener, optionsOrUseCapture?: EventListenerOptionsOrUseCapture, ): void, + focus(): void, }; function FragmentInstancePseudoElement( @@ -2164,6 +2165,16 @@ FragmentInstancePseudoElement.prototype.removeEventListener = function ( }); this._eventListeners.splice(index, 1); }; +// $FlowFixMe[prop-missing] +FragmentInstancePseudoElement.prototype.focus = function ( + this: FragmentInstance, +) { + this._children.forEach(child => { + if (setFocusIfFocusable(child)) { + return; + } + }); +}; function normalizeListenerOptions( opts: ?EventListenerOptionsOrUseCapture, diff --git a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js index 139e95084898b..c827c6c86dba8 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js @@ -198,6 +198,56 @@ describe('FragmentRefs', () => { await act(() => root.render()); }); + describe('focus()', () => { + // @gate enableFragmentRefs + it.skip('preserves document order', async () => { + const fragmentRef = React.createRef(); + const root = ReactDOMClient.createRoot(container); + let focusedElement = null; + + function Test({showA, showB}) { + return ( + + {showA && } + {showB && } + + ); + } + + const focusMock = jest.spyOn(HTMLElement.prototype, 'focus'); + focusMock.mockImplementation(function () { + focusedElement = this.id; + }); + + // Render with A as the first focusable child + await act(() => { + root.render(); + }); + await act(() => { + fragmentRef.current.focus(); + }); + expect(focusedElement).toEqual('child-a'); + + // A is still the first focusable child, but B is also tracked + await act(() => { + root.render(); + }); + await act(() => { + fragmentRef.current.focus(); + }); + expect(focusedElement).toEqual('child-a'); + + // B is now the first focusable child + await act(() => { + root.render(); + }); + await act(() => { + fragmentRef.current.focus(); + }); + expect(focusedElement).toEqual('child-b'); + }); + }); + describe('event listeners', () => { // @gate enableFragmentRefs it('adds and removes event listeners from children', async () => { From 76a1058deab3970eb8ec1fc0b96ba535c37febc5 Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Fri, 21 Feb 2025 17:28:03 -0500 Subject: [PATCH 06/21] Dont store children on fragment instance, calculate at time of operation --- packages/react-art/src/ReactFiberConfigART.js | 10 +- .../src/client/ReactFiberConfigDOM.js | 123 ++++++----- .../__tests__/ReactDOMFragmentRefs-test.js | 192 +++++++----------- .../src/ReactFiberConfigFabric.js | 15 +- .../src/ReactFiberConfigNative.js | 15 +- .../src/createReactNoop.js | 4 +- .../src/ReactFiberCommitEffects.js | 18 +- .../src/ReactFiberCommitHostEffects.js | 44 ---- .../src/ReactFiberCommitWork.js | 26 ++- .../src/forks/ReactFiberConfig.custom.js | 14 +- .../src/ReactFiberConfigTestHost.js | 7 +- 11 files changed, 195 insertions(+), 273 deletions(-) diff --git a/packages/react-art/src/ReactFiberConfigART.js b/packages/react-art/src/ReactFiberConfigART.js index 048b5c3e5adfc..0a9e406bfc565 100644 --- a/packages/react-art/src/ReactFiberConfigART.js +++ b/packages/react-art/src/ReactFiberConfigART.js @@ -319,8 +319,7 @@ export function cloneMutableTextInstance(textInstance) { } export type FragmentInstance = null | { - appendChild: (child: any) => void, - parentInstance: any, + _fragmentFiber: any, ... }; @@ -328,11 +327,14 @@ export function createFragmentInstance(parentInstance): null { return null; } -export function appendChildToFragmentInstance(child, fragmentInstance): void { +export function commitNewChildToFragmentInstance( + child, + fragmentInstance, +): void { // Noop } -export function removeChildFromFragmentInstance(child, fragmentInstance): void { +export function deleteChildFromFragmentInstance(child, fragmentInstance): void { // Noop } diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index 32a826c888d37..e5d7b86b1f0cb 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -2093,9 +2093,8 @@ type StoredEventListener = { }; export type FragmentInstance = { - _children: Set, - _eventListeners: Array, - parentInstance: Instance | Container, + _fragmentFiber: Fiber, + _eventListeners: void | Array, addEventListener( type: string, listener: EventListener, @@ -2111,19 +2110,9 @@ export type FragmentInstance = { function FragmentInstancePseudoElement( this: FragmentInstance, - parentInstance: Container, + fragmentFiber: Fiber, ) { - this.parentInstance = parentInstance || document.documentElement; - if (this.parentInstance === null) { - throw new Error( - 'React expected a Fragment Ref to have a parent HTMLElement or to fallback to an element ' + - '(document.documentElement). But no parent or element was found. React never removes the ' + - 'documentElement for any Document it renders into so the cause is likely in some other script ' + - 'running on this page.', - ); - } - this._children = new Set(); - this._eventListeners = []; + this._fragmentFiber = fragmentFiber; } // $FlowFixMe[prop-missing] FragmentInstancePseudoElement.prototype.addEventListener = function ( @@ -2132,6 +2121,9 @@ FragmentInstancePseudoElement.prototype.addEventListener = function ( listener: EventListener, optionsOrUseCapture?: EventListenerOptionsOrUseCapture, ): void { + if (!this._eventListeners) { + this._eventListeners = []; + } // Element.addEventListener will only apply uniquely new event listeners by default. Since we // need to collect the listeners to apply to appended children, we track them ourselves and use // custom equality check for the options. @@ -2141,9 +2133,10 @@ FragmentInstancePseudoElement.prototype.addEventListener = function ( listener, optionsOrUseCapture, }) === -1; - if (isNewEventListener) { + if (isNewEventListener && this._eventListeners !== undefined) { this._eventListeners.push({type, listener, optionsOrUseCapture}); - this._children.forEach(child => { + const children = getFragmentInstanceChildren(this); + children.forEach(child => { child.addEventListener(type, listener, optionsOrUseCapture); }); } @@ -2155,27 +2148,65 @@ FragmentInstancePseudoElement.prototype.removeEventListener = function ( listener: EventListener, optionsOrUseCapture?: EventListenerOptionsOrUseCapture, ): void { - this._children.forEach(child => { - child.removeEventListener(type, listener, optionsOrUseCapture); - }); - const index = indexOfEventListener(this._eventListeners, { - type, - listener, - optionsOrUseCapture, - }); - this._eventListeners.splice(index, 1); + if (Array.isArray(this._eventListeners) && this._eventListeners.length > 0) { + const children = getFragmentInstanceChildren(this); + children.forEach(child => { + child.removeEventListener(type, listener, optionsOrUseCapture); + }); + const index = indexOfEventListener(this._eventListeners || [], { + type, + listener, + optionsOrUseCapture, + }); + this._eventListeners = (this._eventListeners || []).splice(index, 1); + } }; // $FlowFixMe[prop-missing] FragmentInstancePseudoElement.prototype.focus = function ( this: FragmentInstance, ) { - this._children.forEach(child => { - if (setFocusIfFocusable(child)) { - return; + const children = getFragmentInstanceChildren(this); + const iterator = children.values(); + let child = iterator.next(); + while (!child.done) { + if (setFocusIfFocusable(child.value)) { + break; } - }); + child = iterator.next(); + } }; +function getFragmentInstanceChildren( + fragmentInstance: FragmentInstance, +): Set { + const children = new Set(); + recursivelyGetFragmentInstanceChildren( + fragmentInstance._fragmentFiber.child, + children, + ); + return children; +} + +function recursivelyGetFragmentInstanceChildren( + child: Fiber | null, + childElements: Set, +): void { + if (child === null) { + return; + } + + if (child.sibling !== null) { + recursivelyGetFragmentInstanceChildren(child.sibling, childElements); + } + + if (child.tag === HostComponent) { + childElements.add(child.stateNode); + return; + } + + recursivelyGetFragmentInstanceChildren(child.child, childElements); +} + function normalizeListenerOptions( opts: ?EventListenerOptionsOrUseCapture, ): string { @@ -2208,33 +2239,33 @@ function indexOfEventListener( return -1; } -export function createFragmentInstance( - parentInstance: Instance | Container, -): FragmentInstance { - return new (FragmentInstancePseudoElement: any)(parentInstance); +export function createFragmentInstance(fragmentFiber: Fiber): FragmentInstance { + return new (FragmentInstancePseudoElement: any)(fragmentFiber); } -export function appendChildToFragmentInstance( +export function commitNewChildToFragmentInstance( childElement: Element, fragmentInstance: FragmentInstance, ): void { - fragmentInstance._children.add(childElement); - for (let i = 0; i < fragmentInstance._eventListeners.length; i++) { - const {type, listener, optionsOrUseCapture} = - fragmentInstance._eventListeners[i]; - childElement.addEventListener(type, listener, optionsOrUseCapture); + const eventListeners = fragmentInstance._eventListeners; + if (eventListeners !== undefined) { + for (let i = 0; i < eventListeners.length; i++) { + const {type, listener, optionsOrUseCapture} = eventListeners[i]; + childElement.addEventListener(type, listener, optionsOrUseCapture); + } } } -export function removeChildFromFragmentInstance( +export function deleteChildFromFragmentInstance( childElement: Element, fragmentInstance: FragmentInstance, ): void { - fragmentInstance._children.delete(childElement); - for (let i = 0; i < fragmentInstance._eventListeners.length; i++) { - const {type, listener, optionsOrUseCapture} = - fragmentInstance._eventListeners[i]; - childElement.removeEventListener(type, listener, optionsOrUseCapture); + const eventListeners = fragmentInstance._eventListeners; + if (eventListeners !== undefined) { + for (let i = 0; i < eventListeners.length; i++) { + const {type, listener, optionsOrUseCapture} = eventListeners[i]; + childElement.removeEventListener(type, listener, optionsOrUseCapture); + } } } diff --git a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js index c827c6c86dba8..5a5cdc58a7efe 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js @@ -21,6 +21,11 @@ describe('FragmentRefs', () => { ReactDOMClient = require('react-dom/client'); act = require('internal-test-utils').act; container = document.createElement('div'); + document.body.appendChild(container); + }); + + afterEach(() => { + document.body.removeChild(container); }); // @gate enableFragmentRefs @@ -44,123 +49,6 @@ describe('FragmentRefs', () => { expect(fragmentRef.current).not.toBe(null); }); - // @gate enableFragmentRefs - it('tracks added and removed children', async () => { - let addChild; - let removeChild; - const fragmentRef = React.createRef(); - - function Test() { - const [extraChildCount, setExtraChildCount] = React.useState(0); - addChild = () => { - setExtraChildCount(prev => prev + 1); - }; - removeChild = () => { - setExtraChildCount(prev => prev - 1); - }; - - return ( -
- - {Array.from({length: extraChildCount}).map((_, index) => ( -
- Extra Child {index} -
- ))} -
-
- ); - } - - const root = ReactDOMClient.createRoot(container); - await act(() => root.render()); - expect(fragmentRef.current._children.size).toBe(0); - await act(() => addChild()); - expect(fragmentRef.current._children.size).toBe(1); - await act(() => removeChild()); - expect(fragmentRef.current._children.size).toBe(0); - }); - - // @gate enableFragmentRefs - it('tracks nested host children', async () => { - const fragmentRef = React.createRef(); - const root = ReactDOMClient.createRoot(container); - - function Wrapper({children}) { - return children; - } - - await act(() => { - root.render( - - -
- - -
- -
-
-
- - - -
- -
- , - ); - }); - - expect(fragmentRef.current._children.size).toBe(5); - }); - - // @gate enableFragmentRefs - it('can share tracked children with nested fragment instances', async () => { - const fragmentRefA = React.createRef(); - const fragmentRefB = React.createRef(); - const root = ReactDOMClient.createRoot(container); - - function Test({showB}) { - return ( - -
- -
- {showB &&
} - - - ); - } - - await act(() => root.render()); - - expect(fragmentRefA.current._children.size).toBe(2); - expect(fragmentRefB.current._children.size).toBe(1); - - await act(() => root.render()); - - expect(fragmentRefA.current._children.size).toBe(3); - expect(fragmentRefB.current._children.size).toBe(2); - - await act(() => root.render()); - - expect(fragmentRefA.current._children.size).toBe(2); - expect(fragmentRefB.current._children.size).toBe(1); - }); - - // @gate enableFragmentRefs - it('handles empty children', async () => { - const fragmentRef = React.createRef(); - const root = ReactDOMClient.createRoot(container); - - await act(() => { - root.render(); - }); - - expect(fragmentRef.current._children.size).toBe(0); - }); - // @gate enableFragmentRefs it('accepts a ref callback', async () => { let fragmentRef; @@ -174,7 +62,7 @@ describe('FragmentRefs', () => { ); }); - expect(fragmentRef._children.size).toBe(1); + expect(fragmentRef.current).not.toEqual(null); }); // @gate enableFragmentRefs @@ -200,7 +88,49 @@ describe('FragmentRefs', () => { describe('focus()', () => { // @gate enableFragmentRefs - it.skip('preserves document order', async () => { + it('focuses the first focusable child', async () => { + const parentRef = React.createRef(); + const fragmentRef = React.createRef(); + const root = ReactDOMClient.createRoot(container); + let focusedElement = null; + + function Test() { + return ( +
+ +
+ + A + +
+ +
+ ); + } + + await act(() => { + root.render(); + }); + + const focusableChildren = parentRef.current.querySelectorAll( + 'button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])', + ); + const focusMock = jest.spyOn(HTMLElement.prototype, 'focus'); + focusMock.mockImplementation(function () { + if (Array.from(focusableChildren).includes(this)) { + focusedElement = this.id; + } else { + return false; + } + }); + await act(() => { + fragmentRef.current.focus(); + }); + expect(focusedElement).toEqual('child-b'); + }); + + // @gate enableFragmentRefs + it('preserves document order when adding and removing children', async () => { const fragmentRef = React.createRef(); const root = ReactDOMClient.createRoot(container); let focusedElement = null; @@ -271,9 +201,6 @@ describe('FragmentRefs', () => { ); return () => { - // TODO: This is a noop here because the tracked child nodes were deleted and they - // remove their own event listeners. Decide if we want to change the timing of this. - // instance._children is currently empty at this point. fragmentRef.current.removeEventListener( 'click', handleFragmentRefClicks, @@ -332,8 +259,10 @@ describe('FragmentRefs', () => { it('adds and removes event listeners from children with multiple fragments', async () => { const fragmentRef = React.createRef(); const nestedFragmentRef = React.createRef(); + const nestedFragmentRef2 = React.createRef(); const childARef = React.createRef(); const childBRef = React.createRef(); + const childCRef = React.createRef(); const root = ReactDOMClient.createRoot(container); await act(() => { @@ -346,6 +275,9 @@ describe('FragmentRefs', () => {
B
+ +
C
+
, ); @@ -361,11 +293,19 @@ describe('FragmentRefs', () => { logs.push('nestedFragmentRef'); } + function handleNestedFragmentRef2Clicks() { + logs.push('nestedFragmentRef2'); + } + fragmentRef.current.addEventListener('click', handleFragmentRefClicks); nestedFragmentRef.current.addEventListener( 'click', handleNestedFragmentRefClicks, ); + nestedFragmentRef2.current.addEventListener( + 'click', + handleNestedFragmentRef2Clicks, + ); childBRef.current.click(); // Event bubbles to the parent fragment @@ -376,6 +316,10 @@ describe('FragmentRefs', () => { childARef.current.click(); expect(logs).toEqual(['fragmentRef']); + logs = []; + childCRef.current.click(); + expect(logs).toEqual(['fragmentRef', 'nestedFragmentRef2']); + logs = []; fragmentRef.current.removeEventListener('click', handleFragmentRefClicks); @@ -383,8 +327,8 @@ describe('FragmentRefs', () => { 'click', handleNestedFragmentRefClicks, ); - childBRef.current.click(); - expect(logs).toEqual([]); + childCRef.current.click(); + expect(logs).toEqual(['nestedFragmentRef2']); }); // @gate enableFragmentRefs diff --git a/packages/react-native-renderer/src/ReactFiberConfigFabric.js b/packages/react-native-renderer/src/ReactFiberConfigFabric.js index 9ba26aee8ef95..0022ec57a620a 100644 --- a/packages/react-native-renderer/src/ReactFiberConfigFabric.js +++ b/packages/react-native-renderer/src/ReactFiberConfigFabric.js @@ -592,26 +592,23 @@ export function waitForCommitToBeReady(): null { } export type FragmentInstance = null | { - appendChild: (child: Instance) => void, - parentInstance: Instance, + _fragmentFiber: Fiber, ... }; -export function createFragmentInstance( - parentInstance: Instance, -): FragmentInstance { +export function createFragmentInstance(fragmentFiber: Fiber): FragmentInstance { return null; } -export function appendChildToFragmentInstance( - child: Instance, +export function commitNewChildToFragmentInstance( + child: PublicInstance, fragmentInstance: FragmentInstance, ): void { // Noop } -export function removeChildFromFragmentInstance( - child: Instance, +export function deleteChildFromFragmentInstance( + child: PublicInstance, fragmentInstance: FragmentInstance, ): void { // Noop diff --git a/packages/react-native-renderer/src/ReactFiberConfigNative.js b/packages/react-native-renderer/src/ReactFiberConfigNative.js index b7bb29b6c9ee7..092d58cc8b412 100644 --- a/packages/react-native-renderer/src/ReactFiberConfigNative.js +++ b/packages/react-native-renderer/src/ReactFiberConfigNative.js @@ -203,26 +203,23 @@ export function cloneMutableTextInstance( } export type FragmentInstance = null | { - appendChild: (child: Instance) => void, - parentInstance: Instance, + _fragmentFiber: Fiber, ... }; -export function createFragmentInstance( - parentInstance: Instance, -): FragmentInstance { +export function createFragmentInstance(fragmentFiber: Fiber): FragmentInstance { return null; } -export function appendChildToFragmentInstance( - child: Instance, +export function commitNewChildToFragmentInstance( + child: PublicInstance, fragmentInstance: FragmentInstance, ): void { // Noop } -export function removeChildFromFragmentInstance( - child: Instance, +export function deleteChildFromFragmentInstance( + child: PublicInstance, fragmentInstance: FragmentInstance, ): void { // Noop diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index c014a6f862daf..b41f22b373b54 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -516,11 +516,11 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { return null; }, - appendChildToFragmentInstance(child, fragmentInstance) { + commitNewChildToFragmentInstance(child, fragmentInstance) { // Noop }, - removeChildFromFragmentInstance(child, fragmentInstance) { + deleteChildFromFragmentInstance(child, fragmentInstance) { // Noop }, diff --git a/packages/react-reconciler/src/ReactFiberCommitEffects.js b/packages/react-reconciler/src/ReactFiberCommitEffects.js index 69e7f823c2f1f..3b73e3627ffaf 100644 --- a/packages/react-reconciler/src/ReactFiberCommitEffects.js +++ b/packages/react-reconciler/src/ReactFiberCommitEffects.js @@ -31,7 +31,6 @@ import { Fragment, HostComponent, HostHoistable, - HostRoot, HostSingleton, ViewTransitionComponent, } from './ReactWorkTags'; @@ -88,10 +87,6 @@ import { ResourceEffectIdentityKind, ResourceEffectUpdateKind, } from './ReactFiberHooks'; -import { - appendHostChildrenToFragmentInstance, - getHostParentFiber, -} from './ReactFiberCommitHostEffects'; import type {FragmentState} from './ReactFiberFragmentComponent'; function shouldProfile(current: Fiber): boolean { @@ -903,20 +898,11 @@ function commitAttachRef(finishedWork: Fiber) { case Fragment: if (enableFragmentRefs) { const instance: FragmentState = finishedWork.stateNode; - const hostParentFiber = getHostParentFiber(finishedWork); - const hostParentInstance = - hostParentFiber.tag === HostRoot - ? hostParentFiber.stateNode.containerInfo - : hostParentFiber.stateNode; if ( instance.ref === null || - instance.ref.parentInstance !== hostParentInstance + instance.ref._fragmentFiber !== finishedWork ) { - instance.ref = createFragmentInstance(hostParentInstance); - appendHostChildrenToFragmentInstance( - finishedWork.child, - instance.ref, - ); + instance.ref = createFragmentInstance(finishedWork); } instanceToUse = instance.ref; break; diff --git a/packages/react-reconciler/src/ReactFiberCommitHostEffects.js b/packages/react-reconciler/src/ReactFiberCommitHostEffects.js index 934d5e3291917..e8729eb716ac7 100644 --- a/packages/react-reconciler/src/ReactFiberCommitHostEffects.js +++ b/packages/react-reconciler/src/ReactFiberCommitHostEffects.js @@ -51,13 +51,11 @@ import { acquireSingletonInstance, releaseSingletonInstance, isSingletonScope, - appendChildToFragmentInstance, } from './ReactFiberConfig'; import {captureCommitPhaseError} from './ReactFiberWorkLoop'; import {trackHostMutation} from './ReactFiberMutationTracking'; import {runWithFiberInDEV} from './ReactCurrentFiber'; -import {enableFragmentRefs} from 'shared/ReactFeatureFlags'; import type {FragmentInstance} from './ReactFiberFragmentComponent'; export function commitHostMount(finishedWork: Fiber) { @@ -262,30 +260,6 @@ function isFragmentInstanceParent(fiber: Fiber): boolean { ); } -export function appendHostChildrenToFragmentInstance( - child: Fiber | null, - instance: FragmentInstance, -): void { - if (child === null) { - return; - } - - if (instance === null) { - return; - } - - if (child.sibling !== null) { - appendHostChildrenToFragmentInstance(child.sibling, instance); - } - - if (child.tag === HostComponent) { - appendChildToFragmentInstance(child.stateNode, instance); - return; - } - - appendHostChildrenToFragmentInstance(child.child, instance); -} - 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 @@ -359,15 +333,6 @@ function insertOrAppendPlacementNodeIntoContainer( appendChildToContainer(parent, stateNode); } trackHostMutation(); - if (enableFragmentRefs && tag === HostComponent) { - const parentFragmentInstances = getFragmentInstanceParents(node); - if (parentFragmentInstances !== null) { - for (let i = 0; i < parentFragmentInstances.length; i++) { - const instance = parentFragmentInstances[i]; - appendChildToFragmentInstance(node.stateNode, instance); - } - } - } return; } else if (tag === HostPortal) { // If the insertion itself is a portal, then we don't want to traverse @@ -412,15 +377,6 @@ function insertOrAppendPlacementNode( appendChild(parent, stateNode); } trackHostMutation(); - if (enableFragmentRefs && tag === HostComponent) { - const parentFragmentInstances = getFragmentInstanceParents(node); - if (parentFragmentInstances !== null) { - for (let i = 0; i < parentFragmentInstances.length; i++) { - const instance = parentFragmentInstances[i]; - appendChildToFragmentInstance(node.stateNode, instance); - } - } - } return; } else if (tag === HostPortal) { // If the insertion itself is a portal, then we don't want to traverse diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 6602d127a698d..e9470dda63b7a 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -167,7 +167,8 @@ import { restoreRootViewTransitionName, getPublicInstance, isSingletonScope, - removeChildFromFragmentInstance, + commitNewChildToFragmentInstance, + deleteChildFromFragmentInstance, } from './ReactFiberConfig'; import { captureCommitPhaseError, @@ -1371,11 +1372,13 @@ function commitDeletionEffectsOnFiber( getFragmentInstanceParents(deletedFiber); if (parentFragmentInstances !== null) { const element = getPublicInstance(deletedFiber.stateNode); - for (let i = 0; i < parentFragmentInstances.length; i++) { - removeChildFromFragmentInstance( - element, - parentFragmentInstances[i], - ); + if (element !== null) { + for (let i = 0; i < parentFragmentInstances.length; i++) { + deleteChildFromFragmentInstance( + element, + parentFragmentInstances[i], + ); + } } } } @@ -1973,6 +1976,7 @@ function commitMutationEffectsOnFiber( } case HostComponent: { recursivelyTraverseMutationEffects(root, finishedWork, lanes); + commitReconciliationEffects(finishedWork, lanes); if (flags & Ref) { @@ -2395,6 +2399,16 @@ function commitReconciliationEffects( const flags = finishedWork.flags; if (flags & Placement) { commitHostPlacement(finishedWork); + // After placement + if (enableFragmentRefs && finishedWork.tag === HostComponent) { + const parentFragmentInstances = getFragmentInstanceParents(finishedWork); + if (parentFragmentInstances !== null) { + for (let i = 0; i < parentFragmentInstances.length; i++) { + const instance = parentFragmentInstances[i]; + commitNewChildToFragmentInstance(finishedWork.stateNode, instance); + } + } + } // Clear the "placement" from effect tag so that we know that this is // inserted, before any life-cycles like componentDidMount gets called. // TODO: findDOMNode doesn't rely on this any more but isMounted does diff --git a/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js b/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js index 008b1843e1c56..a10e8b8cd6d66 100644 --- a/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js +++ b/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js @@ -45,11 +45,7 @@ export type ViewTransitionInstance = null | {name: string, ...}; export opaque type InstanceMeasurement = mixed; export type EventResponder = any; export type GestureTimeline = any; -export type FragmentInstance = null | { - appendChild: (child: Instance) => void, - parentInstance: Instance, - ... -}; +export type FragmentInstance = null | {_fragmentFiber: any, ...}; export const rendererVersion = $$$config.rendererVersion; export const rendererPackageName = $$$config.rendererPackageName; @@ -166,10 +162,10 @@ export const createViewTransitionInstance = $$$config.createViewTransitionInstance; export const clearContainer = $$$config.clearContainer; export const createFragmentInstance = $$$config.createFragmentInstance; -export const appendChildToFragmentInstance = - $$$config.appendChildToFragmentInstance; -export const removeChildFromFragmentInstance = - $$$config.removeChildFromFragmentInstance; +export const commitNewChildToFragmentInstance = + $$$config.commitNewChildToFragmentInstance; +export const deleteChildFromFragmentInstance = + $$$config.deleteChildFromFragmentInstance; // ------------------- // Persistence diff --git a/packages/react-test-renderer/src/ReactFiberConfigTestHost.js b/packages/react-test-renderer/src/ReactFiberConfigTestHost.js index d46ad3036f5d5..0b6e24bfc395e 100644 --- a/packages/react-test-renderer/src/ReactFiberConfigTestHost.js +++ b/packages/react-test-renderer/src/ReactFiberConfigTestHost.js @@ -448,8 +448,7 @@ export function createViewTransitionInstance( } export type FragmentInstance = null | { - appendChild: (child: Instance) => void, - parentInstance: Instance, + _fragmentFiber: any, ... }; @@ -459,14 +458,14 @@ export function createFragmentInstance( return null; } -export function appendChildToFragmentInstance( +export function commitNewChildToFragmentInstance( child: Instance, fragmentInstance: FragmentInstance, ): void { // noop } -export function removeChildFromFragmentInstance( +export function deleteChildFromFragmentInstance( child: Instance, fragmentInstance: FragmentInstance, ): void { From 8683fa3c33864625a4de162452337fa13abf995f Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Mon, 24 Feb 2025 16:43:21 -0500 Subject: [PATCH 07/21] Handle Activity mode switch --- .../__tests__/ReactDOMFragmentRefs-test.js | 129 ++++++++++++++++++ .../src/ReactFiberCommitWork.js | 57 ++++---- 2 files changed, 160 insertions(+), 26 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js index 5a5cdc58a7efe..c4076365d589f 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js @@ -13,11 +13,13 @@ let React; let ReactDOMClient; let act; let container; +let Activity; describe('FragmentRefs', () => { beforeEach(() => { jest.resetModules(); React = require('react'); + Activity = React.unstable_Activity; ReactDOMClient = require('react-dom/client'); act = require('internal-test-utils').act; container = document.createElement('div'); @@ -417,5 +419,132 @@ describe('FragmentRefs', () => { nestedChildRef.current.click(); expect(logs).toEqual(['Host A', 'Host B']); }); + + describe('with activity', () => { + // @gate enableFragmentRefs && enableActivity + it('does not apply event listeners to hidden trees', async () => { + const parentRef = React.createRef(); + const fragmentRef = React.createRef(); + const root = ReactDOMClient.createRoot(container); + + function Test() { + return ( +
+ +
Child 1
+ +
Child 2
+
+
Child 3
+
+
+ ); + } + + await act(() => { + root.render(); + }); + + const logs = []; + fragmentRef.current.addEventListener('click', e => { + logs.push(e.target.textContent); + }); + + const [child1, child2, child3] = parentRef.current.children; + child1.click(); + child2.click(); + child3.click(); + expect(logs).toEqual(['Child 1', 'Child 3']); + }); + + // @gate enableFragmentRefs && enableActivity + it('applies event listeners to visible trees', async () => { + const parentRef = React.createRef(); + const fragmentRef = React.createRef(); + const root = ReactDOMClient.createRoot(container); + + function Test() { + return ( +
+ +
Child 1
+ +
Child 2
+
+
Child 3
+
+
+ ); + } + + await act(() => { + root.render(); + }); + + const logs = []; + fragmentRef.current.addEventListener('click', e => { + logs.push(e.target.textContent); + }); + + const [child1, child2, child3] = parentRef.current.children; + child1.click(); + child2.click(); + child3.click(); + expect(logs).toEqual(['Child 1', 'Child 2', 'Child 3']); + }); + + // @gate enableFragmentRefs && enableActivity + it('handles Activity modes switching', async () => { + const fragmentRef = React.createRef(); + const fragmentRef2 = React.createRef(); + const parentRef = React.createRef(); + const root = ReactDOMClient.createRoot(container); + + function Test({mode}) { + return ( +
+ + +
Child
+ +
Child 2
+
+
+
+
+ ); + } + + await act(() => { + root.render(); + }); + + let logs = []; + fragmentRef.current.addEventListener('click', () => { + logs.push('clicked 1'); + }); + fragmentRef2.current.addEventListener('click', () => { + logs.push('clicked 2'); + }); + parentRef.current.lastChild.click(); + expect(logs).toEqual(['clicked 1', 'clicked 2']); + + logs = []; + await act(() => { + root.render(); + }); + parentRef.current.firstChild.click(); + parentRef.current.lastChild.click(); + expect(logs).toEqual([]); + + logs = []; + await act(() => { + root.render(); + }); + parentRef.current.lastChild.click(); + // Event order is flipped here because the nested child re-registers first + expect(logs).toEqual(['clicked 2', 'clicked 1']); + }); + }); }); }); diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index e9470dda63b7a..4c306008023c4 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -1366,22 +1366,7 @@ function commitDeletionEffectsOnFiber( if (!offscreenSubtreeWasHidden) { safelyDetachRef(deletedFiber, nearestMountedAncestor); } - - if (enableFragmentRefs) { - const parentFragmentInstances = - getFragmentInstanceParents(deletedFiber); - if (parentFragmentInstances !== null) { - const element = getPublicInstance(deletedFiber.stateNode); - if (element !== null) { - for (let i = 0; i < parentFragmentInstances.length; i++) { - deleteChildFromFragmentInstance( - element, - parentFragmentInstances[i], - ); - } - } - } - } + commitFragmentInstanceDeletionEffects(deletedFiber); // Intentional fallthrough to next branch } case HostText: { @@ -2399,16 +2384,7 @@ function commitReconciliationEffects( const flags = finishedWork.flags; if (flags & Placement) { commitHostPlacement(finishedWork); - // After placement - if (enableFragmentRefs && finishedWork.tag === HostComponent) { - const parentFragmentInstances = getFragmentInstanceParents(finishedWork); - if (parentFragmentInstances !== null) { - for (let i = 0; i < parentFragmentInstances.length; i++) { - const instance = parentFragmentInstances[i]; - commitNewChildToFragmentInstance(finishedWork.stateNode, instance); - } - } - } + commitFragmentInstanceInsertionEffects(finishedWork); // Clear the "placement" from effect tag so that we know that this is // inserted, before any life-cycles like componentDidMount gets called. // TODO: findDOMNode doesn't rely on this any more but isMounted does @@ -2686,6 +2662,8 @@ export function disappearLayoutEffects(finishedWork: Fiber) { // TODO (Offscreen) Check: flags & RefStatic safelyDetachRef(finishedWork, finishedWork.return); + commitFragmentInstanceDeletionEffects(finishedWork); + recursivelyTraverseDisappearLayoutEffects(finishedWork); break; } @@ -2813,6 +2791,7 @@ export function reappearLayoutEffects( } case HostHoistable: case HostComponent: { + commitFragmentInstanceInsertionEffects(finishedWork); recursivelyTraverseReappearLayoutEffects( finishedRoot, finishedWork, @@ -3061,6 +3040,32 @@ function commitOffscreenPassiveMountEffects( } } +function commitFragmentInstanceInsertionEffects(finishedWork: Fiber): void { + if (enableFragmentRefs && finishedWork.tag === HostComponent) { + const parentFragmentInstances = getFragmentInstanceParents(finishedWork); + if (parentFragmentInstances !== null) { + for (let i = 0; i < parentFragmentInstances.length; i++) { + const instance = parentFragmentInstances[i]; + commitNewChildToFragmentInstance(finishedWork.stateNode, instance); + } + } + } +} + +function commitFragmentInstanceDeletionEffects(finishedWork: Fiber): void { + if (enableFragmentRefs && finishedWork.tag === HostComponent) { + const parentFragmentInstances = getFragmentInstanceParents(finishedWork); + if (parentFragmentInstances !== null) { + const element = getPublicInstance(finishedWork.stateNode); + if (element !== null) { + for (let i = 0; i < parentFragmentInstances.length; i++) { + deleteChildFromFragmentInstance(element, parentFragmentInstances[i]); + } + } + } + } +} + function commitCachePassiveMountEffect( current: Fiber | null, finishedWork: Fiber, From b1c8f82d0447f1084b8c8e29a59f8d42fbf82459 Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Mon, 24 Feb 2025 17:32:43 -0500 Subject: [PATCH 08/21] clean up --- packages/react-art/src/ReactFiberConfigART.js | 2 +- .../src/client/ReactFiberConfigDOM.js | 34 +++++---- .../__tests__/ReactDOMFragmentRefs-test.js | 71 ++++++++++--------- .../src/forks/ReactFiberConfig.custom.js | 2 +- .../src/ReactFiberConfigTestHost.js | 4 +- 5 files changed, 63 insertions(+), 50 deletions(-) diff --git a/packages/react-art/src/ReactFiberConfigART.js b/packages/react-art/src/ReactFiberConfigART.js index 0a9e406bfc565..7afc61dd3b69e 100644 --- a/packages/react-art/src/ReactFiberConfigART.js +++ b/packages/react-art/src/ReactFiberConfigART.js @@ -319,7 +319,7 @@ export function cloneMutableTextInstance(textInstance) { } export type FragmentInstance = null | { - _fragmentFiber: any, + _fragmentFiber: Object, ... }; diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index e5d7b86b1f0cb..db6da4eb78ee8 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -2124,22 +2124,25 @@ FragmentInstancePseudoElement.prototype.addEventListener = function ( if (!this._eventListeners) { this._eventListeners = []; } + + const listeners = this._eventListeners; // Element.addEventListener will only apply uniquely new event listeners by default. Since we // need to collect the listeners to apply to appended children, we track them ourselves and use // custom equality check for the options. const isNewEventListener = - indexOfEventListener(this._eventListeners, { + indexOfEventListener(listeners, { type, listener, optionsOrUseCapture, }) === -1; - if (isNewEventListener && this._eventListeners !== undefined) { - this._eventListeners.push({type, listener, optionsOrUseCapture}); + if (isNewEventListener) { + listeners.push({type, listener, optionsOrUseCapture}); const children = getFragmentInstanceChildren(this); children.forEach(child => { child.addEventListener(type, listener, optionsOrUseCapture); }); } + this._eventListeners = listeners; }; // $FlowFixMe[prop-missing] FragmentInstancePseudoElement.prototype.removeEventListener = function ( @@ -2148,18 +2151,21 @@ FragmentInstancePseudoElement.prototype.removeEventListener = function ( listener: EventListener, optionsOrUseCapture?: EventListenerOptionsOrUseCapture, ): void { - if (Array.isArray(this._eventListeners) && this._eventListeners.length > 0) { - const children = getFragmentInstanceChildren(this); - children.forEach(child => { - child.removeEventListener(type, listener, optionsOrUseCapture); - }); - const index = indexOfEventListener(this._eventListeners || [], { - type, - listener, - optionsOrUseCapture, - }); - this._eventListeners = (this._eventListeners || []).splice(index, 1); + const listeners = this._eventListeners; + if (listeners === undefined || listeners.length === 0) { + return; } + + const children = getFragmentInstanceChildren(this); + children.forEach(child => { + child.removeEventListener(type, listener, optionsOrUseCapture); + }); + const index = indexOfEventListener(listeners, { + type, + listener, + optionsOrUseCapture, + }); + this._eventListeners = listeners.splice(index, 1); }; // $FlowFixMe[prop-missing] FragmentInstancePseudoElement.prototype.focus = function ( diff --git a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js index c4076365d589f..1550dd2d3d1cd 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js @@ -13,12 +13,14 @@ let React; let ReactDOMClient; let act; let container; +let Fragment; let Activity; describe('FragmentRefs', () => { beforeEach(() => { jest.resetModules(); React = require('react'); + Fragment = React.Fragment; Activity = React.unstable_Activity; ReactDOMClient = require('react-dom/client'); act = require('internal-test-utils').act; @@ -38,9 +40,9 @@ describe('FragmentRefs', () => { await act(() => root.render(
- +
Hi
-
+
, ), ); @@ -58,9 +60,9 @@ describe('FragmentRefs', () => { await act(() => { root.render( - (fragmentRef = ref)}> + (fragmentRef = ref)}>
Hi
-
, + , ); }); @@ -78,9 +80,9 @@ describe('FragmentRefs', () => { expect(fragmentRef.current).not.toBe(null); }); return ( - +
- + ); } @@ -99,13 +101,15 @@ describe('FragmentRefs', () => { function Test() { return (
- +
- A + B -
- + + C + +
); } @@ -114,6 +118,9 @@ describe('FragmentRefs', () => { root.render(); }); + // The test environment doesn't implement focus. + // Mock it here, along with a naive focusable query so we can assert + // that the first _focusable_ element is found. const focusableChildren = parentRef.current.querySelectorAll( 'button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])', ); @@ -139,10 +146,10 @@ describe('FragmentRefs', () => { function Test({showA, showB}) { return ( - + {showA && } {showB && } - + ); } @@ -211,10 +218,10 @@ describe('FragmentRefs', () => { }, []); return (
- +
A
B
-
+
); } @@ -270,17 +277,17 @@ describe('FragmentRefs', () => { await act(() => { root.render(
- +
A
- +
B
-
+
- +
C
-
-
+ +
, ); }); @@ -348,14 +355,14 @@ describe('FragmentRefs', () => { return (
- +
A
{shouldShowChild && (
B
)} -
+
); } @@ -395,7 +402,7 @@ describe('FragmentRefs', () => { await act(() => { root.render(
- +
Host A
@@ -404,7 +411,7 @@ describe('FragmentRefs', () => { -
+
, ); }); @@ -430,13 +437,13 @@ describe('FragmentRefs', () => { function Test() { return (
- +
Child 1
Child 2
Child 3
-
+
); } @@ -466,13 +473,13 @@ describe('FragmentRefs', () => { function Test() { return (
- +
Child 1
Child 2
Child 3
-
+
); } @@ -503,14 +510,14 @@ describe('FragmentRefs', () => { function Test({mode}) { return (
- +
Child
- +
Child 2
-
+
-
+
); } diff --git a/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js b/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js index a10e8b8cd6d66..d45afd09da974 100644 --- a/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js +++ b/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js @@ -45,7 +45,7 @@ export type ViewTransitionInstance = null | {name: string, ...}; export opaque type InstanceMeasurement = mixed; export type EventResponder = any; export type GestureTimeline = any; -export type FragmentInstance = null | {_fragmentFiber: any, ...}; +export type FragmentInstance = null | {_fragmentFiber: Object, ...}; export const rendererVersion = $$$config.rendererVersion; export const rendererPackageName = $$$config.rendererPackageName; diff --git a/packages/react-test-renderer/src/ReactFiberConfigTestHost.js b/packages/react-test-renderer/src/ReactFiberConfigTestHost.js index 0b6e24bfc395e..3796eff01c5eb 100644 --- a/packages/react-test-renderer/src/ReactFiberConfigTestHost.js +++ b/packages/react-test-renderer/src/ReactFiberConfigTestHost.js @@ -448,12 +448,12 @@ export function createViewTransitionInstance( } export type FragmentInstance = null | { - _fragmentFiber: any, + _fragmentFiber: Object, ... }; export function createFragmentInstance( - parentInstance: Instance | Container, + fragmentFiber: Object, ): FragmentInstance { return null; } From 0b658a83a061343d62ffd43e71c71442a77c8f62 Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Tue, 25 Feb 2025 11:39:19 -0500 Subject: [PATCH 09/21] Fix traversal order, remove focus mocks --- .../src/client/ReactFiberConfigDOM.js | 11 ++++++----- .../src/__tests__/ReactDOMFragmentRefs-test.js | 18 ++---------------- 2 files changed, 8 insertions(+), 21 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index db6da4eb78ee8..6a25889b1355b 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -2201,16 +2201,17 @@ function recursivelyGetFragmentInstanceChildren( return; } + if (child.tag === HostComponent) { + childElements.add(child.stateNode); + } + if (child.sibling !== null) { recursivelyGetFragmentInstanceChildren(child.sibling, childElements); } - if (child.tag === HostComponent) { - childElements.add(child.stateNode); - return; + if (child.tag !== HostComponent) { + recursivelyGetFragmentInstanceChildren(child.child, childElements); } - - recursivelyGetFragmentInstanceChildren(child.child, childElements); } function normalizeListenerOptions( diff --git a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js index 1550dd2d3d1cd..2c740203c1d83 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js @@ -96,13 +96,13 @@ describe('FragmentRefs', () => { const parentRef = React.createRef(); const fragmentRef = React.createRef(); const root = ReactDOMClient.createRoot(container); - let focusedElement = null; function Test() { return (
+ B @@ -118,24 +118,10 @@ describe('FragmentRefs', () => { root.render(); }); - // The test environment doesn't implement focus. - // Mock it here, along with a naive focusable query so we can assert - // that the first _focusable_ element is found. - const focusableChildren = parentRef.current.querySelectorAll( - 'button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])', - ); - const focusMock = jest.spyOn(HTMLElement.prototype, 'focus'); - focusMock.mockImplementation(function () { - if (Array.from(focusableChildren).includes(this)) { - focusedElement = this.id; - } else { - return false; - } - }); await act(() => { fragmentRef.current.focus(); }); - expect(focusedElement).toEqual('child-b'); + expect(document.activeElement.id).toEqual('child-b'); }); // @gate enableFragmentRefs From b928cd3c24d80924a39a1a33554ab15f367b10d0 Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Tue, 25 Feb 2025 14:45:27 -0500 Subject: [PATCH 10/21] Set stateNode in commit phase --- .../react-reconciler/src/ReactChildFiber.js | 5 ----- packages/react-reconciler/src/ReactFiber.js | 17 +---------------- .../react-reconciler/src/ReactFiberBeginWork.js | 4 ---- .../src/ReactFiberCommitEffects.js | 13 +++++-------- .../src/ReactFiberCommitHostEffects.js | 11 +++-------- .../src/ReactFiberFragmentComponent.js | 16 ---------------- 6 files changed, 9 insertions(+), 57 deletions(-) delete mode 100644 packages/react-reconciler/src/ReactFiberFragmentComponent.js diff --git a/packages/react-reconciler/src/ReactChildFiber.js b/packages/react-reconciler/src/ReactChildFiber.js index b40ebb60405b4..6af8c1356f9ca 100644 --- a/packages/react-reconciler/src/ReactChildFiber.js +++ b/packages/react-reconciler/src/ReactChildFiber.js @@ -617,9 +617,6 @@ function createChildReconciler( returnFiber.mode, lanes, key, - enableFragmentRefs && - returnFiber.stateNode !== null && - returnFiber.stateNode.ref !== undefined, ); created.return = returnFiber; if (__DEV__) { @@ -725,7 +722,6 @@ function createChildReconciler( returnFiber.mode, lanes, null, - false, ); created.return = returnFiber; if (__DEV__) { @@ -1692,7 +1688,6 @@ function createChildReconciler( returnFiber.mode, lanes, element.key, - enableFragmentRefs && element.props.ref !== undefined, ); if (enableFragmentRefs) { coerceRef(created, element); diff --git a/packages/react-reconciler/src/ReactFiber.js b/packages/react-reconciler/src/ReactFiber.js index 70af89f7e21d5..70cb3ed9de023 100644 --- a/packages/react-reconciler/src/ReactFiber.js +++ b/packages/react-reconciler/src/ReactFiber.js @@ -24,7 +24,6 @@ import type { ViewTransitionState, } from './ReactFiberViewTransitionComponent'; import type {TracingMarkerInstance} from './ReactFiberTracingMarkerComponent'; -import type {FragmentState} from './ReactFiberFragmentComponent'; import { supportsResources, @@ -42,7 +41,6 @@ import { disableLegacyMode, enableObjectFiber, enableViewTransition, - enableFragmentRefs, } from 'shared/ReactFeatureFlags'; import {NoFlags, Placement, StaticMask} from './ReactFiberFlags'; import {ConcurrentRoot} from './ReactRootTags'; @@ -591,13 +589,7 @@ export function createFiberFromTypeAndProps( } else { getTag: switch (type) { case REACT_FRAGMENT_TYPE: - return createFiberFromFragment( - pendingProps.children, - mode, - lanes, - key, - enableFragmentRefs && pendingProps.ref !== undefined, - ); + return createFiberFromFragment(pendingProps.children, mode, lanes, key); case REACT_STRICT_MODE_TYPE: fiberTag = Mode; mode |= StrictLegacyMode; @@ -778,15 +770,8 @@ export function createFiberFromFragment( mode: TypeOfMode, lanes: Lanes, key: null | string, - hasRef: boolean, ): Fiber { const fiber = createFiber(Fragment, elements, key, mode); - if (enableFragmentRefs && hasRef) { - const instance: FragmentState = { - ref: null, - }; - fiber.stateNode = instance; - } fiber.lanes = lanes; return fiber; } diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 9b817b11590ba..b37bd4ea3f44f 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -2359,7 +2359,6 @@ function mountSuspenseFallbackChildren( mode, renderLanes, null, - false, ); } else { primaryChildFragment = mountWorkInProgressOffscreenFiber( @@ -2372,7 +2371,6 @@ function mountSuspenseFallbackChildren( mode, renderLanes, null, - false, ); } @@ -2515,7 +2513,6 @@ function updateSuspenseFallbackChildren( mode, renderLanes, null, - false, ); // Needs a placement effect because the parent (the Suspense boundary) already // mounted but this is a new fiber. @@ -2580,7 +2577,6 @@ function mountSuspenseFallbackAfterRetryWithoutHydrating( fiberMode, renderLanes, null, - false, ); // Needs a placement effect because the parent (the Suspense // boundary) already mounted but this is a new fiber. diff --git a/packages/react-reconciler/src/ReactFiberCommitEffects.js b/packages/react-reconciler/src/ReactFiberCommitEffects.js index 3b73e3627ffaf..563b117a054b9 100644 --- a/packages/react-reconciler/src/ReactFiberCommitEffects.js +++ b/packages/react-reconciler/src/ReactFiberCommitEffects.js @@ -11,6 +11,7 @@ import type {Fiber} from './ReactInternalTypes'; import type {UpdateQueue} from './ReactFiberClassUpdateQueue'; import type {FunctionComponentUpdateQueue} from './ReactFiberHooks'; import type {HookFlags} from './ReactHookEffectTags'; +import type {FragmentInstance} from './ReactFiberConfig'; import { getViewTransitionName, type ViewTransitionState, @@ -87,7 +88,6 @@ import { ResourceEffectIdentityKind, ResourceEffectUpdateKind, } from './ReactFiberHooks'; -import type {FragmentState} from './ReactFiberFragmentComponent'; function shouldProfile(current: Fiber): boolean { return ( @@ -897,14 +897,11 @@ function commitAttachRef(finishedWork: Fiber) { } case Fragment: if (enableFragmentRefs) { - const instance: FragmentState = finishedWork.stateNode; - if ( - instance.ref === null || - instance.ref._fragmentFiber !== finishedWork - ) { - instance.ref = createFragmentInstance(finishedWork); + const instance: null | FragmentInstance = finishedWork.stateNode; + if (instance === null || instance._fragmentFiber !== finishedWork) { + finishedWork.stateNode = createFragmentInstance(finishedWork); } - instanceToUse = instance.ref; + instanceToUse = finishedWork.stateNode; break; } // Fallthrough diff --git a/packages/react-reconciler/src/ReactFiberCommitHostEffects.js b/packages/react-reconciler/src/ReactFiberCommitHostEffects.js index e8729eb716ac7..abd102b9a60df 100644 --- a/packages/react-reconciler/src/ReactFiberCommitHostEffects.js +++ b/packages/react-reconciler/src/ReactFiberCommitHostEffects.js @@ -13,6 +13,7 @@ import type { SuspenseInstance, Container, ChildSet, + FragmentInstance, } from './ReactFiberConfig'; import type {Fiber, FiberRoot} from './ReactInternalTypes'; @@ -56,7 +57,6 @@ import {captureCommitPhaseError} from './ReactFiberWorkLoop'; import {trackHostMutation} from './ReactFiberMutationTracking'; import {runWithFiberInDEV} from './ReactCurrentFiber'; -import type {FragmentInstance} from './ReactFiberFragmentComponent'; export function commitHostMount(finishedWork: Fiber) { const type = finishedWork.type; @@ -226,7 +226,7 @@ export function getFragmentInstanceParents( if (fragmentParents === null) { fragmentParents = []; } - const fragmentInstance: FragmentInstance = parent.stateNode.ref; + const fragmentInstance: FragmentInstance = parent.stateNode; fragmentParents.push(fragmentInstance); } @@ -252,12 +252,7 @@ function isHostParent(fiber: Fiber): boolean { } function isFragmentInstanceParent(fiber: Fiber): boolean { - return ( - fiber && - fiber.tag === Fragment && - fiber.stateNode !== null && - fiber.stateNode.ref !== null - ); + return fiber && fiber.tag === Fragment && fiber.stateNode !== null; } function getHostSibling(fiber: Fiber): ?Instance { diff --git a/packages/react-reconciler/src/ReactFiberFragmentComponent.js b/packages/react-reconciler/src/ReactFiberFragmentComponent.js deleted file mode 100644 index cfbb8500be523..0000000000000 --- a/packages/react-reconciler/src/ReactFiberFragmentComponent.js +++ /dev/null @@ -1,16 +0,0 @@ -/** - * 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 {FragmentInstance as _FragmentInstance} from './ReactFiberConfig'; - -export type FragmentInstance = _FragmentInstance; - -export type FragmentState = { - ref: null | FragmentInstance, -}; From 62f1619e1a39f99cf6dab371bf9ef2ccff7ab6e7 Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Tue, 25 Feb 2025 15:06:55 -0500 Subject: [PATCH 11/21] Refactor fragment parent traversal to avoid allocations --- .../src/ReactFiberCommitHostEffects.js | 48 +++++++++++++++---- .../src/ReactFiberCommitWork.js | 48 ++++++------------- 2 files changed, 52 insertions(+), 44 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitHostEffects.js b/packages/react-reconciler/src/ReactFiberCommitHostEffects.js index abd102b9a60df..b00e76da46229 100644 --- a/packages/react-reconciler/src/ReactFiberCommitHostEffects.js +++ b/packages/react-reconciler/src/ReactFiberCommitHostEffects.js @@ -52,6 +52,8 @@ import { acquireSingletonInstance, releaseSingletonInstance, isSingletonScope, + commitNewChildToFragmentInstance, + deleteChildFromFragmentInstance, } from './ReactFiberConfig'; import {captureCommitPhaseError} from './ReactFiberWorkLoop'; import {trackHostMutation} from './ReactFiberMutationTracking'; @@ -216,27 +218,53 @@ export function getHostParentFiber(fiber: Fiber): Fiber { ); } -export function getFragmentInstanceParents( - fiber: Fiber, -): null | Array { +export function commitFragmentInstanceInsertionEffects(fiber: Fiber): void { + let parent = fiber.return; + while (parent !== null) { + if (isFragmentInstanceParent(parent)) { + const fragmentInstance: FragmentInstance = parent.stateNode; + commitNewChildToFragmentInstance(fiber.stateNode, fragmentInstance); + } + + if (isHostParent(parent)) { + return; + } + + parent = parent.return; + } +} + +export function commitFragmentInstanceDeletionEffects(fiber: Fiber): void { + commitEffectsForFragmentInstanceParents(fiber); + let parent = fiber.return; + while (parent !== null) { + if (isFragmentInstanceParent(parent)) { + const fragmentInstance: FragmentInstance = parent.stateNode; + deleteChildFromFragmentInstance(fiber.stateNode, fragmentInstance); + } + + if (isHostParent(parent)) { + return; + } + + parent = parent.return; + } +} + +function commitEffectsForFragmentInstanceParents(fiber: Fiber): void { let parent = fiber.return; - let fragmentParents: null | Array = null; while (parent !== null) { if (isFragmentInstanceParent(parent)) { - if (fragmentParents === null) { - fragmentParents = []; - } const fragmentInstance: FragmentInstance = parent.stateNode; - fragmentParents.push(fragmentInstance); + deleteChildFromFragmentInstance(fiber.stateNode, fragmentInstance); } if (isHostParent(parent)) { - return fragmentParents; + return; } parent = parent.return; } - return fragmentParents; } function isHostParent(fiber: Fiber): boolean { diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 4c306008023c4..625403fef4e7f 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -165,10 +165,7 @@ import { cancelViewTransitionName, cancelRootViewTransitionName, restoreRootViewTransitionName, - getPublicInstance, isSingletonScope, - commitNewChildToFragmentInstance, - deleteChildFromFragmentInstance, } from './ReactFiberConfig'; import { captureCommitPhaseError, @@ -240,7 +237,8 @@ import { commitHostRemoveChild, commitHostSingletonAcquisition, commitHostSingletonRelease, - getFragmentInstanceParents, + commitFragmentInstanceInsertionEffects, + commitFragmentInstanceDeletionEffects, } from './ReactFiberCommitHostEffects'; import { commitEnterViewTransitions, @@ -1366,7 +1364,9 @@ function commitDeletionEffectsOnFiber( if (!offscreenSubtreeWasHidden) { safelyDetachRef(deletedFiber, nearestMountedAncestor); } - commitFragmentInstanceDeletionEffects(deletedFiber); + if (enableFragmentRefs) { + commitFragmentInstanceDeletionEffects(deletedFiber); + } // Intentional fallthrough to next branch } case HostText: { @@ -2384,7 +2384,9 @@ function commitReconciliationEffects( const flags = finishedWork.flags; if (flags & Placement) { commitHostPlacement(finishedWork); - commitFragmentInstanceInsertionEffects(finishedWork); + if (enableFragmentRefs && finishedWork.tag === HostComponent) { + commitFragmentInstanceInsertionEffects(finishedWork); + } // Clear the "placement" from effect tag so that we know that this is // inserted, before any life-cycles like componentDidMount gets called. // TODO: findDOMNode doesn't rely on this any more but isMounted does @@ -2662,7 +2664,9 @@ export function disappearLayoutEffects(finishedWork: Fiber) { // TODO (Offscreen) Check: flags & RefStatic safelyDetachRef(finishedWork, finishedWork.return); - commitFragmentInstanceDeletionEffects(finishedWork); + if (enableFragmentRefs && finishedWork.tag === HostComponent) { + commitFragmentInstanceDeletionEffects(finishedWork); + } recursivelyTraverseDisappearLayoutEffects(finishedWork); break; @@ -2791,7 +2795,9 @@ export function reappearLayoutEffects( } case HostHoistable: case HostComponent: { - commitFragmentInstanceInsertionEffects(finishedWork); + if (enableFragmentRefs && finishedWork.tag === HostComponent) { + commitFragmentInstanceInsertionEffects(finishedWork); + } recursivelyTraverseReappearLayoutEffects( finishedRoot, finishedWork, @@ -3040,32 +3046,6 @@ function commitOffscreenPassiveMountEffects( } } -function commitFragmentInstanceInsertionEffects(finishedWork: Fiber): void { - if (enableFragmentRefs && finishedWork.tag === HostComponent) { - const parentFragmentInstances = getFragmentInstanceParents(finishedWork); - if (parentFragmentInstances !== null) { - for (let i = 0; i < parentFragmentInstances.length; i++) { - const instance = parentFragmentInstances[i]; - commitNewChildToFragmentInstance(finishedWork.stateNode, instance); - } - } - } -} - -function commitFragmentInstanceDeletionEffects(finishedWork: Fiber): void { - if (enableFragmentRefs && finishedWork.tag === HostComponent) { - const parentFragmentInstances = getFragmentInstanceParents(finishedWork); - if (parentFragmentInstances !== null) { - const element = getPublicInstance(finishedWork.stateNode); - if (element !== null) { - for (let i = 0; i < parentFragmentInstances.length; i++) { - deleteChildFromFragmentInstance(element, parentFragmentInstances[i]); - } - } - } - } -} - function commitCachePassiveMountEffect( current: Fiber | null, finishedWork: Fiber, From cd64b20f27d447c1556e87d6c0f486be77f83fe3 Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Tue, 25 Feb 2025 17:43:09 -0500 Subject: [PATCH 12/21] Avoid allocations while traversing host children --- .../src/client/ReactFiberConfigDOM.js | 97 ++++++++++--------- .../__tests__/ReactDOMFragmentRefs-test.js | 5 +- 2 files changed, 57 insertions(+), 45 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index 6a25889b1355b..aa9d75f59cee2 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -34,6 +34,7 @@ import {getCurrentRootHostContainer} from 'react-reconciler/src/ReactFiberHostCo import hasOwnProperty from 'shared/hasOwnProperty'; import {checkAttributeStringCoercion} from 'shared/CheckStringCoercion'; import {REACT_CONTEXT_TYPE} from 'shared/ReactSymbols'; +import {OffscreenComponent} from 'react-reconciler/src/ReactWorkTags'; export { setCurrentUpdatePriority, @@ -2137,13 +2138,22 @@ FragmentInstancePseudoElement.prototype.addEventListener = function ( }) === -1; if (isNewEventListener) { listeners.push({type, listener, optionsOrUseCapture}); - const children = getFragmentInstanceChildren(this); - children.forEach(child => { - child.addEventListener(type, listener, optionsOrUseCapture); - }); + traverseFragmentInstanceChildren( + this, + this._fragmentFiber.child, + addEventListenerToChild.bind(null, type, listener, optionsOrUseCapture), + ); } this._eventListeners = listeners; }; +function addEventListenerToChild( + type: string, + listener: EventListener, + optionsOrUseCapture?: EventListenerOptionsOrUseCapture, + child: Element, +): void { + child.addEventListener(type, listener, optionsOrUseCapture); +} // $FlowFixMe[prop-missing] FragmentInstancePseudoElement.prototype.removeEventListener = function ( this: FragmentInstance, @@ -2155,11 +2165,16 @@ FragmentInstancePseudoElement.prototype.removeEventListener = function ( if (listeners === undefined || listeners.length === 0) { return; } - - const children = getFragmentInstanceChildren(this); - children.forEach(child => { - child.removeEventListener(type, listener, optionsOrUseCapture); - }); + traverseFragmentInstanceChildren( + this, + this._fragmentFiber.child, + removeEventListenerFromChild.bind( + null, + type, + listener, + optionsOrUseCapture, + ), + ); const index = indexOfEventListener(listeners, { type, listener, @@ -2167,50 +2182,44 @@ FragmentInstancePseudoElement.prototype.removeEventListener = function ( }); this._eventListeners = listeners.splice(index, 1); }; +function removeEventListenerFromChild( + type: string, + listener: EventListener, + optionsOrUseCapture?: EventListenerOptionsOrUseCapture, + child: Element, +): void { + child.removeEventListener(type, listener, optionsOrUseCapture); +} // $FlowFixMe[prop-missing] FragmentInstancePseudoElement.prototype.focus = function ( this: FragmentInstance, ) { - const children = getFragmentInstanceChildren(this); - const iterator = children.values(); - let child = iterator.next(); - while (!child.done) { - if (setFocusIfFocusable(child.value)) { - break; - } - child = iterator.next(); - } + traverseFragmentInstanceChildren( + this, + this._fragmentFiber.child, + setFocusIfFocusable, + ); }; -function getFragmentInstanceChildren( +function traverseFragmentInstanceChildren( fragmentInstance: FragmentInstance, -): Set { - const children = new Set(); - recursivelyGetFragmentInstanceChildren( - fragmentInstance._fragmentFiber.child, - children, - ); - return children; -} - -function recursivelyGetFragmentInstanceChildren( child: Fiber | null, - childElements: Set, + fn: (child: Element) => boolean | void, ): void { - if (child === null) { - return; - } - - if (child.tag === HostComponent) { - childElements.add(child.stateNode); - } - - if (child.sibling !== null) { - recursivelyGetFragmentInstanceChildren(child.sibling, childElements); - } - - if (child.tag !== HostComponent) { - recursivelyGetFragmentInstanceChildren(child.child, childElements); + while (child !== null) { + if (child.tag === HostComponent) { + if (fn(child.stateNode)) { + return; + } + } else if ( + child.tag === OffscreenComponent && + child.memoizedState !== null + ) { + // Skip hidden subtrees + } else { + traverseFragmentInstanceChildren(fragmentInstance, child.child, fn); + } + child = child.sibling; } } diff --git a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js index 2c740203c1d83..d5f4cfcf936af 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js @@ -205,8 +205,11 @@ describe('FragmentRefs', () => { return (
+ <>Text
A
-
B
+ <> +
B
+
); From 1a507ffb839897f2d85fa96e7697ddedb9c9e821 Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Wed, 26 Feb 2025 17:35:11 -0500 Subject: [PATCH 13/21] Keep fiber reference up to date --- .../__tests__/ReactDOMFragmentRefs-test.js | 24 ++++++++----------- .../src/ReactFiberCommitWork.js | 3 +++ 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js index d5f4cfcf936af..9f7ec809c96e1 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js @@ -93,13 +93,12 @@ describe('FragmentRefs', () => { describe('focus()', () => { // @gate enableFragmentRefs it('focuses the first focusable child', async () => { - const parentRef = React.createRef(); const fragmentRef = React.createRef(); const root = ReactDOMClient.createRoot(container); function Test() { return ( -
+
@@ -122,28 +121,23 @@ describe('FragmentRefs', () => { fragmentRef.current.focus(); }); expect(document.activeElement.id).toEqual('child-b'); + document.activeElement.blur(); }); // @gate enableFragmentRefs it('preserves document order when adding and removing children', async () => { const fragmentRef = React.createRef(); const root = ReactDOMClient.createRoot(container); - let focusedElement = null; function Test({showA, showB}) { return ( - {showA && } - {showB && } + {showA && } + {showB && } ); } - const focusMock = jest.spyOn(HTMLElement.prototype, 'focus'); - focusMock.mockImplementation(function () { - focusedElement = this.id; - }); - // Render with A as the first focusable child await act(() => { root.render(); @@ -151,8 +145,8 @@ describe('FragmentRefs', () => { await act(() => { fragmentRef.current.focus(); }); - expect(focusedElement).toEqual('child-a'); - + expect(document.activeElement.id).toEqual('child-a'); + document.activeElement.blur(); // A is still the first focusable child, but B is also tracked await act(() => { root.render(); @@ -160,7 +154,8 @@ describe('FragmentRefs', () => { await act(() => { fragmentRef.current.focus(); }); - expect(focusedElement).toEqual('child-a'); + expect(document.activeElement.id).toEqual('child-a'); + document.activeElement.blur(); // B is now the first focusable child await act(() => { @@ -169,7 +164,8 @@ describe('FragmentRefs', () => { await act(() => { fragmentRef.current.focus(); }); - expect(focusedElement).toEqual('child-b'); + expect(document.activeElement.id).toEqual('child-b'); + document.activeElement.blur(); }); }); diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 625403fef4e7f..3570f79b1a0bd 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -2339,6 +2339,9 @@ function commitMutationEffectsOnFiber( } case Fragment: if (enableFragmentRefs) { + if (current && current.stateNode !== null) { + current.stateNode._fragmentFiber = finishedWork; + } if (flags & Ref) { safelyAttachRef(finishedWork, finishedWork.return); } From 5ad4e551d7919ab81651227bd25025a84f40c4d8 Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Fri, 28 Feb 2025 20:50:13 -0500 Subject: [PATCH 14/21] Fix event listener untracking --- .../src/client/ReactFiberConfigDOM.js | 33 ++++---- .../__tests__/ReactDOMFragmentRefs-test.js | 78 +++++++++++++++++++ 2 files changed, 95 insertions(+), 16 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index aa9d75f59cee2..9b55b389f3b7b 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -2162,25 +2162,26 @@ FragmentInstancePseudoElement.prototype.removeEventListener = function ( optionsOrUseCapture?: EventListenerOptionsOrUseCapture, ): void { const listeners = this._eventListeners; - if (listeners === undefined || listeners.length === 0) { - return; - } - traverseFragmentInstanceChildren( - this, - this._fragmentFiber.child, - removeEventListenerFromChild.bind( - null, + if (typeof listeners !== 'undefined' && listeners.length > 0) { + traverseFragmentInstanceChildren( + this, + this._fragmentFiber.child, + removeEventListenerFromChild.bind( + null, + type, + listener, + optionsOrUseCapture, + ), + ); + const index = indexOfEventListener(listeners, { type, listener, optionsOrUseCapture, - ), - ); - const index = indexOfEventListener(listeners, { - type, - listener, - optionsOrUseCapture, - }); - this._eventListeners = listeners.splice(index, 1); + }); + if (this._eventListeners !== undefined) { + this._eventListeners.splice(index, 1); + } + } }; function removeEventListenerFromChild( type: string, diff --git a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js index 9f7ec809c96e1..532c21a12cb20 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js @@ -412,6 +412,84 @@ describe('FragmentRefs', () => { expect(logs).toEqual(['Host A', 'Host B']); }); + // @gate enableFragmentRefs + it('allows adding and cleaning up listeners in effects', async () => { + const root = ReactDOMClient.createRoot(container); + + let logs = []; + function logClick(e) { + logs.push(e.currentTarget.id); + } + + let rerender; + let removeEventListeners; + + function Test() { + const fragmentRef = React.useRef(null); + // eslint-disable-next-line no-unused-vars + const [_, setState] = React.useState(0); + rerender = () => { + setState(p => p + 1); + }; + removeEventListeners = () => { + fragmentRef.current.removeEventListener('click', logClick); + }; + React.useEffect(() => { + fragmentRef.current.addEventListener('click', logClick); + + return removeEventListeners; + }); + + return ( + +
+ + ); + } + + // The event listener was applied + await act(() => root.render()); + expect(logs).toEqual([]); + document.querySelector('#child-a').click(); + expect(logs).toEqual(['child-a']); + + // The event listener can be removed and re-added + logs = []; + await act(rerender); + document.querySelector('#child-a').click(); + expect(logs).toEqual(['child-a']); + }); + + // @gate enableFragmentRefs + it('does not apply removed event listeners to new children', async () => { + const root = ReactDOMClient.createRoot(container); + const fragmentRef = React.createRef(null); + function Test() { + return ( + +
+ + ); + } + + let logs = []; + function logClick(e) { + logs.push(e.currentTarget.id); + } + await act(() => { + root.render(); + }); + fragmentRef.current.addEventListener('click', logClick); + const childA = document.querySelector('#child-a'); + childA.click(); + expect(logs).toEqual(['child-a']); + + logs = []; + fragmentRef.current.removeEventListener('click', logClick); + childA.click(); + expect(logs).toEqual([]); + }); + describe('with activity', () => { // @gate enableFragmentRefs && enableActivity it('does not apply event listeners to hidden trees', async () => { From 4eab01b67bf0b15abc15aab60431475a3915dcc4 Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Mon, 3 Mar 2025 10:01:18 -0500 Subject: [PATCH 15/21] Track new nodes in fragment instances from host placement flow --- .../react-reconciler/src/ReactFiberCommitHostEffects.js | 7 +++++++ packages/react-reconciler/src/ReactFiberCommitWork.js | 5 +---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitHostEffects.js b/packages/react-reconciler/src/ReactFiberCommitHostEffects.js index b00e76da46229..2e9303a963162 100644 --- a/packages/react-reconciler/src/ReactFiberCommitHostEffects.js +++ b/packages/react-reconciler/src/ReactFiberCommitHostEffects.js @@ -59,6 +59,7 @@ import {captureCommitPhaseError} from './ReactFiberWorkLoop'; import {trackHostMutation} from './ReactFiberMutationTracking'; import {runWithFiberInDEV} from './ReactCurrentFiber'; +import {enableFragmentRefs} from 'shared/ReactFeatureFlags'; export function commitHostMount(finishedWork: Fiber) { const type = finishedWork.type; @@ -355,6 +356,9 @@ function insertOrAppendPlacementNodeIntoContainer( } else { appendChildToContainer(parent, stateNode); } + if (enableFragmentRefs) { + commitFragmentInstanceInsertionEffects(node); + } trackHostMutation(); return; } else if (tag === HostPortal) { @@ -399,6 +403,9 @@ function insertOrAppendPlacementNode( } else { appendChild(parent, stateNode); } + if (enableFragmentRefs) { + commitFragmentInstanceInsertionEffects(node); + } trackHostMutation(); return; } else if (tag === HostPortal) { diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 3570f79b1a0bd..fbe96598da33b 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -237,8 +237,8 @@ import { commitHostRemoveChild, commitHostSingletonAcquisition, commitHostSingletonRelease, - commitFragmentInstanceInsertionEffects, commitFragmentInstanceDeletionEffects, + commitFragmentInstanceInsertionEffects, } from './ReactFiberCommitHostEffects'; import { commitEnterViewTransitions, @@ -2387,9 +2387,6 @@ function commitReconciliationEffects( const flags = finishedWork.flags; if (flags & Placement) { commitHostPlacement(finishedWork); - if (enableFragmentRefs && finishedWork.tag === HostComponent) { - commitFragmentInstanceInsertionEffects(finishedWork); - } // Clear the "placement" from effect tag so that we know that this is // inserted, before any life-cycles like componentDidMount gets called. // TODO: findDOMNode doesn't rely on this any more but isMounted does From 0a2d423c5fdde7b300589a5ce21a41d2ab7c6a01 Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Mon, 10 Mar 2025 12:16:44 -0400 Subject: [PATCH 16/21] Update test assertion --- packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js index 532c21a12cb20..727fd3014201f 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js @@ -66,7 +66,7 @@ describe('FragmentRefs', () => { ); }); - expect(fragmentRef.current).not.toEqual(null); + expect(fragmentRef._fragmentFiber).toBeTruthy(); }); // @gate enableFragmentRefs From bb9e7f0f41efbc6a0a24c05709b8a4d19d7914c7 Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Mon, 10 Mar 2025 15:11:20 -0400 Subject: [PATCH 17/21] Remove config references from reconciler, detach refs, cleanup --- packages/react-art/src/ReactFiberConfigART.js | 11 +++++----- .../src/client/ReactFiberConfigDOM.js | 7 +++++++ .../src/ReactFiberConfigFabric.js | 12 +++++++---- .../src/ReactFiberConfigNative.js | 12 +++++++---- .../src/ReactFiberCommitEffects.js | 2 +- .../src/ReactFiberCommitHostEffects.js | 19 +----------------- .../src/ReactFiberCommitWork.js | 20 +++++++++++++++++-- .../src/forks/ReactFiberConfig.custom.js | 4 +++- .../src/ReactFiberConfigTestHost.js | 12 +++++++---- 9 files changed, 60 insertions(+), 39 deletions(-) diff --git a/packages/react-art/src/ReactFiberConfigART.js b/packages/react-art/src/ReactFiberConfigART.js index 7afc61dd3b69e..f27f1d23aa96b 100644 --- a/packages/react-art/src/ReactFiberConfigART.js +++ b/packages/react-art/src/ReactFiberConfigART.js @@ -318,15 +318,16 @@ export function cloneMutableTextInstance(textInstance) { return textInstance; } -export type FragmentInstance = null | { - _fragmentFiber: Object, - ... -}; +export type FragmentInstance = null; -export function createFragmentInstance(parentInstance): null { +export function createFragmentInstance(fiber): null { return null; } +export function updateFragmentInstanceFiber(fiber, instance): void { + // Noop +} + export function commitNewChildToFragmentInstance( child, fragmentInstance, diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index 9b55b389f3b7b..80b0f851600bb 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -2260,6 +2260,13 @@ export function createFragmentInstance(fragmentFiber: Fiber): FragmentInstance { return new (FragmentInstancePseudoElement: any)(fragmentFiber); } +export function updateFragmentInstanceFiber( + fragmentFiber: Fiber, + instance: FragmentInstance, +): void { + instance._fragmentFiber = fragmentFiber; +} + export function commitNewChildToFragmentInstance( childElement: Element, fragmentInstance: FragmentInstance, diff --git a/packages/react-native-renderer/src/ReactFiberConfigFabric.js b/packages/react-native-renderer/src/ReactFiberConfigFabric.js index 0022ec57a620a..b1012771d8511 100644 --- a/packages/react-native-renderer/src/ReactFiberConfigFabric.js +++ b/packages/react-native-renderer/src/ReactFiberConfigFabric.js @@ -591,15 +591,19 @@ export function waitForCommitToBeReady(): null { return null; } -export type FragmentInstance = null | { - _fragmentFiber: Fiber, - ... -}; +export type FragmentInstance = null; export function createFragmentInstance(fragmentFiber: Fiber): FragmentInstance { return null; } +export function updateFragmentInstanceFiber( + fragmentFiber: Fiber, + instance: FragmentInstance, +): void { + // Noop +} + export function commitNewChildToFragmentInstance( child: PublicInstance, fragmentInstance: FragmentInstance, diff --git a/packages/react-native-renderer/src/ReactFiberConfigNative.js b/packages/react-native-renderer/src/ReactFiberConfigNative.js index 092d58cc8b412..b432bde3a2607 100644 --- a/packages/react-native-renderer/src/ReactFiberConfigNative.js +++ b/packages/react-native-renderer/src/ReactFiberConfigNative.js @@ -202,15 +202,19 @@ export function cloneMutableTextInstance( throw new Error('Not yet implemented.'); } -export type FragmentInstance = null | { - _fragmentFiber: Fiber, - ... -}; +export type FragmentInstance = null; export function createFragmentInstance(fragmentFiber: Fiber): FragmentInstance { return null; } +export function updateFragmentInstanceFiber( + fragmentFiber: Fiber, + instance: FragmentInstance, +): void { + // Noop +} + export function commitNewChildToFragmentInstance( child: PublicInstance, fragmentInstance: FragmentInstance, diff --git a/packages/react-reconciler/src/ReactFiberCommitEffects.js b/packages/react-reconciler/src/ReactFiberCommitEffects.js index 563b117a054b9..391ed32046574 100644 --- a/packages/react-reconciler/src/ReactFiberCommitEffects.js +++ b/packages/react-reconciler/src/ReactFiberCommitEffects.js @@ -898,7 +898,7 @@ function commitAttachRef(finishedWork: Fiber) { case Fragment: if (enableFragmentRefs) { const instance: null | FragmentInstance = finishedWork.stateNode; - if (instance === null || instance._fragmentFiber !== finishedWork) { + if (instance === null) { finishedWork.stateNode = createFragmentInstance(finishedWork); } instanceToUse = finishedWork.stateNode; diff --git a/packages/react-reconciler/src/ReactFiberCommitHostEffects.js b/packages/react-reconciler/src/ReactFiberCommitHostEffects.js index 2e9303a963162..1c7b9c9b01d80 100644 --- a/packages/react-reconciler/src/ReactFiberCommitHostEffects.js +++ b/packages/react-reconciler/src/ReactFiberCommitHostEffects.js @@ -204,7 +204,7 @@ export function commitShowHideHostTextInstance(node: Fiber, isHidden: boolean) { } } -export function getHostParentFiber(fiber: Fiber): Fiber { +function getHostParentFiber(fiber: Fiber): Fiber { let parent = fiber.return; while (parent !== null) { if (isHostParent(parent)) { @@ -236,23 +236,6 @@ export function commitFragmentInstanceInsertionEffects(fiber: Fiber): void { } export function commitFragmentInstanceDeletionEffects(fiber: Fiber): void { - commitEffectsForFragmentInstanceParents(fiber); - let parent = fiber.return; - while (parent !== null) { - if (isFragmentInstanceParent(parent)) { - const fragmentInstance: FragmentInstance = parent.stateNode; - deleteChildFromFragmentInstance(fiber.stateNode, fragmentInstance); - } - - if (isHostParent(parent)) { - return; - } - - parent = parent.return; - } -} - -function commitEffectsForFragmentInstanceParents(fiber: Fiber): void { let parent = fiber.return; while (parent !== null) { if (isFragmentInstanceParent(parent)) { diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index fbe96598da33b..970b4da5dc1b9 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -166,6 +166,7 @@ import { cancelRootViewTransitionName, restoreRootViewTransitionName, isSingletonScope, + updateFragmentInstanceFiber, } from './ReactFiberConfig'; import { captureCommitPhaseError, @@ -1364,7 +1365,7 @@ function commitDeletionEffectsOnFiber( if (!offscreenSubtreeWasHidden) { safelyDetachRef(deletedFiber, nearestMountedAncestor); } - if (enableFragmentRefs) { + if (enableFragmentRefs && deletedFiber.tag === HostComponent) { commitFragmentInstanceDeletionEffects(deletedFiber); } // Intentional fallthrough to next branch @@ -1577,6 +1578,14 @@ function commitDeletionEffectsOnFiber( } break; } + case Fragment: { + if (enableFragmentRefs) { + if (!offscreenSubtreeWasHidden) { + safelyDetachRef(deletedFiber, nearestMountedAncestor); + } + } + // Fallthrough + } default: { recursivelyTraverseDeletionEffects( finishedRoot, @@ -2340,7 +2349,7 @@ function commitMutationEffectsOnFiber( case Fragment: if (enableFragmentRefs) { if (current && current.stateNode !== null) { - current.stateNode._fragmentFiber = finishedWork; + updateFragmentInstanceFiber(finishedWork, current.stateNode); } if (flags & Ref) { safelyAttachRef(finishedWork, finishedWork.return); @@ -2688,6 +2697,13 @@ export function disappearLayoutEffects(finishedWork: Fiber) { if (enableViewTransition) { safelyDetachRef(finishedWork, finishedWork.return); } + recursivelyTraverseDisappearLayoutEffects(finishedWork); + break; + } + case Fragment: { + if (enableFragmentRefs) { + safelyDetachRef(finishedWork, finishedWork.return); + } // Fallthrough } default: { diff --git a/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js b/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js index d45afd09da974..fba91377887a5 100644 --- a/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js +++ b/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js @@ -45,7 +45,7 @@ export type ViewTransitionInstance = null | {name: string, ...}; export opaque type InstanceMeasurement = mixed; export type EventResponder = any; export type GestureTimeline = any; -export type FragmentInstance = null | {_fragmentFiber: Object, ...}; +export type FragmentInstance = null; export const rendererVersion = $$$config.rendererVersion; export const rendererPackageName = $$$config.rendererPackageName; @@ -162,6 +162,8 @@ export const createViewTransitionInstance = $$$config.createViewTransitionInstance; export const clearContainer = $$$config.clearContainer; export const createFragmentInstance = $$$config.createFragmentInstance; +export const updateFragmentInstanceFiber = + $$$config.updateFragmentInstanceFiber; export const commitNewChildToFragmentInstance = $$$config.commitNewChildToFragmentInstance; export const deleteChildFromFragmentInstance = diff --git a/packages/react-test-renderer/src/ReactFiberConfigTestHost.js b/packages/react-test-renderer/src/ReactFiberConfigTestHost.js index 3796eff01c5eb..de2cd1f1927ac 100644 --- a/packages/react-test-renderer/src/ReactFiberConfigTestHost.js +++ b/packages/react-test-renderer/src/ReactFiberConfigTestHost.js @@ -447,10 +447,7 @@ export function createViewTransitionInstance( return null; } -export type FragmentInstance = null | { - _fragmentFiber: Object, - ... -}; +export type FragmentInstance = null; export function createFragmentInstance( fragmentFiber: Object, @@ -458,6 +455,13 @@ export function createFragmentInstance( return null; } +export function updateFragmentInstanceFiber( + fragmentFiber: Object, + instance: FragmentInstance, +): void { + // Noop +} + export function commitNewChildToFragmentInstance( child: Instance, fragmentInstance: FragmentInstance, From ed883ab03c071ba12ac5230da7eb47415232145b Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Mon, 10 Mar 2025 17:52:28 -0400 Subject: [PATCH 18/21] Address additional feedback --- .../src/client/ReactFiberConfigDOM.js | 115 ++++++++++-------- .../src/ReactFiberCommitEffects.js | 4 +- .../src/ReactFiberCommitHostEffects.js | 17 ++- .../src/ReactFiberCommitWork.js | 14 ++- 4 files changed, 88 insertions(+), 62 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index 80b0f851600bb..efbaebe2ab06c 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -2093,9 +2093,9 @@ type StoredEventListener = { optionsOrUseCapture: void | EventListenerOptionsOrUseCapture, }; -export type FragmentInstance = { +export type FragmentInstanceType = { _fragmentFiber: Fiber, - _eventListeners: void | Array, + _eventListeners: null | Array, addEventListener( type: string, listener: EventListener, @@ -2109,20 +2109,18 @@ export type FragmentInstance = { focus(): void, }; -function FragmentInstancePseudoElement( - this: FragmentInstance, - fragmentFiber: Fiber, -) { +function FragmentInstance(this: FragmentInstanceType, fragmentFiber: Fiber) { this._fragmentFiber = fragmentFiber; + this._eventListeners = null; } // $FlowFixMe[prop-missing] -FragmentInstancePseudoElement.prototype.addEventListener = function ( - this: FragmentInstance, +FragmentInstance.prototype.addEventListener = function ( + this: FragmentInstanceType, type: string, listener: EventListener, optionsOrUseCapture?: EventListenerOptionsOrUseCapture, ): void { - if (!this._eventListeners) { + if (this._eventListeners === null) { this._eventListeners = []; } @@ -2131,70 +2129,71 @@ FragmentInstancePseudoElement.prototype.addEventListener = function ( // need to collect the listeners to apply to appended children, we track them ourselves and use // custom equality check for the options. const isNewEventListener = - indexOfEventListener(listeners, { - type, - listener, - optionsOrUseCapture, - }) === -1; + indexOfEventListener(listeners, type, listener, optionsOrUseCapture) === -1; if (isNewEventListener) { listeners.push({type, listener, optionsOrUseCapture}); traverseFragmentInstanceChildren( this, this._fragmentFiber.child, - addEventListenerToChild.bind(null, type, listener, optionsOrUseCapture), + addEventListenerToChild, + type, + listener, + optionsOrUseCapture, ); } this._eventListeners = listeners; }; function addEventListenerToChild( + child: Instance, type: string, listener: EventListener, optionsOrUseCapture?: EventListenerOptionsOrUseCapture, - child: Element, -): void { +): boolean { child.addEventListener(type, listener, optionsOrUseCapture); + return false; } // $FlowFixMe[prop-missing] -FragmentInstancePseudoElement.prototype.removeEventListener = function ( - this: FragmentInstance, +FragmentInstance.prototype.removeEventListener = function ( + this: FragmentInstanceType, type: string, listener: EventListener, optionsOrUseCapture?: EventListenerOptionsOrUseCapture, ): void { const listeners = this._eventListeners; + if (listeners === null) { + return; + } if (typeof listeners !== 'undefined' && listeners.length > 0) { traverseFragmentInstanceChildren( this, this._fragmentFiber.child, - removeEventListenerFromChild.bind( - null, - type, - listener, - optionsOrUseCapture, - ), + removeEventListenerFromChild, + type, + listener, + optionsOrUseCapture, ); - const index = indexOfEventListener(listeners, { + const index = indexOfEventListener( + listeners, type, listener, optionsOrUseCapture, - }); - if (this._eventListeners !== undefined) { + ); + if (this._eventListeners !== null) { this._eventListeners.splice(index, 1); } } }; function removeEventListenerFromChild( + child: Instance, type: string, listener: EventListener, optionsOrUseCapture?: EventListenerOptionsOrUseCapture, - child: Element, -): void { +): boolean { child.removeEventListener(type, listener, optionsOrUseCapture); + return false; } // $FlowFixMe[prop-missing] -FragmentInstancePseudoElement.prototype.focus = function ( - this: FragmentInstance, -) { +FragmentInstance.prototype.focus = function (this: FragmentInstanceType) { traverseFragmentInstanceChildren( this, this._fragmentFiber.child, @@ -2202,14 +2201,17 @@ FragmentInstancePseudoElement.prototype.focus = function ( ); }; -function traverseFragmentInstanceChildren( - fragmentInstance: FragmentInstance, +function traverseFragmentInstanceChildren( + fragmentInstance: FragmentInstanceType, child: Fiber | null, - fn: (child: Element) => boolean | void, + fn: (Instance, A, B, C) => boolean, + a: A, + b: B, + c: C, ): void { while (child !== null) { if (child.tag === HostComponent) { - if (fn(child.stateNode)) { + if (fn(child.stateNode, a, b, c)) { return; } } else if ( @@ -2218,7 +2220,14 @@ function traverseFragmentInstanceChildren( ) { // Skip hidden subtrees } else { - traverseFragmentInstanceChildren(fragmentInstance, child.child, fn); + traverseFragmentInstanceChildren( + fragmentInstance, + child.child, + fn, + a, + b, + c, + ); } child = child.sibling; } @@ -2240,15 +2249,17 @@ function normalizeListenerOptions( function indexOfEventListener( eventListeners: Array, - eventListenerToFind: StoredEventListener, + type: string, + listener: EventListener, + optionsOrUseCapture: void | EventListenerOptionsOrUseCapture, ): number { for (let i = 0; i < eventListeners.length; i++) { const item = eventListeners[i]; if ( - item.type === eventListenerToFind.type && - item.listener === eventListenerToFind.listener && + item.type === type && + item.listener === listener && normalizeListenerOptions(item.optionsOrUseCapture) === - normalizeListenerOptions(eventListenerToFind.optionsOrUseCapture) + normalizeListenerOptions(optionsOrUseCapture) ) { return i; } @@ -2256,23 +2267,25 @@ function indexOfEventListener( return -1; } -export function createFragmentInstance(fragmentFiber: Fiber): FragmentInstance { - return new (FragmentInstancePseudoElement: any)(fragmentFiber); +export function createFragmentInstance( + fragmentFiber: Fiber, +): FragmentInstanceType { + return new (FragmentInstance: any)(fragmentFiber); } export function updateFragmentInstanceFiber( fragmentFiber: Fiber, - instance: FragmentInstance, + instance: FragmentInstanceType, ): void { instance._fragmentFiber = fragmentFiber; } export function commitNewChildToFragmentInstance( - childElement: Element, - fragmentInstance: FragmentInstance, + childElement: Instance, + fragmentInstance: FragmentInstanceType, ): void { const eventListeners = fragmentInstance._eventListeners; - if (eventListeners !== undefined) { + if (eventListeners !== null) { for (let i = 0; i < eventListeners.length; i++) { const {type, listener, optionsOrUseCapture} = eventListeners[i]; childElement.addEventListener(type, listener, optionsOrUseCapture); @@ -2281,11 +2294,11 @@ export function commitNewChildToFragmentInstance( } export function deleteChildFromFragmentInstance( - childElement: Element, - fragmentInstance: FragmentInstance, + childElement: Instance, + fragmentInstance: FragmentInstanceType, ): void { const eventListeners = fragmentInstance._eventListeners; - if (eventListeners !== undefined) { + if (eventListeners !== null) { for (let i = 0; i < eventListeners.length; i++) { const {type, listener, optionsOrUseCapture} = eventListeners[i]; childElement.removeEventListener(type, listener, optionsOrUseCapture); diff --git a/packages/react-reconciler/src/ReactFiberCommitEffects.js b/packages/react-reconciler/src/ReactFiberCommitEffects.js index 391ed32046574..a0f6b54780a2d 100644 --- a/packages/react-reconciler/src/ReactFiberCommitEffects.js +++ b/packages/react-reconciler/src/ReactFiberCommitEffects.js @@ -11,7 +11,7 @@ import type {Fiber} from './ReactInternalTypes'; import type {UpdateQueue} from './ReactFiberClassUpdateQueue'; import type {FunctionComponentUpdateQueue} from './ReactFiberHooks'; import type {HookFlags} from './ReactHookEffectTags'; -import type {FragmentInstance} from './ReactFiberConfig'; +import type {FragmentInstanceType} from './ReactFiberConfig'; import { getViewTransitionName, type ViewTransitionState, @@ -897,7 +897,7 @@ function commitAttachRef(finishedWork: Fiber) { } case Fragment: if (enableFragmentRefs) { - const instance: null | FragmentInstance = finishedWork.stateNode; + const instance: null | FragmentInstanceType = finishedWork.stateNode; if (instance === null) { finishedWork.stateNode = createFragmentInstance(finishedWork); } diff --git a/packages/react-reconciler/src/ReactFiberCommitHostEffects.js b/packages/react-reconciler/src/ReactFiberCommitHostEffects.js index 1c7b9c9b01d80..953b3d0fc72bc 100644 --- a/packages/react-reconciler/src/ReactFiberCommitHostEffects.js +++ b/packages/react-reconciler/src/ReactFiberCommitHostEffects.js @@ -13,7 +13,7 @@ import type { SuspenseInstance, Container, ChildSet, - FragmentInstance, + FragmentInstanceType, } from './ReactFiberConfig'; import type {Fiber, FiberRoot} from './ReactInternalTypes'; @@ -223,7 +223,7 @@ export function commitFragmentInstanceInsertionEffects(fiber: Fiber): void { let parent = fiber.return; while (parent !== null) { if (isFragmentInstanceParent(parent)) { - const fragmentInstance: FragmentInstance = parent.stateNode; + const fragmentInstance: FragmentInstanceType = parent.stateNode; commitNewChildToFragmentInstance(fiber.stateNode, fragmentInstance); } @@ -239,7 +239,7 @@ export function commitFragmentInstanceDeletionEffects(fiber: Fiber): void { let parent = fiber.return; while (parent !== null) { if (isFragmentInstanceParent(parent)) { - const fragmentInstance: FragmentInstance = parent.stateNode; + const fragmentInstance: FragmentInstanceType = parent.stateNode; deleteChildFromFragmentInstance(fiber.stateNode, fragmentInstance); } @@ -339,7 +339,13 @@ function insertOrAppendPlacementNodeIntoContainer( } else { appendChildToContainer(parent, stateNode); } - if (enableFragmentRefs) { + // TODO: Enable HostText for RN + if ( + enableFragmentRefs && + tag === HostComponent && + // Only run fragment insertion effects for initial insertions + node.alternate === null + ) { commitFragmentInstanceInsertionEffects(node); } trackHostMutation(); @@ -386,7 +392,8 @@ function insertOrAppendPlacementNode( } else { appendChild(parent, stateNode); } - if (enableFragmentRefs) { + // TODO: Enable HostText for RN + if (enableFragmentRefs && tag === HostComponent) { commitFragmentInstanceInsertionEffects(node); } trackHostMutation(); diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 970b4da5dc1b9..22f1c54587fe8 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -772,7 +772,7 @@ function commitLayoutEffectOnFiber( } break; } - // Fallthrough + break; } case Fragment: if (enableFragmentRefs) { @@ -2351,9 +2351,6 @@ function commitMutationEffectsOnFiber( if (current && current.stateNode !== null) { updateFragmentInstanceFiber(finishedWork, current.stateNode); } - if (flags & Ref) { - safelyAttachRef(finishedWork, finishedWork.return); - } } // Fallthrough default: { @@ -2811,6 +2808,7 @@ export function reappearLayoutEffects( } case HostHoistable: case HostComponent: { + // TODO: Enable HostText for RN if (enableFragmentRefs && finishedWork.tag === HostComponent) { commitFragmentInstanceInsertionEffects(finishedWork); } @@ -2906,6 +2904,14 @@ export function reappearLayoutEffects( safelyAttachRef(finishedWork, finishedWork.return); break; } + break; + } + case Fragment: { + if (enableFragmentRefs) { + if (flags & Ref) { + safelyAttachRef(finishedWork, finishedWork.return); + } + } // Fallthrough } default: { From f69d0c0d1dbb52a9ef8dc1cc87813721c0914696 Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Tue, 11 Mar 2025 09:43:54 -0400 Subject: [PATCH 19/21] Unify getHostParentFiber and commitFragmentInstanceInsertionEffects loops --- .../src/ReactFiberCommitHostEffects.js | 117 +++++++++++++----- 1 file changed, 87 insertions(+), 30 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitHostEffects.js b/packages/react-reconciler/src/ReactFiberCommitHostEffects.js index 953b3d0fc72bc..7dad8b330d7e2 100644 --- a/packages/react-reconciler/src/ReactFiberCommitHostEffects.js +++ b/packages/react-reconciler/src/ReactFiberCommitHostEffects.js @@ -204,19 +204,14 @@ export function commitShowHideHostTextInstance(node: Fiber, isHidden: boolean) { } } -function getHostParentFiber(fiber: Fiber): Fiber { - let parent = fiber.return; - while (parent !== null) { - if (isHostParent(parent)) { - return parent; - } - parent = parent.return; +export function commitNewChildToFragmentInstances( + fiber: Fiber, + parentFragmentInstances: Array, +): void { + for (let i = 0; i < parentFragmentInstances.length; i++) { + const fragmentInstance = parentFragmentInstances[i]; + commitNewChildToFragmentInstance(fiber.stateNode, fragmentInstance); } - - throw new Error( - 'Expected to find a host parent. This error is likely caused by a bug ' + - 'in React. Please file an issue.', - ); } export function commitFragmentInstanceInsertionEffects(fiber: Fiber): void { @@ -329,6 +324,7 @@ function insertOrAppendPlacementNodeIntoContainer( node: Fiber, before: ?Instance, parent: Container, + parentFragmentInstances: null | Array, ): void { const {tag} = node; const isHost = tag === HostComponent || tag === HostText; @@ -344,9 +340,10 @@ function insertOrAppendPlacementNodeIntoContainer( enableFragmentRefs && tag === HostComponent && // Only run fragment insertion effects for initial insertions - node.alternate === null + node.alternate === null && + parentFragmentInstances !== null ) { - commitFragmentInstanceInsertionEffects(node); + commitNewChildToFragmentInstances(node, parentFragmentInstances); } trackHostMutation(); return; @@ -369,10 +366,20 @@ function insertOrAppendPlacementNodeIntoContainer( const child = node.child; if (child !== null) { - insertOrAppendPlacementNodeIntoContainer(child, before, parent); + insertOrAppendPlacementNodeIntoContainer( + child, + before, + parent, + parentFragmentInstances, + ); let sibling = child.sibling; while (sibling !== null) { - insertOrAppendPlacementNodeIntoContainer(sibling, before, parent); + insertOrAppendPlacementNodeIntoContainer( + sibling, + before, + parent, + parentFragmentInstances, + ); sibling = sibling.sibling; } } @@ -382,6 +389,7 @@ function insertOrAppendPlacementNode( node: Fiber, before: ?Instance, parent: Instance, + parentFragmentInstances: null | Array, ): void { const {tag} = node; const isHost = tag === HostComponent || tag === HostText; @@ -393,8 +401,14 @@ function insertOrAppendPlacementNode( appendChild(parent, stateNode); } // TODO: Enable HostText for RN - if (enableFragmentRefs && tag === HostComponent) { - commitFragmentInstanceInsertionEffects(node); + if ( + enableFragmentRefs && + tag === HostComponent && + // Only run fragment insertion effects for initial insertions + node.alternate === null && + parentFragmentInstances !== null + ) { + commitNewChildToFragmentInstances(node, parentFragmentInstances); } trackHostMutation(); return; @@ -416,10 +430,15 @@ function insertOrAppendPlacementNode( const child = node.child; if (child !== null) { - insertOrAppendPlacementNode(child, before, parent); + insertOrAppendPlacementNode(child, before, parent, parentFragmentInstances); let sibling = child.sibling; while (sibling !== null) { - insertOrAppendPlacementNode(sibling, before, parent); + insertOrAppendPlacementNode( + sibling, + before, + parent, + parentFragmentInstances, + ); sibling = sibling.sibling; } } @@ -431,40 +450,78 @@ function commitPlacement(finishedWork: Fiber): void { } // Recursively insert all host nodes into the parent. - const parentFiber = getHostParentFiber(finishedWork); + let hostParentFiber; + let parentFragmentInstances = null; + let parentFiber = finishedWork.return; + while (parentFiber !== null) { + if (enableFragmentRefs && isFragmentInstanceParent(parentFiber)) { + const fragmentInstance: FragmentInstanceType = parentFiber.stateNode; + if (parentFragmentInstances === null) { + parentFragmentInstances = [fragmentInstance]; + } else { + parentFragmentInstances.push(fragmentInstance); + } + } + if (isHostParent(parentFiber)) { + hostParentFiber = parentFiber; + break; + } + parentFiber = parentFiber.return; + } + if (hostParentFiber == null) { + throw new Error( + 'Expected to find a host parent. This error is likely caused by a bug ' + + 'in React. Please file an issue.', + ); + } - switch (parentFiber.tag) { + switch (hostParentFiber.tag) { case HostSingleton: { if (supportsSingletons) { - const parent: Instance = parentFiber.stateNode; + const parent: Instance = hostParentFiber.stateNode; const before = getHostSibling(finishedWork); // We only have the top Fiber that was inserted but we need to recurse down its // children to find all the terminal nodes. - insertOrAppendPlacementNode(finishedWork, before, parent); + insertOrAppendPlacementNode( + finishedWork, + before, + parent, + parentFragmentInstances, + ); break; } // Fall through } case HostComponent: { - const parent: Instance = parentFiber.stateNode; - if (parentFiber.flags & ContentReset) { + const parent: Instance = hostParentFiber.stateNode; + if (hostParentFiber.flags & ContentReset) { // Reset the text content of the parent before doing any insertions resetTextContent(parent); // Clear ContentReset from the effect tag - parentFiber.flags &= ~ContentReset; + hostParentFiber.flags &= ~ContentReset; } const before = getHostSibling(finishedWork); // We only have the top Fiber that was inserted but we need to recurse down its // children to find all the terminal nodes. - insertOrAppendPlacementNode(finishedWork, before, parent); + insertOrAppendPlacementNode( + finishedWork, + before, + parent, + parentFragmentInstances, + ); break; } case HostRoot: case HostPortal: { - const parent: Container = parentFiber.stateNode.containerInfo; + const parent: Container = hostParentFiber.stateNode.containerInfo; const before = getHostSibling(finishedWork); - insertOrAppendPlacementNodeIntoContainer(finishedWork, before, parent); + insertOrAppendPlacementNodeIntoContainer( + finishedWork, + before, + parent, + parentFragmentInstances, + ); break; } default: From 33d65eeb6576441e283ddd46cccb60a1a574c8c5 Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Tue, 11 Mar 2025 12:26:12 -0400 Subject: [PATCH 20/21] Fix types --- packages/react-art/src/ReactFiberConfigART.js | 2 +- .../src/ReactFiberConfigFabric.js | 12 +++++++----- .../src/ReactFiberConfigNative.js | 12 +++++++----- .../src/forks/ReactFiberConfig.custom.js | 2 +- .../src/ReactFiberConfigTestHost.js | 10 +++++----- 5 files changed, 21 insertions(+), 17 deletions(-) diff --git a/packages/react-art/src/ReactFiberConfigART.js b/packages/react-art/src/ReactFiberConfigART.js index f27f1d23aa96b..5b8788453f6af 100644 --- a/packages/react-art/src/ReactFiberConfigART.js +++ b/packages/react-art/src/ReactFiberConfigART.js @@ -318,7 +318,7 @@ export function cloneMutableTextInstance(textInstance) { return textInstance; } -export type FragmentInstance = null; +export type FragmentInstanceType = null; export function createFragmentInstance(fiber): null { return null; diff --git a/packages/react-native-renderer/src/ReactFiberConfigFabric.js b/packages/react-native-renderer/src/ReactFiberConfigFabric.js index b1012771d8511..c3b8ac4d35d14 100644 --- a/packages/react-native-renderer/src/ReactFiberConfigFabric.js +++ b/packages/react-native-renderer/src/ReactFiberConfigFabric.js @@ -591,29 +591,31 @@ export function waitForCommitToBeReady(): null { return null; } -export type FragmentInstance = null; +export type FragmentInstanceType = null; -export function createFragmentInstance(fragmentFiber: Fiber): FragmentInstance { +export function createFragmentInstance( + fragmentFiber: Fiber, +): FragmentInstanceType { return null; } export function updateFragmentInstanceFiber( fragmentFiber: Fiber, - instance: FragmentInstance, + instance: FragmentInstanceType, ): void { // Noop } export function commitNewChildToFragmentInstance( child: PublicInstance, - fragmentInstance: FragmentInstance, + fragmentInstance: FragmentInstanceType, ): void { // Noop } export function deleteChildFromFragmentInstance( child: PublicInstance, - fragmentInstance: FragmentInstance, + fragmentInstance: FragmentInstanceType, ): void { // Noop } diff --git a/packages/react-native-renderer/src/ReactFiberConfigNative.js b/packages/react-native-renderer/src/ReactFiberConfigNative.js index b432bde3a2607..b5f07a77b24c2 100644 --- a/packages/react-native-renderer/src/ReactFiberConfigNative.js +++ b/packages/react-native-renderer/src/ReactFiberConfigNative.js @@ -202,29 +202,31 @@ export function cloneMutableTextInstance( throw new Error('Not yet implemented.'); } -export type FragmentInstance = null; +export type FragmentInstanceType = null; -export function createFragmentInstance(fragmentFiber: Fiber): FragmentInstance { +export function createFragmentInstance( + fragmentFiber: Fiber, +): FragmentInstanceType { return null; } export function updateFragmentInstanceFiber( fragmentFiber: Fiber, - instance: FragmentInstance, + instance: FragmentInstanceType, ): void { // Noop } export function commitNewChildToFragmentInstance( child: PublicInstance, - fragmentInstance: FragmentInstance, + fragmentInstance: FragmentInstanceType, ): void { // Noop } export function deleteChildFromFragmentInstance( child: PublicInstance, - fragmentInstance: FragmentInstance, + fragmentInstance: FragmentInstanceType, ): void { // Noop } diff --git a/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js b/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js index fba91377887a5..2be7d18b87caf 100644 --- a/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js +++ b/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js @@ -45,7 +45,7 @@ export type ViewTransitionInstance = null | {name: string, ...}; export opaque type InstanceMeasurement = mixed; export type EventResponder = any; export type GestureTimeline = any; -export type FragmentInstance = null; +export type FragmentInstanceType = null; export const rendererVersion = $$$config.rendererVersion; export const rendererPackageName = $$$config.rendererPackageName; diff --git a/packages/react-test-renderer/src/ReactFiberConfigTestHost.js b/packages/react-test-renderer/src/ReactFiberConfigTestHost.js index de2cd1f1927ac..6ac2777c92662 100644 --- a/packages/react-test-renderer/src/ReactFiberConfigTestHost.js +++ b/packages/react-test-renderer/src/ReactFiberConfigTestHost.js @@ -447,31 +447,31 @@ export function createViewTransitionInstance( return null; } -export type FragmentInstance = null; +export type FragmentInstanceType = null; export function createFragmentInstance( fragmentFiber: Object, -): FragmentInstance { +): FragmentInstanceType { return null; } export function updateFragmentInstanceFiber( fragmentFiber: Object, - instance: FragmentInstance, + instance: FragmentInstanceType, ): void { // Noop } export function commitNewChildToFragmentInstance( child: Instance, - fragmentInstance: FragmentInstance, + fragmentInstance: FragmentInstanceType, ): void { // noop } export function deleteChildFromFragmentInstance( child: Instance, - fragmentInstance: FragmentInstance, + fragmentInstance: FragmentInstanceType, ): void { // Noop } From b46ae582d24f76c4f5fd82963a6a2569709d9b3f Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Wed, 12 Mar 2025 10:05:29 -0400 Subject: [PATCH 21/21] Remove ref flag check in reappearLayoutEffects --- packages/react-reconciler/src/ReactFiberCommitWork.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 22f1c54587fe8..8b82e6a7e713c 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -2908,9 +2908,7 @@ export function reappearLayoutEffects( } case Fragment: { if (enableFragmentRefs) { - if (flags & Ref) { - safelyAttachRef(finishedWork, finishedWork.return); - } + safelyAttachRef(finishedWork, finishedWork.return); } // Fallthrough }