Double invoke effects in a subtree wrapped with StrictMode

Author: Andarist

packages/react-reconciler/src/ReactFiberWorkLoop.js

@@ -4079,16 +4079,20 @@ function flushRenderPhaseStrictModeWarningsInDEV() {
 function recursivelyTraverseAndDoubleInvokeEffectsInDEV(
   root: FiberRoot,
   parentFiber: Fiber,
+  treeFlags: Flags,
   isInStrictMode: boolean,
 ) {
-  if ((parentFiber.subtreeFlags & (PlacementDEV | Visibility)) === NoFlags) {
+  if (
+    ((treeFlags | parentFiber.subtreeFlags) & (PlacementDEV | Visibility)) ===
+    NoFlags
+  ) {
     // Parent's descendants have already had effects double invoked.
     // Early exit to avoid unnecessary tree traversal.
     return;
   }
   let child = parentFiber.child;
   while (child !== null) {
-    doubleInvokeEffectsInDEVIfNecessary(root, child, isInStrictMode);
+    doubleInvokeEffectsInDEVIfNecessary(root, child, treeFlags, isInStrictMode);
     child = child.sibling;
   }
 }
@@ -4117,6 +4121,7 @@ function doubleInvokeEffectsOnFiber(
 function doubleInvokeEffectsInDEVIfNecessary(
   root: FiberRoot,
   fiber: Fiber,
+  treeFlags: Flags,
   parentIsInStrictMode: boolean,
 ) {
   const isStrictModeFiber = fiber.type === REACT_STRICT_MODE_TYPE;
@@ -4125,7 +4130,7 @@ function doubleInvokeEffectsInDEVIfNecessary(
   // First case: the fiber **is not** of type OffscreenComponent. No
   // special rules apply to double invoking effects.
   if (fiber.tag !== OffscreenComponent) {
-    if (fiber.flags & PlacementDEV) {
+    if ((treeFlags | fiber.flags) & PlacementDEV) {
       if (isInStrictMode) {
         runWithFiberInDEV(
           fiber,
@@ -4134,14 +4139,15 @@ function doubleInvokeEffectsInDEVIfNecessary(
           fiber,
           (fiber.mode & NoStrictPassiveEffectsMode) === NoMode,
         );
+        return;
       }
-    } else {
-      recursivelyTraverseAndDoubleInvokeEffectsInDEV(
-        root,
-        fiber,
-        isInStrictMode,
-      );
     }
+    recursivelyTraverseAndDoubleInvokeEffectsInDEV(
+      root,
+      fiber,
+      treeFlags | fiber.flags,
+      isInStrictMode,
+    );
     return;
   }
 
@@ -4162,6 +4168,7 @@ function doubleInvokeEffectsInDEVIfNecessary(
         recursivelyTraverseAndDoubleInvokeEffectsInDEV,
         root,
         fiber,
+        treeFlags | fiber.flags,
         isInStrictMode,
       );
     }
@@ -4185,6 +4192,7 @@ function commitDoubleInvokeEffectsInDEV(
       recursivelyTraverseAndDoubleInvokeEffectsInDEV(
         root,
         root.current,
+        root.current.flags,
         doubleInvokeEffects,
       );
     } else {

packages/react-reconciler/src/__tests__/StrictEffectsModeDefaults-test.internal.js

@@ -240,6 +240,64 @@ describe('StrictEffectsMode defaults', () => {
       });
     });
 
+    it('should double invoke effects in a mounted strict subtree', async () => {
+      const log = [];
+      function ComponentWithEffects({label}) {
+        React.useEffect(() => {
+          log.push(`useEffect mount "${label}"`);
+          Scheduler.log(`useEffect mount "${label}"`);
+          return () => {
+            log.push(`useEffect unmount "${label}"`);
+            Scheduler.log(`useEffect unmount "${label}"`);
+          };
+        });
+
+        React.useLayoutEffect(() => {
+          log.push(`useLayoutEffect mount "${label}"`);
+          Scheduler.log(`useLayoutEffect mount "${label}"`);
+          return () => {
+            log.push(`useLayoutEffect unmount "${label}"`);
+            Scheduler.log(`useLayoutEffect unmount "${label}"`);
+          };
+        });
+
+        return label;
+      }
+
+      // this component intentionally introduces a component layer between the root and the StrictMode
+      function RenderChildren({children}) {
+        return children;
+      }
+
+      await act(async () => {
+        ReactNoop.render(
+          <RenderChildren>
+            <ComponentWithEffects label={'one'} />
+            <React.StrictMode>
+              <ComponentWithEffects label={'two'} />
+            </React.StrictMode>
+          </RenderChildren>,
+        );
+
+        await waitForAll([
+          'useLayoutEffect mount "one"',
+          'useLayoutEffect mount "two"',
+          'useEffect mount "one"',
+          'useEffect mount "two"',
+        ]);
+        expect(log).toEqual([
+          'useLayoutEffect mount "one"',
+          'useLayoutEffect mount "two"',
+          'useEffect mount "one"',
+          'useEffect mount "two"',
+          'useLayoutEffect unmount "two"',
+          'useEffect unmount "two"',
+          'useLayoutEffect mount "two"',
+          'useEffect mount "two"',
+        ]);
+      });
+    });
+
     it('double invoking for effects for modern roots', async () => {
       const log = [];
       function App({text}) {