Dim console calls when invoking effects due to StrictMode

Author: eps1lon

packages/internal-test-utils/__tests__/ReactInternalTestUtils-test.js

@@ -146,6 +146,12 @@ describe('ReactInternalTestUtils', () => {
   test('assertLog', async () => {
     const Yield = ({id}) => {
       Scheduler.log(id);
+      React.useEffect(() => {
+        Scheduler.log(`create effect ${id}`);
+        return () => {
+          Scheduler.log(`cleanup effect ${id}`);
+        };
+      });
       return id;
     };
 
@@ -167,7 +173,20 @@ describe('ReactInternalTestUtils', () => {
         </React.StrictMode>
       );
     });
-    assertLog(['A', 'B', 'C']);
+    assertLog([
+      'A',
+      'B',
+      'C',
+      'create effect A',
+      'create effect B',
+      'create effect C',
+    ]);
+
+    await act(() => {
+      root.render(null);
+    });
+
+    assertLog(['cleanup effect A', 'cleanup effect B', 'cleanup effect C']);
   });
 });
 

packages/react-reconciler/src/ReactFiberWorkLoop.js

@@ -249,6 +249,7 @@ import {
   markRenderStopped,
   onCommitRoot as onCommitRootDevTools,
   onPostCommitRoot as onPostCommitRootDevTools,
+  setIsStrictModeForDevtools,
 } from './ReactFiberDevToolsHook';
 import {onCommitRoot as onCommitRootTestSelector} from './ReactTestSelectors';
 import {releaseCache} from './ReactFiberCacheComponent';
@@ -3667,6 +3668,7 @@ function doubleInvokeEffectsOnFiber(
   fiber: Fiber,
   shouldDoubleInvokePassiveEffects: boolean = true,
 ) {
+  setIsStrictModeForDevtools(true);
   disappearLayoutEffects(fiber);
   if (shouldDoubleInvokePassiveEffects) {
     disconnectPassiveEffect(fiber);
@@ -3675,6 +3677,7 @@ function doubleInvokeEffectsOnFiber(
   if (shouldDoubleInvokePassiveEffects) {
     reconnectPassiveEffects(root, fiber, NoLanes, null, false);
   }
+  setIsStrictModeForDevtools(false);
 }
 
 function doubleInvokeEffectsInDEVIfNecessary(

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

@@ -81,6 +81,7 @@ describe('updaters', () => {
       markRenderScheduled: jest.fn(() => {}),
       markForceUpdateScheduled: jest.fn(() => {}),
       markStateUpdateScheduled: jest.fn(() => {}),
+      setIsStrictModeForDevtools: jest.fn(() => {}),
     };
 
     jest.mock(

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

@@ -115,11 +115,7 @@ describe('StrictEffectsMode defaults', () => {
           </React.StrictMode>,
         );
 
-        await waitForPaint([
-          'useLayoutEffect mount "one"',
-          'useLayoutEffect unmount "one"',
-          'useLayoutEffect mount "one"',
-        ]);
+        await waitForPaint(['useLayoutEffect mount "one"']);
         expect(log).toEqual([
           'useLayoutEffect mount "one"',
           'useLayoutEffect unmount "one"',
@@ -142,10 +138,6 @@ describe('StrictEffectsMode defaults', () => {
           'useLayoutEffect unmount "one"',
           'useLayoutEffect mount "one"',
           'useLayoutEffect mount "two"',
-
-          // Since "two" is new, it should be double-invoked.
-          'useLayoutEffect unmount "two"',
-          'useLayoutEffect mount "two"',
         ]);
         expect(log).toEqual([
           // Cleanup and re-run "one" (and "two") since there is no dependencies array.
@@ -196,10 +188,6 @@ describe('StrictEffectsMode defaults', () => {
         await waitForAll([
           'useLayoutEffect mount "one"',
           'useEffect mount "one"',
-          'useLayoutEffect unmount "one"',
-          'useEffect unmount "one"',
-          'useLayoutEffect mount "one"',
-          'useEffect mount "one"',
         ]);
         expect(log).toEqual([
           'useLayoutEffect mount "one"',
@@ -237,12 +225,6 @@ describe('StrictEffectsMode defaults', () => {
           'useEffect unmount "one"',
           'useEffect mount "one"',
           'useEffect mount "two"',
-
-          // Since "two" is new, it should be double-invoked.
-          'useLayoutEffect unmount "two"',
-          'useEffect unmount "two"',
-          'useLayoutEffect mount "two"',
-          'useEffect mount "two"',
         ]);
         expect(log).toEqual([
           'useEffect unmount "one"',

packages/react/src/__tests__/ReactStrictMode-test.js

@@ -1343,6 +1343,40 @@ describe('context legacy', () => {
         // and on the next render they'd get deduplicated and ignored.
         expect(console.log).toBeCalledWith('foo 1');
       });
+
+      it('does not disable logs for effect double invoke', async () => {
+        let create = 0;
+        let cleanup = 0;
+        function Foo() {
+          React.useEffect(() => {
+            create++;
+            console.log('foo create ' + create);
+            return () => {
+              cleanup++;
+              console.log('foo cleanup ' + cleanup);
+            };
+          });
+          return null;
+        }
+
+        const container = document.createElement('div');
+        const root = ReactDOMClient.createRoot(container);
+        await act(() => {
+          root.render(
+            <React.StrictMode>
+              <Foo />
+            </React.StrictMode>,
+          );
+        });
+        expect(create).toBe(__DEV__ ? 2 : 1);
+        expect(cleanup).toBe(__DEV__ ? 1 : 0);
+        expect(console.log).toBeCalledTimes(__DEV__ ? 3 : 1);
+        // Note: we should display the first log because otherwise
+        // there is a risk of suppressing warnings when they happen,
+        // and on the next render they'd get deduplicated and ignored.
+        expect(console.log).toBeCalledWith('foo create 1');
+        expect(console.log).toBeCalledWith('foo cleanup 1');
+      });
     } else {
       it('disable logs for class double render', async () => {
         let count = 0;
@@ -1530,6 +1564,40 @@ describe('context legacy', () => {
         // and on the next render they'd get deduplicated and ignored.
         expect(console.log).toBeCalledWith('foo 1');
       });
+
+      it('disable logs for effect double invoke', async () => {
+        let create = 0;
+        let cleanup = 0;
+        function Foo() {
+          React.useEffect(() => {
+            create++;
+            console.log('foo create ' + create);
+            return () => {
+              cleanup++;
+              console.log('foo cleanup ' + cleanup);
+            };
+          });
+          return null;
+        }
+
+        const container = document.createElement('div');
+        const root = ReactDOMClient.createRoot(container);
+        await act(() => {
+          root.render(
+            <React.StrictMode>
+              <Foo />
+            </React.StrictMode>,
+          );
+        });
+        expect(create).toBe(__DEV__ ? 2 : 1);
+        expect(cleanup).toBe(__DEV__ ? 1 : 0);
+        expect(console.log).toBeCalledTimes(1);
+        // Note: we should display the first log because otherwise
+        // there is a risk of suppressing warnings when they happen,
+        // and on the next render they'd get deduplicated and ignored.
+        expect(console.log).toBeCalledWith('foo create 1');
+        expect(console.log).toBeCalledWith('foo cleanup 1');
+      });
     }
   });
 });