From 1d68bce19c9409ed70604d1d16b70b68ce71dc4a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= <sebastian@calyptus.eu>
Date: Sun, 12 Oct 2025 19:50:06 -0400
Subject: [PATCH] [Fiber] Don't unhide a node if a direct parent offscreen is
 still hidden (#34821)

If an inner Offscreen commits an unhide, but an outer Offscreen is still
hidden but they're controlling the same DOM node then we shouldn't
unhide the DOM node yet.

This keeps track of whether we're directly inside a hidden offscreen. It
might be better to just do the tree search instead of keeping the stack
state since it's a rare case. Although this hide/unhide path does
trigger a lot of times even when there's no change.

This was technically a bug with Suspense too but it doesn't appear
because a suspended Suspense boundary never commits its partial state.
If it did, it would trigger this same path. But it can happen with an
outer Activity and inner Suspense.
---
 .../src/ReactFiberCommitWork.js               | 20 +++-
 .../src/__tests__/Activity-test.js            | 68 +++++++++++++
 .../ReactSuspenseEffectsSemantics-test.js     | 99 +++++++++++++++++++
 3 files changed, 184 insertions(+), 3 deletions(-)

diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js
index c98edc7a0bc57..825b814db5103 100644
--- a/packages/react-reconciler/src/ReactFiberCommitWork.js
+++ b/packages/react-reconciler/src/ReactFiberCommitWork.js
@@ -292,6 +292,9 @@ import type {Flags} from './ReactFiberFlags';
 // Allows us to avoid traversing the return path to find the nearest Offscreen ancestor.
 let offscreenSubtreeIsHidden: boolean = false;
 let offscreenSubtreeWasHidden: boolean = false;
+// Track whether there's a hidden offscreen above with no HostComponent between. If so,
+// it overrides the hiddenness of the HostComponent below.
+let offscreenDirectParentIsHidden: boolean = false;
 
 // Used to track if a form needs to be reset at the end of the mutation phase.
 let needsFormReset = false;
@@ -2141,8 +2144,14 @@ function commitMutationEffectsOnFiber(
       // Fall through
     }
     case HostComponent: {
+      // We've hit a host component, so it's no longer a direct parent.
+      const prevOffscreenDirectParentIsHidden = offscreenDirectParentIsHidden;
+      offscreenDirectParentIsHidden = false;
+
       recursivelyTraverseMutationEffects(root, finishedWork, lanes);
 
+      offscreenDirectParentIsHidden = prevOffscreenDirectParentIsHidden;
+
       commitReconciliationEffects(finishedWork, lanes);
 
       if (flags & Ref) {
@@ -2422,10 +2431,14 @@ function commitMutationEffectsOnFiber(
         // effects again.
         const prevOffscreenSubtreeIsHidden = offscreenSubtreeIsHidden;
         const prevOffscreenSubtreeWasHidden = offscreenSubtreeWasHidden;
+        const prevOffscreenDirectParentIsHidden = offscreenDirectParentIsHidden;
         offscreenSubtreeIsHidden = prevOffscreenSubtreeIsHidden || isHidden;
+        offscreenDirectParentIsHidden =
+          prevOffscreenDirectParentIsHidden || isHidden;
         offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden || wasHidden;
         recursivelyTraverseMutationEffects(root, finishedWork, lanes);
         offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden;
+        offscreenDirectParentIsHidden = prevOffscreenDirectParentIsHidden;
         offscreenSubtreeIsHidden = prevOffscreenSubtreeIsHidden;
 
         if (
@@ -2504,9 +2517,10 @@ function commitMutationEffectsOnFiber(
         }
 
         if (supportsMutation) {
-          // TODO: This needs to run whenever there's an insertion or update
-          // inside a hidden Offscreen tree.
-          hideOrUnhideAllChildren(finishedWork, isHidden);
+          // If it's trying to unhide but the parent is still hidden, then we should not unhide.
+          if (isHidden || !offscreenDirectParentIsHidden) {
+            hideOrUnhideAllChildren(finishedWork, isHidden);
+          }
         }
       }
 
diff --git a/packages/react-reconciler/src/__tests__/Activity-test.js b/packages/react-reconciler/src/__tests__/Activity-test.js
index 01311217e2444..2fdcc16701dbc 100644
--- a/packages/react-reconciler/src/__tests__/Activity-test.js
+++ b/packages/react-reconciler/src/__tests__/Activity-test.js
@@ -14,6 +14,7 @@ let waitForPaint;
 let waitFor;
 let assertLog;
 let assertConsoleErrorDev;
+let Suspense;
 
 describe('Activity', () => {
   beforeEach(() => {
@@ -25,6 +26,7 @@ describe('Activity', () => {
     act = require('internal-test-utils').act;
     LegacyHidden = React.unstable_LegacyHidden;
     Activity = React.Activity;
+    Suspense = React.Suspense;
     useState = React.useState;
     useInsertionEffect = React.useInsertionEffect;
     useLayoutEffect = React.useLayoutEffect;
@@ -1424,6 +1426,72 @@ describe('Activity', () => {
     );
   });
 
+  // @gate enableActivity
+  it('reveal an inner Activity boundary without revealing an outer one on the same host child', async () => {
+    // This ensures that no update is scheduled, which would cover up the bug if the parent
+    // then re-hides the child on the way up.
+    const memoizedElement = <div />;
+    function App({showOuter, showInner}) {
+      return (
+        <Activity mode={showOuter ? 'visible' : 'hidden'} name="Outer">
+          <Activity mode={showInner ? 'visible' : 'hidden'} name="Inner">
+            {memoizedElement}
+          </Activity>
+        </Activity>
+      );
+    }
+
+    const root = ReactNoop.createRoot();
+
+    // Prerender the whole tree.
+    await act(() => {
+      root.render(<App showOuter={false} showInner={false} />);
+    });
+    expect(root).toMatchRenderedOutput(<div hidden={true} />);
+
+    await act(() => {
+      root.render(<App showOuter={false} showInner={true} />);
+    });
+    expect(root).toMatchRenderedOutput(<div hidden={true} />);
+  });
+
+  // @gate enableActivity
+  it('reveal an inner Suspense boundary without revealing an outer Activity on the same host child', async () => {
+    // This ensures that no update is scheduled, which would cover up the bug if the parent
+    // then re-hides the child on the way up.
+    const memoizedElement = <div />;
+    const promise = new Promise(() => {});
+    function App({showOuter, showInner}) {
+      return (
+        <Activity mode={showOuter ? 'visible' : 'hidden'} name="Outer">
+          <Suspense name="Inner">
+            {memoizedElement}
+            {showInner ? null : promise}
+          </Suspense>
+        </Activity>
+      );
+    }
+
+    const root = ReactNoop.createRoot();
+
+    // Prerender the whole tree.
+    await act(() => {
+      root.render(<App showOuter={false} showInner={true} />);
+    });
+    expect(root).toMatchRenderedOutput(<div hidden={true} />);
+
+    // Resuspend the inner.
+    await act(() => {
+      root.render(<App showOuter={false} showInner={false} />);
+    });
+    expect(root).toMatchRenderedOutput(<div hidden={true} />);
+
+    await act(() => {
+      root.render(<App showOuter={false} showInner={true} />);
+    });
+    expect(root).toMatchRenderedOutput(<div hidden={true} />);
+  });
+
   // @gate enableActivity
   it('insertion effects are not disconnected when the visibility changes', async () => {
     function Child({step}) {
diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseEffectsSemantics-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseEffectsSemantics-test.js
index 9fc94d7d0eee4..eeb00ac98502d 100644
--- a/packages/react-reconciler/src/__tests__/ReactSuspenseEffectsSemantics-test.js
+++ b/packages/react-reconciler/src/__tests__/ReactSuspenseEffectsSemantics-test.js
@@ -1401,6 +1401,105 @@ describe('ReactSuspenseEffectsSemantics', () => {
       );
     });
 
+    // @gate enableLegacyCache
+    it('should wait to reveal an inner child when inner one reveals first', async () => {
+      function App({outerChildren, innerChildren}) {
+        return (
+          <Suspense fallback={<Text text="OuterFallback" />} name="Outer">
+            <Suspense fallback={<Text text="InnerFallback" />} name="Inner">
+              <div>{innerChildren}</div>
+            </Suspense>
+            {outerChildren}
+          </Suspense>
+        );
+      }
+
+      // Mount
+      await act(() => {
+        ReactNoop.render(<App />);
+      });
+      assertLog([]);
+      expect(ReactNoop).toMatchRenderedOutput(<div />);
+
+      // Resuspend inner boundary
+      await act(() => {
+        ReactNoop.render(
+          <App
+            outerChildren={null}
+            innerChildren={<AsyncText text="InnerAsync" />}
+          />,
+        );
+      });
+      assertLog([
+        'Suspend:InnerAsync',
+        'Text:InnerFallback render',
+        'Text:InnerFallback create insertion',
+        'Text:InnerFallback create layout',
+        'Text:InnerFallback create passive',
+        'Suspend:InnerAsync',
+      ]);
+      expect(ReactNoop).toMatchRenderedOutput(
+        <>
+          <div hidden={true} />
+          <span prop="InnerFallback" />
+        </>,
+      );
+
+      // Resuspend both boundaries
+      await act(() => {
+        ReactNoop.render(
+          <App
+            outerChildren={<AsyncText text="OuterAsync" />}
+            innerChildren={<AsyncText text="InnerAsync" />}
+          />,
+        );
+      });
+      assertLog([
+        'Suspend:InnerAsync',
+        'Text:InnerFallback render',
+        'Suspend:OuterAsync',
+        'Text:OuterFallback render',
+        'Text:InnerFallback destroy layout',
+        'Text:OuterFallback create insertion',
+        'Text:OuterFallback create layout',
+        'Text:OuterFallback create passive',
+        'Suspend:InnerAsync',
+        'Text:InnerFallback render',
+        'Suspend:OuterAsync',
+      ]);
+      expect(ReactNoop).toMatchRenderedOutput(
+        <>
+          <div hidden={true} />
+          <span prop="InnerFallback" hidden={true} />
+          <span prop="OuterFallback" />
+        </>,
+      );
+
+      // Unsuspend the inner Suspense subtree only
+      // Interestingly, this never commits because the tree is left suspended.
+      // If it did commit, it would potentially cause the div to incorrectly reappear.
+      await act(() => {
+        ReactNoop.render(
+          <App
+            outerChildren={<AsyncText text="OuterAsync" />}
+            innerChildren={null}
+          />,
+        );
+      });
+      assertLog([
+        'Suspend:OuterAsync',
+        'Text:OuterFallback render',
+        'Suspend:OuterAsync',
+      ]);
+      expect(ReactNoop).toMatchRenderedOutput(
+        <>
+          <div hidden={true} />
+          <span prop="InnerFallback" hidden={true} />
+          <span prop="OuterFallback" />
+        </>,
+      );
+    });
+
     // @gate enableLegacyCache
     it('should show nested host nodes if multiple boundaries resolve at the same time', async () => {
       function App({innerChildren = null, outerChildren = null}) {