feat(StrictMode): Double-invoke render for every component (#18430)

eps1lon · web-flow · commit ba31ad40a9c6 · 2020-03-29T23:13:46.000+01:00
* feat(StrictMode): Double-invoke render for every component

* fix: Mark ReactTestRendererAsync as internal
diff --git a/packages/react-art/src/__tests__/ReactART-test.js b/packages/react-art/src/__tests__/ReactART-test.js
@@ -391,7 +391,7 @@ describe('ReactART', () => {
       </CurrentRendererContext.Provider>,
     );
 
-    expect(Scheduler).toFlushAndYieldThrough(['A']);
+    expect(Scheduler).toFlushAndYieldThrough(__DEV__ ? ['A', 'A'] : ['A']);
 
     ReactDOM.render(
       <Surface>
@@ -406,7 +406,9 @@ describe('ReactART', () => {
     expect(ops).toEqual([null, 'ART']);
 
     ops = [];
-    expect(Scheduler).toFlushAndYield(['B', 'C']);
+    expect(Scheduler).toFlushAndYield(
+      __DEV__ ? ['B', 'B', 'C', 'C'] : ['B', 'C'],
+    );
 
     expect(ops).toEqual(['Test']);
   });
diff --git a/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.internal.js
@@ -639,7 +639,9 @@ describe('ReactDOMFiberAsync', () => {
       expect(container.textContent).toEqual('');
 
       // Everything should render immediately in the next event
-      expect(Scheduler).toFlushExpired(['A', 'B', 'C']);
+      expect(Scheduler).toFlushExpired(
+        __DEV__ ? ['A', 'A', 'B', 'B', 'C', 'C'] : ['A', 'B', 'C'],
+      );
       expect(container.textContent).toEqual('ABC');
     });
   });
diff --git a/packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js b/packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js
@@ -607,7 +607,9 @@ function runActTests(label, render, unmount, rerender) {
             },
           );
 
-          expect(Component).toHaveBeenCalledTimes(4);
+          expect(Component).toHaveBeenCalledTimes(
+            label === 'legacy mode' ? 4 : 8,
+          );
           unmount(secondContainer);
         });
       }
