From 652a5001e1953b336860f85fbfa7b2c2b81e49a1 Mon Sep 17 00:00:00 2001 From: Ron Nakamoto <14256602+ronnakamoto@users.noreply.github.com> Date: Sun, 26 Oct 2025 18:20:47 +0530 Subject: [PATCH] fix: prevent metadata hoisting in hidden Activity boundaries --- .../src/__tests__/ReactDOMFloat-test.js | 183 ++++++++++++++++++ .../src/ReactFiberCommitWork.js | 127 ++++++++++-- .../src/ReactFiberCompleteWork.js | 7 + .../react-reconciler/src/ReactFiberFlags.js | 7 +- 4 files changed, 312 insertions(+), 12 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js index 848fdd65219..8f814cca7cc 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js @@ -9424,5 +9424,188 @@ background-color: green; another title, ); }); + + // @gate enableActivity + it('does not hoist title tags inside hidden Activity boundaries', async () => { + const Activity = React.Activity; + const root = ReactDOMClient.createRoot(container); + + await act(() => { + root.render( +
+ + Visible Title + + + Hidden Title + +
, + ); + }); + await waitForAll([]); + + // Only the visible Activity's title should be hoisted + expect(getMeaningfulChildren(document.head)).toEqual( + Visible Title, + ); + }); + + // @gate enableActivity + it('removes title tags when Activity transitions from visible to hidden', async () => { + const Activity = React.Activity; + const root = ReactDOMClient.createRoot(container); + + await act(() => { + root.render( +
+ + Activity Title + +
, + ); + }); + await waitForAll([]); + + // Title should be hoisted + expect(getMeaningfulChildren(document.head)).toEqual( + Activity Title, + ); + + // Hide the Activity + await act(() => { + root.render( +
+ + Activity Title + +
, + ); + }); + await waitForAll([]); + + // Title should be removed from document head + expect(getMeaningfulChildren(document.head)).toEqual(undefined); + }); + + // @gate enableActivity + it('adds title tags when Activity transitions from hidden to visible', async () => { + const Activity = React.Activity; + const root = ReactDOMClient.createRoot(container); + + await act(() => { + root.render( +
+ + Activity Title + +
, + ); + }); + await waitForAll([]); + + // Title should not be hoisted + expect(getMeaningfulChildren(document.head)).toEqual(undefined); + + // Show the Activity + await act(() => { + root.render( +
+ + Activity Title + +
, + ); + }); + await waitForAll([]); + + // Title should now be hoisted + // Note: The title may have a style attribute from being previously hidden + const titleElement = getMeaningfulChildren(document.head); + expect(titleElement).toBeTruthy(); + expect(titleElement.type).toBe('title'); + expect(titleElement.props.children).toBe('Activity Title'); + }); + + // @gate enableActivity + it('handles multiple Activity boundaries with different visibility states', async () => { + const Activity = React.Activity; + const root = ReactDOMClient.createRoot(container); + + await act(() => { + root.render( +
+ + First Title + + + Second Title + + + Third Title + +
, + ); + }); + await waitForAll([]); + + // Only visible Activities' titles should be hoisted + // Both visible titles are hoisted, but the last one in tree order wins + const titles = getMeaningfulChildren(document.head); + expect(Array.isArray(titles)).toBe(true); + expect(titles.length).toBe(2); + expect(titles[0].props.children).toBe('Third Title'); + expect(titles[1].props.children).toBe('First Title'); + }); + + // @gate enableActivity + it('handles nested Activity boundaries correctly', async () => { + const Activity = React.Activity; + const root = ReactDOMClient.createRoot(container); + + await act(() => { + root.render( +
+ + Outer Title + + Inner Hidden Title + + +
, + ); + }); + await waitForAll([]); + + // Only the outer visible Activity's title should be hoisted + // The inner hidden Activity's title should not be hoisted + expect(getMeaningfulChildren(document.head)).toEqual( + Outer Title, + ); + }); + + // @gate enableActivity + it('handles meta tags inside hidden Activity boundaries', async () => { + const Activity = React.Activity; + const root = ReactDOMClient.createRoot(container); + + await act(() => { + root.render( +
+ + + + + + +
, + ); + }); + await waitForAll([]); + + // Only the visible Activity's meta should be hoisted + expect(getMeaningfulChildren(document.head)).toEqual( + , + ); + }); }); }); diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index adff6647f6d..9461341ce03 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -116,6 +116,7 @@ import { ForceClientRender, DidCapture, AffectedParentLayout, + HoistableStatic, ViewTransitionNamedStatic, } from './ReactFiberFlags'; import { @@ -1412,6 +1413,71 @@ function commitDeletionEffects( detachFiberMutation(deletedFiber); } +// Helper function to recursively mount hoistables when an Activity becomes visible +function recursivelyMountHoistablesInSubtree( + hoistableRoot: HoistableRoot, + fiber: Fiber, +): void { + if (supportsResources) { + // Traverse the fiber tree and mount any hoistables + let child = fiber.child; + while (child !== null) { + if (child.tag === HostHoistable) { + // This is a hoistable element + if (child.memoizedState === null && child.stateNode !== null) { + // This is a Hoistable Instance (not a Resource) + // Mount it to the document head + mountHoistable(hoistableRoot, child.type, child.stateNode); + } + } else if ( + child.tag !== OffscreenComponent && + child.tag !== ActivityComponent + ) { + // Don't traverse into nested Offscreen/Activity boundaries + // They manage their own hoistables + + // Check if this subtree contains any hoistables before traversing. + // The HoistableStatic flag is set during the complete phase and bubbled up + // through subtreeFlags, allowing us to skip entire subtrees that don't + // contain any hoistables. + if ((child.subtreeFlags & HoistableStatic) !== NoFlags) { + recursivelyMountHoistablesInSubtree(hoistableRoot, child); + } + } + child = child.sibling; + } + } +} + +// Helper function to recursively unmount hoistables when an Activity becomes hidden +function recursivelyUnmountHoistablesInSubtree(fiber: Fiber): void { + if (supportsResources) { + // Traverse the fiber tree and unmount any hoistables + let child = fiber.child; + while (child !== null) { + if (child.tag === HostHoistable) { + // This is a hoistable element + if (child.memoizedState === null && child.stateNode !== null) { + // This is a Hoistable Instance (not a Resource) + // Only unmount if it was actually mounted to the document + const instance = child.stateNode; + if (instance.parentNode) { + unmountHoistable(instance); + } + } + } else if ( + child.tag !== OffscreenComponent && + child.tag !== ActivityComponent + ) { + if ((child.subtreeFlags & HoistableStatic) !== NoFlags) { + recursivelyUnmountHoistablesInSubtree(child); + } + } + child = child.sibling; + } + } +} + function recursivelyTraverseDeletionEffects( finishedRoot: FiberRoot, nearestMountedAncestor: Fiber, @@ -1455,7 +1521,12 @@ function commitDeletionEffectsOnFiber( if (deletedFiber.memoizedState) { releaseResource(deletedFiber.memoizedState); } else if (deletedFiber.stateNode) { - unmountHoistable(deletedFiber.stateNode); + // Only unmount if the hoistable was actually mounted to the document + // Hoistables in hidden Activity boundaries are never mounted + const instance = deletedFiber.stateNode; + if (instance.parentNode) { + unmountHoistable(instance); + } } break; } @@ -2076,11 +2147,15 @@ function commitMutationEffectsOnFiber( finishedWork, ); } else { - mountHoistable( - hoistableRoot, - finishedWork.type, - finishedWork.stateNode, - ); + // Only mount the hoistable if we're not in a hidden offscreen subtree + // Hidden Activity boundaries should not hoist their metadata to the document + if (!offscreenSubtreeIsHidden) { + mountHoistable( + hoistableRoot, + finishedWork.type, + finishedWork.stateNode, + ); + } } } else { finishedWork.stateNode = acquireResource( @@ -2099,11 +2174,14 @@ function commitMutationEffectsOnFiber( releaseResource(currentResource); } if (newResource === null) { - mountHoistable( - hoistableRoot, - finishedWork.type, - finishedWork.stateNode, - ); + // Only mount the hoistable if we're not in a hidden offscreen subtree + if (!offscreenSubtreeIsHidden) { + mountHoistable( + hoistableRoot, + finishedWork.type, + finishedWork.stateNode, + ); + } } else { acquireResource( hoistableRoot, @@ -2511,6 +2589,33 @@ function commitMutationEffectsOnFiber( ); } } + + // Unmount hoistables when transitioning from visible to hidden + // This ensures that metadata like is removed from the document + // when an Activity becomes hidden + if (supportsResources) { + recursivelyUnmountHoistablesInSubtree(finishedWork); + } + } + } else { + // Becoming visible + // Only mount hoistables if: + // - This is an update (not first mount, which is handled elsewhere) + // - This Offscreen was hidden before + // - No ancestor Offscreen is hidden + if ( + isUpdate && + wasHidden && + !offscreenSubtreeIsHidden && + !offscreenSubtreeWasHidden + ) { + // Mount hoistables when transitioning from hidden to visible + // This ensures that metadata like <title> is added to the document + // when an Activity becomes visible + if (supportsResources) { + const hoistableRoot: HoistableRoot = (currentHoistableRoot: any); + recursivelyMountHoistablesInSubtree(hoistableRoot, finishedWork); + } } } diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index d3abcb466e5..17c56942696 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -99,6 +99,7 @@ import { Cloned, ViewTransitionStatic, Hydrate, + HoistableStatic, } from './ReactFiberFlags'; import { @@ -1177,6 +1178,10 @@ function completeWork( // @TODO refactor this block to create the instance here in complete // phase if we are not hydrating. markUpdate(workInProgress); + // Mark this fiber as containing a hoistable. This flag will bubble up + // through subtreeFlags, allowing us to skip traversals in subtrees that + // don't contain any hoistables during Activity visibility transitions. + workInProgress.flags |= HoistableStatic; if (nextResource !== null) { // This is a Hoistable Resource @@ -1205,6 +1210,8 @@ function completeWork( } } else { // This is an update. + // Mark this fiber as containing a hoistable. + workInProgress.flags |= HoistableStatic; if (nextResource) { // This is a Resource if (nextResource !== current.memoizedState) { diff --git a/packages/react-reconciler/src/ReactFiberFlags.js b/packages/react-reconciler/src/ReactFiberFlags.js index e44301d4ed2..c93d27f8811 100644 --- a/packages/react-reconciler/src/ReactFiberFlags.js +++ b/packages/react-reconciler/src/ReactFiberFlags.js @@ -74,6 +74,10 @@ export const LayoutStatic = /* */ 0b0000000010000000000000000000 export const RefStatic = LayoutStatic; export const PassiveStatic = /* */ 0b0000000100000000000000000000000; export const MaySuspendCommit = /* */ 0b0000001000000000000000000000000; +// HoistableStatic tracks whether there are any HostHoistable fibers in the subtree. +// This enables us to skip traversals during Activity visibility transitions when +// there are no hoistables to mount/unmount. +export const HoistableStatic = /* */ 0b0000010000000000000000000000000; // ViewTransitionNamedStatic tracks explicitly name ViewTransition components deeply // that might need to be visited during clean up. This is similar to SnapshotStatic // if there was any other use for it. It also needs to run in the same phase as @@ -82,7 +86,7 @@ export const ViewTransitionNamedStatic = /* */ SnapshotStatic | MaySuspendCommit; // ViewTransitionStatic tracks whether there are an ViewTransition components from // the nearest HostComponent down. It resets at every HostComponent level. -export const ViewTransitionStatic = /* */ 0b0000010000000000000000000000000; +export const ViewTransitionStatic = /* */ 0b0000100000000000000000000000000; // Flag used to identify newly inserted fibers. It isn't reset after commit unlike `Placement`. export const PlacementDEV = /* */ 0b0000100000000000000000000000000; @@ -138,5 +142,6 @@ export const StaticMask = PassiveStatic | RefStatic | MaySuspendCommit | + HoistableStatic | ViewTransitionStatic | ViewTransitionNamedStatic;