From bbbe2f761b55e1112d686dbfce79e9124b0d8e1c Mon Sep 17 00:00:00 2001
From: Samuel Susla <samuel.susla@gmail.com>
Date: Fri, 9 Sep 2022 21:10:26 +0100
Subject: [PATCH] Prevent infinite re-renders in StrictMode + Offscreen
 (#25203)

* Prevent infinite re-render in StrictMode + Offscreen

* Only fire effects for Offscreen when it is revealed

* Move setting debug fiber into if branch

* Move settings of debug fiber out of if branch
---
 .../src/ReactFiberWorkLoop.new.js             |  9 ++-
 .../src/ReactFiberWorkLoop.old.js             |  9 ++-
 .../ReactOffscreenStrictMode-test.js          | 61 +++++++++++++++++++
 3 files changed, 77 insertions(+), 2 deletions(-)

diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
index 229756596cc16..a40b5ac480728 100644
--- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
+++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
@@ -124,6 +124,7 @@ import {
   LayoutMask,
   PassiveMask,
   PlacementDEV,
+  Visibility,
 } from './ReactFiberFlags';
 import {
   NoLanes,
@@ -3184,9 +3185,15 @@ function doubleInvokeEffectsInDEV(
 ) {
   const isStrictModeFiber = fiber.type === REACT_STRICT_MODE_TYPE;
   const isInStrictMode = parentIsInStrictMode || isStrictModeFiber;
+
   if (fiber.flags & PlacementDEV || fiber.tag === OffscreenComponent) {
     setCurrentDebugFiberInDEV(fiber);
-    if (isInStrictMode) {
+    const isNotOffscreen = fiber.tag !== OffscreenComponent;
+    // Checks if Offscreen is being revealed. For all other components, evaluates to true.
+    const hasOffscreenBecomeVisible =
+      isNotOffscreen ||
+      (fiber.flags & Visibility && fiber.memoizedState === null);
+    if (isInStrictMode && hasOffscreenBecomeVisible) {
       disappearLayoutEffects(fiber);
       disconnectPassiveEffect(fiber);
       reappearLayoutEffects(root, fiber.alternate, fiber, false);
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
index c4caf55518a86..3db85b0b9b23b 100644
--- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
+++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
@@ -124,6 +124,7 @@ import {
   LayoutMask,
   PassiveMask,
   PlacementDEV,
+  Visibility,
 } from './ReactFiberFlags';
 import {
   NoLanes,
@@ -3184,9 +3185,15 @@ function doubleInvokeEffectsInDEV(
 ) {
   const isStrictModeFiber = fiber.type === REACT_STRICT_MODE_TYPE;
   const isInStrictMode = parentIsInStrictMode || isStrictModeFiber;
+
   if (fiber.flags & PlacementDEV || fiber.tag === OffscreenComponent) {
     setCurrentDebugFiberInDEV(fiber);
-    if (isInStrictMode) {
+    const isNotOffscreen = fiber.tag !== OffscreenComponent;
+    // Checks if Offscreen is being revealed. For all other components, evaluates to true.
+    const hasOffscreenBecomeVisible =
+      isNotOffscreen ||
+      (fiber.flags & Visibility && fiber.memoizedState === null);
+    if (isInStrictMode && hasOffscreenBecomeVisible) {
       disappearLayoutEffects(fiber);
       disconnectPassiveEffect(fiber);
       reappearLayoutEffects(root, fiber.alternate, fiber, false);
diff --git a/packages/react-reconciler/src/__tests__/ReactOffscreenStrictMode-test.js b/packages/react-reconciler/src/__tests__/ReactOffscreenStrictMode-test.js
index f750a9b7ef0d6..e6ab08cef5471 100644
--- a/packages/react-reconciler/src/__tests__/ReactOffscreenStrictMode-test.js
+++ b/packages/react-reconciler/src/__tests__/ReactOffscreenStrictMode-test.js
@@ -71,6 +71,21 @@ describe('ReactOffscreenStrictMode', () => {
 
     log = [];
 
+    act(() => {
+      ReactNoop.render(
+        <React.StrictMode>
+          <Offscreen mode="hidden">
+            <Component label="A" />
+            <Component label="B" />
+          </Offscreen>
+        </React.StrictMode>,
+      );
+    });
+
+    expect(log).toEqual(['A: render', 'A: render', 'B: render', 'B: render']);
+
+    log = [];
+
     act(() => {
       ReactNoop.render(
         <React.StrictMode>
@@ -91,5 +106,51 @@ describe('ReactOffscreenStrictMode', () => {
       'A: useLayoutEffect mount',
       'A: useEffect mount',
     ]);
+
+    log = [];
+
+    act(() => {
+      ReactNoop.render(
+        <React.StrictMode>
+          <Offscreen mode="hidden">
+            <Component label="A" />
+          </Offscreen>
+        </React.StrictMode>,
+      );
+    });
+
+    expect(log).toEqual([
+      'A: useLayoutEffect unmount',
+      'A: useEffect unmount',
+      'A: render',
+      'A: render',
+    ]);
+  });
+
+  it('should not cause infinite render loop when StrictMode is used with Suspense and synchronous set states', () => {
+    // This is a regression test, see https://github.com/facebook/react/pull/25179 for more details.
+    function App() {
+      const [state, setState] = React.useState(false);
+
+      React.useLayoutEffect(() => {
+        setState(true);
+      }, []);
+
+      React.useEffect(() => {
+        // Empty useEffect with empty dependency array is needed to trigger infinite render loop.
+      }, []);
+
+      return state;
+    }
+
+    act(() => {
+      ReactNoop.render(
+        <React.StrictMode>
+          <React.Suspense>
+            <App />
+          </React.Suspense>
+        </React.StrictMode>,
+      );
+    });
   });
 });