From bfb7a76884b4ec54b9e29ddc7a0b7e4993d5ecea Mon Sep 17 00:00:00 2001 From: ronnakamoto <14256602+ronnakamoto@users.noreply.github.com> Date: Tue, 11 Aug 2026 15:10:26 +0530 Subject: [PATCH] [Fiber] Prevent metadata hoisting in hidden `` trees (#34983) Co-authored-by: Amp Co-authored-by: Sebastian Sebbie Silbermann --- .../src/__tests__/ReactDOMFloat-test.js | 402 ++++++++++++++++++ .../src/__tests__/ReactRenderDocument-test.js | 87 ++++ .../src/ReactFiberBeginWork.js | 7 + .../src/ReactFiberCommitWork.js | 156 ++++++- 4 files changed, 635 insertions(+), 17 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js index c25717d7b279..1289f0a5a026 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js @@ -9770,6 +9770,408 @@ background-color: green; another title, ); }); + + 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, + ); + }); + + 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); + }); + + 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 + // The title retains an empty style attribute from being previously hidden + expect(getMeaningfulChildren(document.head)).toEqual( + Activity Title, + ); + }); + + 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 + expect(getMeaningfulChildren(document.head)).toEqual([ + Third Title, + First Title, + ]); + }); + + 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, + ); + }); + + 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( + , + ); + }); + + it('does not hoist a hoistable nested under a HostComponent inside a hidden Activity', async () => { + const Activity = React.Activity; + const root = ReactDOMClient.createRoot(container); + + await act(() => { + root.render( + +
+ Nested Hidden Title + +
+ , + ); + }); + await waitForAll([]); + + expect(getMeaningfulChildren(document.head)).toEqual(undefined); + }); + + it('mounts nested hoistables when their ancestor Activity transitions to visible', async () => { + const Activity = React.Activity; + const root = ReactDOMClient.createRoot(container); + + await act(() => { + root.render( + +
+ Reveal Me +
+ , + ); + }); + await waitForAll([]); + expect(getMeaningfulChildren(document.head)).toEqual(undefined); + + await act(() => { + root.render( + +
+ Reveal Me +
+ , + ); + }); + await waitForAll([]); + + expect(getMeaningfulChildren(document.head)).toEqual( + Reveal Me, + ); + }); + + it('updates a hidden title without inserting it into the head', async () => { + const Activity = React.Activity; + const root = ReactDOMClient.createRoot(container); + + await act(() => { + root.render( + + Original Hidden + , + ); + }); + await waitForAll([]); + expect(getMeaningfulChildren(document.head)).toEqual(undefined); + + // Update the prop while still hidden — the head must remain empty. + await act(() => { + root.render( + + Updated Hidden + , + ); + }); + await waitForAll([]); + expect(getMeaningfulChildren(document.head)).toEqual(undefined); + + // Now reveal — the latest text should be in the head. + await act(() => { + root.render( + + Updated Hidden + , + ); + }); + await waitForAll([]); + + expect(getMeaningfulChildren(document.head)).toEqual( + Updated Hidden, + ); + }); + + it('removes a previously-mounted title when its Activity is deleted while hidden', async () => { + const Activity = React.Activity; + const root = ReactDOMClient.createRoot(container); + + await act(() => { + root.render( + + To Be Deleted + , + ); + }); + await waitForAll([]); + expect(getMeaningfulChildren(document.head)).toEqual( + To Be Deleted, + ); + + // Hide first — this unmounts from the head. + await act(() => { + root.render( + + To Be Deleted + , + ); + }); + await waitForAll([]); + expect(getMeaningfulChildren(document.head)).toEqual(undefined); + + // Delete the entire Activity while it's hidden. This must not throw + // (the deletion path needs to tolerate already-detached instances). + await act(() => { + root.render(
); + }); + await waitForAll([]); + expect(getMeaningfulChildren(document.head)).toEqual(undefined); + }); + + it('does not hoist hidden Activity metadata during hydration', async () => { + const Activity = React.Activity; + + // SSR a page where the only title belongs to a VISIBLE Activity. The + // hidden Activity has its own that must not end up in <head>. + await act(() => { + const {pipe} = renderToPipeableStream( + <html> + <body> + <Activity mode="visible"> + <title>Visible Title + + + Hidden Title + + + , + ); + pipe(writable); + }); + + // After SSR, only the visible title should be in the head. + expect(getMeaningfulChildren(document.head)).toEqual( + Visible Title, + ); + + // Hydrate. The hidden Activity's must not be inserted into the + // head as a side effect of hydration. + ReactDOMClient.hydrateRoot( + document, + <html> + <body> + <Activity mode="visible"> + <title>Visible Title + + + Hidden Title + + + , + ); + await waitForAll([]); + + expect(getMeaningfulChildren(document.head)).toEqual( + Visible Title, + ); + }); + + it('handles StrictMode without leaving duplicate or missing hoistables', async () => { + const Activity = React.Activity; + const root = ReactDOMClient.createRoot(container); + + // StrictMode triggers a dev-only double invoke of layout effects, which + // exercises our disappear/reappear hoistable handling. The final state + // must be a single visible title and no hidden title. + await act(() => { + root.render( + +
+ + StrictMode Visible + + + StrictMode Hidden + +
+ , + ); + }); + await waitForAll([]); + + expect(getMeaningfulChildren(document.head)).toEqual( + StrictMode Visible, + ); + + // Toggle visibility — double invoke must still leave a clean head + // with exactly one title (no duplicates, no missing). + await act(() => { + root.render( + +
+ + StrictMode Visible + + + StrictMode Hidden + +
+ , + ); + }); + await waitForAll([]); + + // The previously-hidden title that just became visible carries a + // style="" attribute from hideInstance/unhideInstance. + expect(getMeaningfulChildren(document.head)).toEqual( + StrictMode Hidden, + ); + }); }); it('does not outline a boundary with suspensey CSS when flushing the shell', async () => { diff --git a/packages/react-dom/src/__tests__/ReactRenderDocument-test.js b/packages/react-dom/src/__tests__/ReactRenderDocument-test.js index 583643449f78..f9ca744ebc69 100644 --- a/packages/react-dom/src/__tests__/ReactRenderDocument-test.js +++ b/packages/react-dom/src/__tests__/ReactRenderDocument-test.js @@ -248,6 +248,93 @@ describe('rendering React components at document', () => { expect(container.textContent).toBe('parsnip'); }); + it('removes hoisted when hiding an Activity boundary', async () => { + const Activity = React.Activity; + + function App({mode, titleText}) { + return ( + <html> + <head> + <Activity mode={mode}> + <title>{titleText} + + + Hello + + ); + } + + const testDocument = getTestDocument( + '', + ); + const root = ReactDOMClient.createRoot(testDocument); + + await act(() => root.render()); + expect(testDocument.head.querySelector('title').textContent).toBe('A'); + + await act(() => root.render()); + expect(testDocument.head.querySelector('title')).toBe(null); + + await act(() => root.render()); + expect(testDocument.head.querySelector('title').textContent).toBe('B'); + }); + + it('does not unmount a hoistable that was never mounted when reappearing', async () => { + const Activity = React.Activity; + + function App({mode, showTitle}) { + return ( + + + + {showTitle ? Title : null} + + + Hello + + ); + } + + const testDocument = getTestDocument( + '', + ); + const root = ReactDOMClient.createRoot(testDocument); + + await act(() => root.render()); + expect(testDocument.head.querySelector('title')).toBe(null); + + await act(() => root.render()); + expect(testDocument.head.querySelector('title')).toBe(null); + }); + + it('removes hoistables deleted in the same commit that hides an Activity', async () => { + const Activity = React.Activity; + + function App({mode, showTitle}) { + return ( + + + + {showTitle ? Title : null} + + + Hello + + ); + } + + const testDocument = getTestDocument( + '', + ); + const root = ReactDOMClient.createRoot(testDocument); + + await act(() => root.render()); + expect(testDocument.head.querySelector('title')).not.toBe(null); + + await act(() => root.render()); + expect(testDocument.head.querySelector('title')).toBe(null); + }); + it('should give helpful errors on state desync', async () => { class Component extends React.Component { render() { diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 29cb4f21baa6..a325b9b946ba 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -1168,6 +1168,13 @@ function updateActivityComponent( renderLanes, ); workInProgress.lanes = laneToLanes(OffscreenLane); + // This tree hasn't been mounted yet so there are no baseLanes to carry over. + const nextState: OffscreenState = { + baseLanes: NoLanes, + cachePool: null, + }; + primaryChildFragment.memoizedState = nextState; + return bailoutOffscreenComponent(null, primaryChildFragment); } else { // We must push the suspense handler context *before* attempting to diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 2357bc038281..4faf501fb449 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -169,6 +169,7 @@ import { acquireResource, releaseResource, hydrateHoistable, + createHoistableInstance, mountHoistable, unmountHoistable, prepareToCommitHoistables, @@ -1515,7 +1516,13 @@ function commitDeletionEffectsOnFiber( if (deletedFiber.memoizedState) { releaseResource(deletedFiber.memoizedState); } else if (deletedFiber.stateNode) { - unmountHoistable(deletedFiber.stateNode); + // A Hoistable Instance lives in document.head only when its enclosing + // Activity is visible. If the Activity is hidden (or has been hidden + // since mount), the instance was either never inserted or was + // detached by the disappear traversal. Skip in those cases. + if (!offscreenSubtreeWasHidden) { + unmountHoistable(deletedFiber.stateNode); + } } break; } @@ -2157,13 +2164,32 @@ function commitMutationEffectsOnFiber( // or a Hoistable Resource if (newResource === null) { if (finishedWork.stateNode === null) { - finishedWork.stateNode = hydrateHoistable( - hoistableRoot, - finishedWork.type, - finishedWork.memoizedProps, - finishedWork, - ); - } else { + // Initial mount. The instance has not been created yet, which + // happens during hydration (createHoistableInstance is normally + // called in beginWork's updateHostHoistable, but is skipped + // when hydrating). + if (offscreenSubtreeIsHidden) { + // We're inside a hidden Activity boundary. Create the + // instance off-document so we don't leak metadata into + // the head. It will be mounted by the reappear path when + // the Activity becomes visible. + finishedWork.stateNode = createHoistableInstance( + finishedWork.type, + finishedWork.memoizedProps, + root.containerInfo, + finishedWork, + ); + } else { + finishedWork.stateNode = hydrateHoistable( + hoistableRoot, + finishedWork.type, + finishedWork.memoizedProps, + finishedWork, + ); + } + } else if (!offscreenSubtreeIsHidden) { + // The instance was created in beginWork. Only mount it into + // the document if we're not inside a hidden Activity boundary. mountHoistable( hoistableRoot, finishedWork.type, @@ -2180,18 +2206,27 @@ function commitMutationEffectsOnFiber( } else if (currentResource !== newResource) { // We are moving to or from Hoistable Resource, or between different Hoistable Resources if (currentResource === null) { - if (current.stateNode !== null) { - unmountHoistable(current.stateNode); + // Transitioning from Instance to Resource. Only unmount when the + // Instance is currently mounted in the document; hidden Activity + // boundaries keep instances off-document or detach them before + // this update is processed. + const instance = current.stateNode; + if (instance !== null && !offscreenSubtreeWasHidden) { + unmountHoistable(instance); } } else { releaseResource(currentResource); } if (newResource === null) { - mountHoistable( - hoistableRoot, - finishedWork.type, - finishedWork.stateNode, - ); + // Transitioning to an Instance. Only mount if visible; hidden + // Activity boundaries will mount via the reappear path. + if (!offscreenSubtreeIsHidden) { + mountHoistable( + hoistableRoot, + finishedWork.type, + finishedWork.stateNode, + ); + } } else { acquireResource( hoistableRoot, @@ -2605,6 +2640,16 @@ function commitMutationEffectsOnFiber( } else { layoutEffectTraversalFlags = NoLayoutEffectTraversalFlags; } + const newOffscreenSubtreeIsHidden = + // $FlowFixMe[constant-condition] + isHidden || offscreenSubtreeIsHidden; + const newOffscreenSubtreeWasHidden = + // $FlowFixMe[constant-condition] + wasHidden || offscreenSubtreeWasHidden; + const prevOffscreenSubtreeIsHidden = offscreenSubtreeIsHidden; + const prevOffscreenSubtreeWasHidden = offscreenSubtreeWasHidden; + offscreenSubtreeIsHidden = newOffscreenSubtreeIsHidden; + offscreenSubtreeWasHidden = newOffscreenSubtreeWasHidden; recursivelyTraverseDisappearLayoutEffects( finishedWork, layoutEffectTraversalFlags, @@ -2625,6 +2670,8 @@ function commitMutationEffectsOnFiber( componentEffectEndTime, ); } + offscreenSubtreeIsHidden = prevOffscreenSubtreeIsHidden; + offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden; } } } @@ -3097,7 +3144,6 @@ function disappearLayoutEffects( } // Expected fallthrough to HostComponent } - case HostHoistable: case HostComponent: { // TODO (Offscreen) Check: flags & RefStatic safelyDetachRef(finishedWork, finishedWork.return); @@ -3117,6 +3163,32 @@ function disappearLayoutEffects( ); break; } + case HostHoistable: { + // TODO (Offscreen) Check: flags & RefStatic + safelyDetachRef(finishedWork, finishedWork.return); + + // $FlowFixMe[constant-condition] + if (supportsResources) { + // We only act on Hoistable Instances (memoizedState === null). + // Resources (memoizedState !== null) are ref-counted and intentionally + // remain in the document across Activity visibility transitions; + // they are released only on actual deletion. + const instance = finishedWork.stateNode; + if ( + finishedWork.memoizedState === null && + instance !== null && + !offscreenSubtreeWasHidden + ) { + unmountHoistable(instance); + } + } + + recursivelyTraverseDisappearLayoutEffects( + finishedWork, + layoutEffectTraversalFlags, + ); + break; + } case OffscreenComponent: { const isHidden = finishedWork.memoizedState !== null; if (isHidden) { @@ -3288,7 +3360,6 @@ function reappearLayoutEffects( } // Fallthrough } - case HostHoistable: case HostComponent: { // TODO: Enable HostText for RN if ( @@ -3316,6 +3387,57 @@ function reappearLayoutEffects( safelyAttachRef(finishedWork, finishedWork.return); break; } + case HostHoistable: { + // $FlowFixMe[constant-condition] + if (supportsResources) { + // The reappear traversal runs whenever an Activity transitions from + // hidden to visible. We piggy-back on it (rather than adding a + // separate recursive traversal) to insert hoistable metadata such as + // and <meta> into the document. + // + // We only act on Hoistable Instances (memoizedState === null). + // Resources stay mounted across Activity visibility transitions. + // + // The parentNode guard makes this idempotent and safe under StrictMode + // dev double-invoke: if the instance is already attached we skip. + // + // Note: this runs in the layout phase. A useLayoutEffect on an earlier + // sibling can therefore observe document.title before the hoistable + // is re-attached. Moving this to the mutation phase would require an + // additional unconditional traversal of the Activity subtree (the + // mutation traversal is gated by subtreeFlags and would skip an + // unchanged hoistable). This is the same tradeoff as for HostSingleton. + const instance = finishedWork.stateNode; + if ( + finishedWork.memoizedState === null && + instance !== null && + !offscreenSubtreeIsHidden + ) { + // currentHoistableRoot is only maintained during the mutation phase. + // Derive the hoistable root from the instance's owner document so + // this works in the layout phase too. Hoistable Instances are + // hoisted to document.head, which always lives in ownerDocument. + mountHoistable( + getHoistableRoot(instance.ownerDocument), + finishedWork.type, + instance, + ); + } + } + recursivelyTraverseReappearLayoutEffects( + finishedRoot, + finishedWork, + layoutEffectTraversalFlags, + ); + + if (includeWorkInProgressEffects && current === null && flags & Update) { + commitHostMount(finishedWork); + } + + // TODO: Check flags & Ref + safelyAttachRef(finishedWork, finishedWork.return); + break; + } case Profiler: { // TODO: Figure out how Profiler updates should work with Offscreen if (includeWorkInProgressEffects && flags & Update) {