[Fiber] Don't unhide a node if a direct parent offscreen is still hidden

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);
+          }
         }
       }
 

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}) {

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}) {