From 8b3b5c35245b3aa0b2802b51856fe9dad5bcb671 Mon Sep 17 00:00:00 2001
From: Andrew Clark <git@andrewclark.io>
Date: Tue, 19 May 2020 23:10:49 -0700
Subject: [PATCH] Bugfix: Missing mode check in resetChildLanes (#18961)

Deferring a hidden tree is only supported in Concurrent Mode.

The missing check leads to an infinite loop when an update is scheduled
inside a hidden tree, because the pending work never gets reset.

This "accidentally" worked in the old reconciler because the heurstic
we used to detect offscreen trees was if `childExpirationTime`
was `Never`.

In the new reconciler, we check the tag instead. Which means we also
need to check the mode, like we do in the begin phase.

We should move this check out of the hot path. It shouldn't have been
in the hot path of the old reconciler, either.

Probably by moving `resetChildLanes` into the switch statement
in ReactFiberCompleteWork.
---
 .../src/ReactFiberWorkLoop.new.js             |  3 +-
 .../src/__tests__/ReactOffscreen-test.js      | 88 +++++++++++++++++++
 2 files changed, 90 insertions(+), 1 deletion(-)

diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
index 53ac747aa4..d634fc5c70 100644
--- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
+++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
@@ -1684,7 +1684,8 @@ function resetChildLanes(completedWork: Fiber) {
     (completedWork.tag === LegacyHiddenComponent ||
       completedWork.tag === OffscreenComponent) &&
     completedWork.memoizedState !== null &&
-    !includesSomeLane(subtreeRenderLanes, (OffscreenLane: Lane))
+    !includesSomeLane(subtreeRenderLanes, (OffscreenLane: Lane)) &&
+    (completedWork.mode & ConcurrentMode) !== NoLanes
   ) {
     // The children of this component are hidden. Don't bubble their
     // expiration times.
diff --git a/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js b/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js
index c85f333067..9e3cb7bedc 100644
--- a/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js
+++ b/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js
@@ -2,6 +2,7 @@ let React;
 let ReactNoop;
 let Scheduler;
 let LegacyHidden;
+let useState;
 
 describe('ReactOffscreen', () => {
   beforeEach(() => {
@@ -11,6 +12,7 @@ describe('ReactOffscreen', () => {
     ReactNoop = require('react-noop-renderer');
     Scheduler = require('scheduler');
     LegacyHidden = React.unstable_LegacyHidden;
+    useState = React.useState;
   });
 
   function Text(props) {
@@ -77,4 +79,90 @@ describe('ReactOffscreen', () => {
       </>,
     );
   });
+
+  // @gate experimental
+  // @gate new
+  it('does not defer in legacy mode', async () => {
+    let setState;
+    function Foo() {
+      const [state, _setState] = useState('A');
+      setState = _setState;
+      return <Text text={state} />;
+    }
+
+    const root = ReactNoop.createLegacyRoot();
+    await ReactNoop.act(async () => {
+      root.render(
+        <>
+          <LegacyHidden mode="hidden">
+            <Foo />
+          </LegacyHidden>
+          <Text text="Outside" />
+        </>,
+      );
+      // Should not defer the hidden tree
+      expect(Scheduler).toFlushUntilNextPaint(['A', 'Outside']);
+    });
+    expect(root).toMatchRenderedOutput(
+      <>
+        <span prop="A" />
+        <span prop="Outside" />
+      </>,
+    );
+
+    // Test that the children can be updated
+    await ReactNoop.act(async () => {
+      setState('B');
+    });
+    expect(Scheduler).toHaveYielded(['B']);
+    expect(root).toMatchRenderedOutput(
+      <>
+        <span prop="B" />
+        <span prop="Outside" />
+      </>,
+    );
+  });
+
+  // @gate experimental
+  // @gate new
+  it('does not defer in blocking mode', async () => {
+    let setState;
+    function Foo() {
+      const [state, _setState] = useState('A');
+      setState = _setState;
+      return <Text text={state} />;
+    }
+
+    const root = ReactNoop.createBlockingRoot();
+    await ReactNoop.act(async () => {
+      root.render(
+        <>
+          <LegacyHidden mode="hidden">
+            <Foo />
+          </LegacyHidden>
+          <Text text="Outside" />
+        </>,
+      );
+      // Should not defer the hidden tree
+      expect(Scheduler).toFlushUntilNextPaint(['A', 'Outside']);
+    });
+    expect(root).toMatchRenderedOutput(
+      <>
+        <span prop="A" />
+        <span prop="Outside" />
+      </>,
+    );
+
+    // Test that the children can be updated
+    await ReactNoop.act(async () => {
+      setState('B');
+    });
+    expect(Scheduler).toHaveYielded(['B']);
+    expect(root).toMatchRenderedOutput(
+      <>
+        <span prop="B" />
+        <span prop="Outside" />
+      </>,
+    );
+  });
 });