From ab075a232409ea1f566526c6a5dca30f658280de Mon Sep 17 00:00:00 2001
From: Samuel Susla <samuel.susla@gmail.com>
Date: Tue, 1 Nov 2022 15:11:28 +0000
Subject: [PATCH] Do not unmount layout effects on initial Offscreen mount
 (#25592)

`wasHidden` is evaluted to false if `current` is null. This means
Offscreen has never been shown but this code assumes it is going from
'visible' to 'hidden' and unmounts layout effects.
To fix this, only unmount layout effects if `current` is not null.

I'm not able to repro this problem or write unit test for it. I see this
crash bug in production test.
The problem with repro is that if Offscreen starts as hidden, it's
render is deferred and current is no longer null.
---
 .../src/ReactFiberCommitWork.new.js           |  3 +-
 .../src/ReactFiberCommitWork.old.js           |  3 +-
 .../src/__tests__/ReactOffscreen-test.js      | 38 +++++++++++++++++++
 3 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js
index 40c3bd2b23..f3fa8fbb21 100644
--- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js
+++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js
@@ -2880,7 +2880,8 @@ function commitMutationEffectsOnFiber(
         }
 
         if (isHidden) {
-          if (!wasHidden) {
+          // Check if this is an update, and the tree was previously visible.
+          if (current !== null && !wasHidden) {
             if ((offscreenBoundary.mode & ConcurrentMode) !== NoMode) {
               // Disappear the layout effects of all the children
               recursivelyTraverseDisappearLayoutEffects(offscreenBoundary);
diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js
index 4f960961c5..ea1ff57f3d 100644
--- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js
+++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js
@@ -2880,7 +2880,8 @@ function commitMutationEffectsOnFiber(
         }
 
         if (isHidden) {
-          if (!wasHidden) {
+          // Check if this is an update, and the tree was previously visible.
+          if (current !== null && !wasHidden) {
             if ((offscreenBoundary.mode & ConcurrentMode) !== NoMode) {
               // Disappear the layout effects of all the children
               recursivelyTraverseDisappearLayoutEffects(offscreenBoundary);
diff --git a/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js b/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js
index 66bd613763..f7ca087088 100644
--- a/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js
+++ b/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js
@@ -268,6 +268,44 @@ describe('ReactOffscreen', () => {
     expect(root).toMatchRenderedOutput(<span prop="Child" />);
   });
 
+  // @gate enableOffscreen
+  it('nested offscreen does not call componentWillUnmount when hidden', async () => {
+    // This is a bug that appeared during production test of <unstable_Offscreen />.
+    // It is a very specific scenario with nested Offscreens. The inner offscreen
+    // goes from visible to hidden in synchronous update.
+    class ClassComponent extends React.Component {
+      render() {
+        return <Text text="Child" />;
+      }
+
+      componentWillUnmount() {
+        Scheduler.unstable_yieldValue('componentWillUnmount');
+      }
+    }
+
+    function App() {
+      const [isVisible, setIsVisible] = React.useState(true);
+
+      if (isVisible === true) {
+        setIsVisible(false);
+      }
+
+      return (
+        <Offscreen mode="hidden">
+          <Offscreen mode={isVisible ? 'visible' : 'hidden'}>
+            <ClassComponent />
+          </Offscreen>
+        </Offscreen>
+      );
+    }
+
+    const root = ReactNoop.createRoot();
+    await act(async () => {
+      root.render(<App />);
+    });
+    expect(Scheduler).toHaveYielded(['Child']);
+  });
+
   // @gate enableOffscreen
   it('mounts/unmounts layout effects when visibility changes (starting hidden)', async () => {
     function Child({text}) {