From 83c88ad470d680060f807ef81ed4c14b3b71fd3b Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Tue, 23 Sep 2025 10:56:43 -0400 Subject: [PATCH 1/3] Handle fabric root level fragment with compareDocumentPosition (#34533) The root instance doesn't have a canonical property so we were not returning a public instance that we can call compareDocumentPosition on when a Fragment had no other host parent in Fabric. In this case we need to get the ReactNativeElement from the ReactNativeDocument. I've also added test coverage for this case in DOM for consistency, though it was already working there because we use DOM elements as root. This same test will be copied to RN using Fantom. --- .../__tests__/ReactDOMFragmentRefs-test.js | 110 ++++++++++++++++++ .../src/ReactFiberConfigFabric.js | 17 ++- 2 files changed, 121 insertions(+), 6 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js index 2ac6cb0a4b603..26de45076f359 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js @@ -1613,6 +1613,116 @@ describe('FragmentRefs', () => { ); }); + // @gate enableFragmentRefs + it('compares a root-level Fragment', async () => { + const fragmentRef = React.createRef(); + const emptyFragmentRef = React.createRef(); + const childRef = React.createRef(); + const siblingPrecedingRef = React.createRef(); + const siblingFollowingRef = React.createRef(); + const root = ReactDOMClient.createRoot(container); + + function Test() { + return ( + +
+ +
+ + +
+ + ); + } + + await act(() => root.render()); + + const fragmentInstance = fragmentRef.current; + if (fragmentInstance == null) { + throw new Error('Expected fragment instance to be non-null'); + } + const emptyFragmentInstance = emptyFragmentRef.current; + if (emptyFragmentInstance == null) { + throw new Error('Expected empty fragment instance to be non-null'); + } + + expectPosition( + fragmentInstance.compareDocumentPosition(childRef.current), + { + preceding: false, + following: false, + contains: false, + containedBy: true, + disconnected: false, + implementationSpecific: false, + }, + ); + + expectPosition( + fragmentInstance.compareDocumentPosition(siblingPrecedingRef.current), + { + preceding: true, + following: false, + contains: false, + containedBy: false, + disconnected: false, + implementationSpecific: false, + }, + ); + + expectPosition( + fragmentInstance.compareDocumentPosition(siblingFollowingRef.current), + { + preceding: false, + following: true, + contains: false, + containedBy: false, + disconnected: false, + implementationSpecific: false, + }, + ); + + expectPosition( + emptyFragmentInstance.compareDocumentPosition(childRef.current), + { + preceding: true, + following: false, + contains: false, + containedBy: false, + disconnected: false, + implementationSpecific: true, + }, + ); + + expectPosition( + emptyFragmentInstance.compareDocumentPosition( + siblingPrecedingRef.current, + ), + { + preceding: true, + following: false, + contains: false, + containedBy: false, + disconnected: false, + implementationSpecific: true, + }, + ); + + expectPosition( + emptyFragmentInstance.compareDocumentPosition( + siblingFollowingRef.current, + ), + { + preceding: false, + following: true, + contains: false, + containedBy: false, + disconnected: false, + implementationSpecific: true, + }, + ); + }); + describe('with portals', () => { // @gate enableFragmentRefs it('handles portaled elements', async () => { diff --git a/packages/react-native-renderer/src/ReactFiberConfigFabric.js b/packages/react-native-renderer/src/ReactFiberConfigFabric.js index c030b8528b62c..40ff26ff89be0 100644 --- a/packages/react-native-renderer/src/ReactFiberConfigFabric.js +++ b/packages/react-native-renderer/src/ReactFiberConfigFabric.js @@ -25,7 +25,6 @@ import type {Fiber} from 'react-reconciler/src/ReactInternalTypes'; import {HostText} from 'react-reconciler/src/ReactWorkTags'; import { getFragmentParentHostFiber, - getInstanceFromHostFiber, traverseFragmentInstance, } from 'react-reconciler/src/ReactFiberTreeReflection'; @@ -303,6 +302,13 @@ export function getPublicInstance(instance: Instance): null | PublicInstance { return instance.canonical.publicInstance; } + // Handle root containers + if (instance.containerInfo != null) { + if (instance.containerInfo.publicInstance != null) { + return instance.containerInfo.publicInstance; + } + } + // For compatibility with the legacy renderer, in case it's used with Fabric // in the same app. // $FlowExpectedError[prop-missing] @@ -347,9 +353,8 @@ export function getPublicInstanceFromInternalInstanceHandle( } function getPublicInstanceFromHostFiber(fiber: Fiber): PublicInstance { - const instance = getInstanceFromHostFiber(fiber); - const publicInstance = getPublicInstance(instance); - if (publicInstance == null) { + const publicInstance = getPublicInstance(fiber.stateNode); + if (publicInstance === null) { throw new Error('Expected to find a host node. This is a bug in React.'); } return publicInstance; @@ -698,11 +703,11 @@ FragmentInstance.prototype.compareDocumentPosition = function ( if (parentHostFiber === null) { return Node.DOCUMENT_POSITION_DISCONNECTED; } - const parentHostInstance = getPublicInstanceFromHostFiber(parentHostFiber); const children: Array = []; traverseFragmentInstance(this._fragmentFiber, collectChildren, children); if (children.length === 0) { - return compareDocumentPositionForEmptyFragment( + const parentHostInstance = getPublicInstanceFromHostFiber(parentHostFiber); + return compareDocumentPositionForEmptyFragment( this._fragmentFiber, parentHostInstance, otherNode, From 012b371cde3157a8dd46535839fbcda6c2bed1a0 Mon Sep 17 00:00:00 2001 From: "Sebastian \"Sebbie\" Silbermann" Date: Tue, 23 Sep 2025 20:14:53 +0200 Subject: [PATCH 2/3] [DevTools] Handle LegacyHidden Fibers like Offscreen Fibers. (#34564) --- .../src/__tests__/profilingCache-test.js | 7 +- .../profilingCommitTreeBuilder-test.js | 68 +++++-------------- .../src/__tests__/store-test.js | 4 +- .../src/backend/fiber/renderer.js | 40 ++++++----- 4 files changed, 45 insertions(+), 74 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/profilingCache-test.js b/packages/react-devtools-shared/src/__tests__/profilingCache-test.js index 0d26d7107cb6d..428b22d466d79 100644 --- a/packages/react-devtools-shared/src/__tests__/profilingCache-test.js +++ b/packages/react-devtools-shared/src/__tests__/profilingCache-test.js @@ -725,14 +725,14 @@ describe('ProfilingCache', () => { const commitData = store.profilerStore.getDataForRoot(rootID).commitData; expect(commitData).toHaveLength(2); - const isLegacySuspense = React.version.startsWith('17'); - if (isLegacySuspense) { + if (React.version.startsWith('17')) { + // React 17 will mount all children until it suspends in a LegacyHidden + // The ID gap is from the Fiber for that's in the disconnected tree. expect(commitData[0].fiberActualDurations).toMatchInlineSnapshot(` Map { 1 => 15, 2 => 15, 3 => 5, - 4 => 3, 5 => 2, } `); @@ -741,7 +741,6 @@ describe('ProfilingCache', () => { 1 => 0, 2 => 10, 3 => 3, - 4 => 3, 5 => 2, } `); diff --git a/packages/react-devtools-shared/src/__tests__/profilingCommitTreeBuilder-test.js b/packages/react-devtools-shared/src/__tests__/profilingCommitTreeBuilder-test.js index 176056cf10e63..968fda10dab5f 100644 --- a/packages/react-devtools-shared/src/__tests__/profilingCommitTreeBuilder-test.js +++ b/packages/react-devtools-shared/src/__tests__/profilingCommitTreeBuilder-test.js @@ -19,8 +19,6 @@ describe('commit tree', () => { let Scheduler; let store: Store; let utils; - const isLegacySuspense = - React.version.startsWith('16') || React.version.startsWith('17'); beforeEach(() => { utils = require('./utils'); @@ -186,24 +184,13 @@ describe('commit tree', () => { utils.act(() => store.profilerStore.startProfiling()); utils.act(() => legacyRender()); await Promise.resolve(); - if (isLegacySuspense) { - expect(store).toMatchInlineSnapshot(` - [root] - ▾ - ▾ - - [suspense-root] rects={null} - - `); - } else { - expect(store).toMatchInlineSnapshot(` - [root] - ▾ - - [suspense-root] rects={null} - - `); - } + expect(store).toMatchInlineSnapshot(` + [root] + ▾ + + [suspense-root] rects={null} + + `); utils.act(() => legacyRender()); expect(store).toMatchInlineSnapshot(` [root] @@ -231,13 +218,7 @@ describe('commit tree', () => { ); } - expect(commitTrees[0].nodes.size).toBe( - isLegacySuspense - ? // + + + - 4 - : // + + - 3, - ); + expect(commitTrees[0].nodes.size).toBe(3); expect(commitTrees[1].nodes.size).toBe(4); // + + + expect(commitTrees[2].nodes.size).toBe(2); // + }); @@ -291,24 +272,13 @@ describe('commit tree', () => { it('should support Lazy components that are unmounted before resolving (legacy render)', async () => { utils.act(() => store.profilerStore.startProfiling()); utils.act(() => legacyRender()); - if (isLegacySuspense) { - expect(store).toMatchInlineSnapshot(` - [root] - ▾ - ▾ - - [suspense-root] rects={null} - - `); - } else { - expect(store).toMatchInlineSnapshot(` - [root] - ▾ - - [suspense-root] rects={null} - - `); - } + expect(store).toMatchInlineSnapshot(` + [root] + ▾ + + [suspense-root] rects={null} + + `); utils.act(() => legacyRender()); expect(store).toMatchInlineSnapshot(` [root] @@ -327,13 +297,7 @@ describe('commit tree', () => { ); } - expect(commitTrees[0].nodes.size).toBe( - isLegacySuspense - ? // + + + - 4 - : // + + - 3, - ); + expect(commitTrees[0].nodes.size).toBe(3); expect(commitTrees[1].nodes.size).toBe(2); // + }); diff --git a/packages/react-devtools-shared/src/__tests__/store-test.js b/packages/react-devtools-shared/src/__tests__/store-test.js index a2024be3d2e5e..4d4d5b6affc4f 100644 --- a/packages/react-devtools-shared/src/__tests__/store-test.js +++ b/packages/react-devtools-shared/src/__tests__/store-test.js @@ -2828,7 +2828,7 @@ describe('Store', () => { `); }); - // @reactVersion >= 18.0 + // @reactVersion >= 17.0 it('can reconcile Suspense in fallback positions', async () => { let resolveFallback; const fallbackPromise = new Promise(resolve => { @@ -2907,7 +2907,7 @@ describe('Store', () => { `); }); - // @reactVersion >= 18.0 + // @reactVersion >= 17.0 it('can reconcile resuspended Suspense with Suspense in fallback positions', async () => { let resolveHeadFallback; let resolveHeadContent; diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index 33786a41877b8..39179bc6f472f 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -460,10 +460,10 @@ export function getInternalReactConstants(version: string): { IncompleteFunctionComponent: 28, IndeterminateComponent: 2, // removed in 19.0.0 LazyComponent: 16, - LegacyHiddenComponent: 23, + LegacyHiddenComponent: 23, // Does not exist in 18+ OSS but exists in fb builds MemoComponent: 14, Mode: 8, - OffscreenComponent: 22, // Experimental + OffscreenComponent: 22, // Experimental in 17. Stable in 18+ Profiler: 12, ScopeComponent: 21, // Experimental SimpleMemoComponent: 15, @@ -3057,13 +3057,23 @@ export function attach( } } + function isHiddenOffscreen(fiber: Fiber): boolean { + switch (fiber.tag) { + case LegacyHiddenComponent: + // fallthrough since all published implementations currently implement the same state as Offscreen. + case OffscreenComponent: + return fiber.memoizedState !== null; + default: + return false; + } + } + function unmountRemainingChildren() { if ( reconcilingParent !== null && (reconcilingParent.kind === FIBER_INSTANCE || reconcilingParent.kind === FILTERED_FIBER_INSTANCE) && - reconcilingParent.data.tag === OffscreenComponent && - reconcilingParent.data.memoizedState !== null && + isHiddenOffscreen(reconcilingParent.data) && !isInDisconnectedSubtree ) { // This is a hidden offscreen, we need to execute this in the context of a disconnected subtree. @@ -3170,8 +3180,7 @@ export function attach( if ( (parent.kind === FIBER_INSTANCE || parent.kind === FILTERED_FIBER_INSTANCE) && - parent.data.tag === OffscreenComponent && - parent.data.memoizedState !== null + isHiddenOffscreen(parent.data) ) { // We're inside a hidden offscreen Fiber. We're in a disconnected tree. return; @@ -3819,7 +3828,9 @@ export function attach( (reconcilingParent !== null && reconcilingParent.kind === VIRTUAL_INSTANCE) || fiber.tag === SuspenseComponent || - fiber.tag === OffscreenComponent // Use to keep resuspended instances alive inside a SuspenseComponent. + // Use to keep resuspended instances alive inside a SuspenseComponent. + fiber.tag === OffscreenComponent || + fiber.tag === LegacyHiddenComponent ) { // If the parent is a Virtual Instance and we filtered this Fiber we include a // hidden node. We also include this if it's a Suspense boundary so we can track those @@ -3939,7 +3950,7 @@ export function attach( trackDebugInfoFromHostComponent(nearestInstance, fiber); } - if (fiber.tag === OffscreenComponent && fiber.memoizedState !== null) { + if (isHiddenOffscreen(fiber)) { // If an Offscreen component is hidden, mount its children as disconnected. const stashedDisconnected = isInDisconnectedSubtree; isInDisconnectedSubtree = true; @@ -4261,7 +4272,7 @@ export function attach( while (child !== null) { if (child.kind === FILTERED_FIBER_INSTANCE) { const fiber = child.data; - if (fiber.tag === OffscreenComponent && fiber.memoizedState !== null) { + if (isHiddenOffscreen(fiber)) { // The children of this Offscreen are hidden so they don't get added. } else { addUnfilteredChildrenIDs(child, nextChildren); @@ -4888,9 +4899,8 @@ export function attach( const nextDidTimeOut = isLegacySuspense && nextFiber.memoizedState !== null; - const isOffscreen = nextFiber.tag === OffscreenComponent; - const prevWasHidden = isOffscreen && prevFiber.memoizedState !== null; - const nextIsHidden = isOffscreen && nextFiber.memoizedState !== null; + const prevWasHidden = isHiddenOffscreen(prevFiber); + const nextIsHidden = isHiddenOffscreen(nextFiber); if (isLegacySuspense) { if ( @@ -5245,8 +5255,7 @@ export function attach( if ( (child.kind === FIBER_INSTANCE || child.kind === FILTERED_FIBER_INSTANCE) && - child.data.tag === OffscreenComponent && - child.data.memoizedState !== null + isHiddenOffscreen(child.data) ) { // This instance's children are already disconnected. } else { @@ -5275,8 +5284,7 @@ export function attach( if ( (child.kind === FIBER_INSTANCE || child.kind === FILTERED_FIBER_INSTANCE) && - child.data.tag === OffscreenComponent && - child.data.memoizedState !== null + isHiddenOffscreen(child.data) ) { // This instance's children should remain disconnected. } else { From 24a2ba03fb2e1b59844d98a1ce68dce1e502d8ad Mon Sep 17 00:00:00 2001 From: Ruslan Lesiutin <28902667+hoxyq@users.noreply.github.com> Date: Tue, 23 Sep 2025 11:38:07 -0700 Subject: [PATCH 3/3] [DevTools] fix: dedupe file fetch requests and define a timeout (#34566) If there is a large owner stack, we could potentially spam multiple fetch requests for the same source map. This adds a simple deduplication logic, based on URL. Also, this adds a timeout of 60 seconds to all fetch requests initiated by fileFetcher content script. --- .../src/contentScripts/fileFetcher.js | 2 +- .../src/main/fetchFileWithCaching.js | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/packages/react-devtools-extensions/src/contentScripts/fileFetcher.js b/packages/react-devtools-extensions/src/contentScripts/fileFetcher.js index 198b4f68cd301..78663acc74fdf 100644 --- a/packages/react-devtools-extensions/src/contentScripts/fileFetcher.js +++ b/packages/react-devtools-extensions/src/contentScripts/fileFetcher.js @@ -23,7 +23,7 @@ function fetchResource(url) { }); }; - fetch(url, {cache: 'force-cache'}).then( + fetch(url, {cache: 'force-cache', signal: AbortSignal.timeout(60000)}).then( response => { if (response.ok) { response diff --git a/packages/react-devtools-extensions/src/main/fetchFileWithCaching.js b/packages/react-devtools-extensions/src/main/fetchFileWithCaching.js index 15028e99094e1..f8863372ff93b 100644 --- a/packages/react-devtools-extensions/src/main/fetchFileWithCaching.js +++ b/packages/react-devtools-extensions/src/main/fetchFileWithCaching.js @@ -78,6 +78,18 @@ const fetchFromNetworkCache = (url, resolve, reject) => { }); }; +const pendingFetchRequests = new Set(); +function pendingFetchRequestsCleanup({payload, source}) { + if (source === 'react-devtools-background') { + switch (payload?.type) { + case 'fetch-file-with-cache-complete': + case 'fetch-file-with-cache-error': + pendingFetchRequests.delete(payload.url); + } + } +} +chrome.runtime.onMessage.addListener(pendingFetchRequestsCleanup); + const fetchFromPage = async (url, resolve, reject) => { debugLog('[main] fetchFromPage()', url); @@ -97,7 +109,11 @@ const fetchFromPage = async (url, resolve, reject) => { } chrome.runtime.onMessage.addListener(onPortMessage); + if (pendingFetchRequests.has(url)) { + return; + } + pendingFetchRequests.add(url); chrome.runtime.sendMessage({ source: 'devtools-page', payload: {