diff --git a/packages/react-dom/src/__tests__/ReactUpdates-test.js b/packages/react-dom/src/__tests__/ReactUpdates-test.js
@@ -1333,6 +1333,7 @@ describe('ReactUpdates', () => {
             'Foo',
             'Foo',
             'Baz',
+            'Baz',
             'Foo#effect',
           ]);
         } else {
diff --git a/packages/react-dom/src/events/__tests__/DeprecatedDOMEventResponderSystem-test.internal.js b/packages/react-dom/src/events/__tests__/DeprecatedDOMEventResponderSystem-test.internal.js
@@ -813,7 +813,7 @@ describe('DOMEventResponderSystem', () => {
 
     let root = ReactDOM.createRoot(container);
     root.render(<Test counter={0} />);
-    expect(Scheduler).toFlushAndYield(['Test']);
+    expect(Scheduler).toFlushAndYield(__DEV__ ? ['Test', 'Test'] : ['Test']);
 
     // Click the button
     dispatchClickEvent(ref.current);
@@ -825,7 +825,9 @@ describe('DOMEventResponderSystem', () => {
     // Increase counter
     root.render(<Test counter={1} />);
     // Yield before committing
-    expect(Scheduler).toFlushAndYieldThrough(['Test']);
+    expect(Scheduler).toFlushAndYieldThrough(
+      __DEV__ ? ['Test', 'Test'] : ['Test'],
+    );
 
     // Click the button again
     dispatchClickEvent(ref.current);
diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js
@@ -321,17 +321,14 @@ function updateForwardRef(
       debugRenderPhaseSideEffectsForStrictMode &&
       workInProgress.mode & StrictMode
     ) {
-      // Only double-render components with Hooks
-      if (workInProgress.memoizedState !== null) {
-        nextChildren = renderWithHooks(
-          current,
-          workInProgress,
-          render,
-          nextProps,
-          ref,
-          renderExpirationTime,
-        );
-      }
+      nextChildren = renderWithHooks(
+        current,
+        workInProgress,
+        render,
+        nextProps,
+        ref,
+        renderExpirationTime,
+      );
     }
     setIsRendering(false);
   } else {
@@ -662,17 +659,14 @@ function updateFunctionComponent(
       debugRenderPhaseSideEffectsForStrictMode &&
       workInProgress.mode & StrictMode
     ) {
-      // Only double-render components with Hooks
-      if (workInProgress.memoizedState !== null) {
-        nextChildren = renderWithHooks(
-          current,
-          workInProgress,
-          Component,
-          nextProps,
-          context,
-          renderExpirationTime,
-        );
-      }
+      nextChildren = renderWithHooks(
+        current,
+        workInProgress,
+        Component,
+        nextProps,
+        context,
+        renderExpirationTime,
+      );
     }
     setIsRendering(false);
   } else {
@@ -738,17 +732,14 @@ function updateBlock<Props, Data>(
       debugRenderPhaseSideEffectsForStrictMode &&
       workInProgress.mode & StrictMode
     ) {
-      // Only double-render components with Hooks
-      if (workInProgress.memoizedState !== null) {
-        nextChildren = renderWithHooks(
-          current,
-          workInProgress,
-          render,
-          nextProps,
-          data,
-          renderExpirationTime,
-        );
-      }
+      nextChildren = renderWithHooks(
+        current,
+        workInProgress,
+        render,
+        nextProps,
+        data,
+        renderExpirationTime,
+      );
     }
     setIsRendering(false);
   } else {
@@ -1471,17 +1462,14 @@ function mountIndeterminateComponent(
         debugRenderPhaseSideEffectsForStrictMode &&
         workInProgress.mode & StrictMode
       ) {
-        // Only double-render components with Hooks
-        if (workInProgress.memoizedState !== null) {
-          value = renderWithHooks(
-            null,
-            workInProgress,
-            Component,
-            props,
-            context,
-            renderExpirationTime,
-          );
-        }
+        value = renderWithHooks(
+          null,
+          workInProgress,
+          Component,
+          props,
+          context,
+          renderExpirationTime,
+        );
       }
     }
     reconcileChildren(null, workInProgress, value, renderExpirationTime);
diff --git a/packages/react-reconciler/src/__tests__/ErrorBoundaryReconciliation-test.internal.js b/packages/react-reconciler/src/__tests__/ErrorBoundaryReconciliation-test.internal.js
@@ -12,6 +12,7 @@ describe('ErrorBoundaryReconciliation', () => {
     jest.resetModules();
 
     ReactFeatureFlags = require('shared/ReactFeatureFlags');
+    ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false;
     ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false;
     ReactTestRenderer = require('react-test-renderer');
     React = require('react');
diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js
@@ -1300,14 +1300,14 @@ describe('ReactHooks', () => {
         <NoHooks />
       </StrictMode>,
     );
-    expect(renderCount).toBe(1);
+    expect(renderCount).toBe(__DEV__ ? 2 : 1);
     renderCount = 0;
     renderer.update(
       <StrictMode>
         <NoHooks />
       </StrictMode>,
     );
-    expect(renderCount).toBe(1);
+    expect(renderCount).toBe(__DEV__ ? 2 : 1);
 
     renderCount = 0;
     renderer.update(<FwdRef />);
@@ -1321,14 +1321,14 @@ describe('ReactHooks', () => {
         <FwdRef />
       </StrictMode>,
     );
-    expect(renderCount).toBe(1);
+    expect(renderCount).toBe(__DEV__ ? 2 : 1);
     renderCount = 0;
     renderer.update(
       <StrictMode>
         <FwdRef />
       </StrictMode>,
     );
-    expect(renderCount).toBe(1);
+    expect(renderCount).toBe(__DEV__ ? 2 : 1);
 
     renderCount = 0;
     renderer.update(<Memo arg={1} />);
@@ -1342,14 +1342,14 @@ describe('ReactHooks', () => {
         <Memo arg={1} />
       </StrictMode>,
     );
-    expect(renderCount).toBe(1);
+    expect(renderCount).toBe(__DEV__ ? 2 : 1);
     renderCount = 0;
     renderer.update(
       <StrictMode>
         <Memo arg={2} />
       </StrictMode>,
     );
-    expect(renderCount).toBe(1);
+    expect(renderCount).toBe(__DEV__ ? 2 : 1);
 
     renderCount = 0;
     expect(() => renderer.update(<Factory />)).toErrorDev(
diff --git a/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js b/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js
@@ -1103,7 +1103,9 @@ describe('ReactNewContext', () => {
 
       // Render the provider again using a different renderer
       ReactNoop.render(<App value={1} />);
-      expect(Scheduler).toFlushAndYield(['Foo', 'Foo']);
+      expect(Scheduler).toFlushAndYield(
+        __DEV__ ? ['Foo', 'Foo', 'Foo', 'Foo'] : ['Foo', 'Foo'],
+      );
 
       if (__DEV__) {
         expect(console.error.calls.argsFor(0)[0]).toContain(
diff --git a/packages/react-test-renderer/src/__tests__/ReactTestRendererAsync-test.internal.js b/packages/react-test-renderer/src/__tests__/ReactTestRendererAsync-test.internal.js
@@ -11,12 +11,15 @@
 'use strict';
 
 let React;
+let ReactFeatureFlags;
 let ReactTestRenderer;
 let Scheduler;
 
 describe('ReactTestRendererAsync', () => {
   beforeEach(() => {
     jest.resetModules();
+    ReactFeatureFlags = require('shared/ReactFeatureFlags');
+    ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false;
     React = require('react');
     ReactTestRenderer = require('react-test-renderer');
     Scheduler = require('scheduler');
diff --git a/packages/react/src/__tests__/ReactProfilerDevToolsIntegration-test.internal.js b/packages/react/src/__tests__/ReactProfilerDevToolsIntegration-test.internal.js
@@ -188,7 +188,7 @@ describe('ReactProfiler DevTools integration', () => {
 
     // Commit something
     root.update(<Text text="A" />);
-    expect(Scheduler).toFlushAndYield(['A']);
+    expect(Scheduler).toFlushAndYield(__DEV__ ? ['A', 'A'] : ['A']);
     expect(root).toMatchRenderedOutput('A');
 
     // Advance time by many seconds, larger than the default expiration time
@@ -200,7 +200,7 @@ describe('ReactProfiler DevTools integration', () => {
     // Update B should not instantly expire.
     expect(Scheduler).toFlushExpired([]);
 
-    expect(Scheduler).toFlushAndYield(['B']);
+    expect(Scheduler).toFlushAndYield(__DEV__ ? ['B', 'B'] : ['B']);
     expect(root).toMatchRenderedOutput('B');
   });
 });
diff --git a/packages/react/src/__tests__/forwardRef-test.js b/packages/react/src/__tests__/forwardRef-test.js
@@ -260,11 +260,11 @@ describe('forwardRef', () => {
 
     ReactNoop.render(<RefForwardingComponent ref={ref} optional="foo" />);
     expect(Scheduler).toFlushWithoutYielding();
-    expect(renderCount).toBe(1);
+    expect(renderCount).toBe(__DEV__ ? 2 : 1);
 
     ReactNoop.render(<RefForwardingComponent ref={ref} optional="foo" />);
     expect(Scheduler).toFlushWithoutYielding();
-    expect(renderCount).toBe(2);
+    expect(renderCount).toBe(__DEV__ ? 4 : 2);
   });
 
   it('should bailout if forwardRef is wrapped in memo', () => {
@@ -283,28 +283,28 @@ describe('forwardRef', () => {
 
     ReactNoop.render(<RefForwardingComponent ref={ref} optional="foo" />);
     expect(Scheduler).toFlushWithoutYielding();
-    expect(renderCount).toBe(1);
+    expect(renderCount).toBe(__DEV__ ? 2 : 1);
 
     expect(ref.current.type).toBe('div');
 
     ReactNoop.render(<RefForwardingComponent ref={ref} optional="foo" />);
     expect(Scheduler).toFlushWithoutYielding();
-    expect(renderCount).toBe(1);
+    expect(renderCount).toBe(__DEV__ ? 2 : 1);
 
     const differentRef = React.createRef();
 
     ReactNoop.render(
       <RefForwardingComponent ref={differentRef} optional="foo" />,
     );
     expect(Scheduler).toFlushWithoutYielding();
-    expect(renderCount).toBe(2);
+    expect(renderCount).toBe(__DEV__ ? 4 : 2);
 
     expect(ref.current).toBe(null);
     expect(differentRef.current.type).toBe('div');
 
     ReactNoop.render(<RefForwardingComponent ref={ref} optional="bar" />);
     expect(Scheduler).toFlushWithoutYielding();
-    expect(renderCount).toBe(3);
+    expect(renderCount).toBe(__DEV__ ? 6 : 3);
   });
 
   it('should custom memo comparisons to compose', () => {
@@ -324,19 +324,19 @@ describe('forwardRef', () => {
 
     ReactNoop.render(<RefForwardingComponent ref={ref} a="0" b="0" c="1" />);
     expect(Scheduler).toFlushWithoutYielding();
-    expect(renderCount).toBe(1);
+    expect(renderCount).toBe(__DEV__ ? 2 : 1);
 
     expect(ref.current.type).toBe('div');
 
     // Changing either a or b rerenders
     ReactNoop.render(<RefForwardingComponent ref={ref} a="0" b="1" c="1" />);
     expect(Scheduler).toFlushWithoutYielding();
-    expect(renderCount).toBe(2);
+    expect(renderCount).toBe(__DEV__ ? 4 : 2);
 
     // Changing c doesn't rerender
     ReactNoop.render(<RefForwardingComponent ref={ref} a="0" b="1" c="2" />);
     expect(Scheduler).toFlushWithoutYielding();
-    expect(renderCount).toBe(2);
+    expect(renderCount).toBe(__DEV__ ? 4 : 2);
 
     const ComposedMemo = React.memo(
       RefForwardingComponent,
@@ -345,29 +345,29 @@ describe('forwardRef', () => {
 
     ReactNoop.render(<ComposedMemo ref={ref} a="0" b="0" c="0" />);
     expect(Scheduler).toFlushWithoutYielding();
-    expect(renderCount).toBe(3);
+    expect(renderCount).toBe(__DEV__ ? 6 : 3);
 
     // Changing just b no longer updates
     ReactNoop.render(<ComposedMemo ref={ref} a="0" b="1" c="0" />);
     expect(Scheduler).toFlushWithoutYielding();
-    expect(renderCount).toBe(3);
+    expect(renderCount).toBe(__DEV__ ? 6 : 3);
 
     // Changing just a and c updates
     ReactNoop.render(<ComposedMemo ref={ref} a="2" b="2" c="2" />);
     expect(Scheduler).toFlushWithoutYielding();
-    expect(renderCount).toBe(4);
+    expect(renderCount).toBe(__DEV__ ? 8 : 4);
 
     // Changing just c does not update
     ReactNoop.render(<ComposedMemo ref={ref} a="2" b="2" c="3" />);
     expect(Scheduler).toFlushWithoutYielding();
-    expect(renderCount).toBe(4);
+    expect(renderCount).toBe(__DEV__ ? 8 : 4);
 
     // Changing ref still rerenders
     const differentRef = React.createRef();
 
     ReactNoop.render(<ComposedMemo ref={differentRef} a="2" b="2" c="3" />);
     expect(Scheduler).toFlushWithoutYielding();
-    expect(renderCount).toBe(5);
+    expect(renderCount).toBe(__DEV__ ? 10 : 5);
 
     expect(ref.current).toBe(null);
     expect(differentRef.current.type).toBe('div');
