Warn when rendering tests in concurrent/batched mode without a mocked scheduler

threepointone · threepointone · commit eb9908c6df44 · 2019-07-26T00:47:23.000+01:00
Concurrent/Batched mode tests should always be run with a mocked scheduler (v17 or not). This PR adds a warning for the same. I'll put up a separate PR to the docs with a page detailing how to mock the scheduler.
diff --git a/fixtures/dom/src/index.test.js b/fixtures/dom/src/index.test.js
@@ -24,23 +24,7 @@ function App(props) {
   return 'hello world';
 }
 
-describe('legacy mode', () => {
-  runTests();
-});
-
-describe('mocked scheduler', () => {
-  beforeEach(() => {
-    jest.mock('scheduler', () =>
-      require.requireActual('scheduler/unstable_mock')
-    );
-  });
-  afterEach(() => {
-    jest.unmock('scheduler');
-  });
-  runTests();
-});
-
-function runTests() {
+describe('wrong act warnings', () => {
   beforeEach(() => {
     jest.resetModules();
     React = require('react');
@@ -111,17 +95,6 @@ function runTests() {
     });
   });
 
-  it('warns when using createRoot() + .render', () => {
-    const root = ReactDOM.unstable_createRoot(document.createElement('div'));
-    expect(() => {
-      TestRenderer.act(() => {
-        root.render(<App />);
-      });
-    }).toWarnDev(["It looks like you're using the wrong act()"], {
-      withoutStack: true,
-    });
-  });
-
   it('warns when using the wrong act version - test + dom: render', () => {
     expect(() => {
       TestRenderer.act(() => {
@@ -203,31 +176,103 @@ function runTests() {
     });
   });
 
-  it('flushes work only outside the outermost act(), even when nested from different renderers', () => {
-    const log = [];
-    function Effecty() {
-      React.useEffect(() => {
-        log.push('called');
-      }, []);
-      return null;
-    }
-    // in legacy mode, this tests whether an act only flushes its own effects
-    // with a mocked scheduler, this tests whether it flushes all work only on the outermost act
-    TestRenderer.act(() => {
+  it('warns when using createRoot() + .render', () => {
+    const root = ReactDOM.unstable_createRoot(document.createElement('div'));
+    expect(() => {
+      TestRenderer.act(() => {
+        root.render(<App />);
+      });
+    }).toWarnDev(
+      [
+        'In Concurrent or Sync modes, the "scheduler" module needs to be mocked',
+        "It looks like you're using the wrong act()",
+      ],
+      {
+        withoutStack: true,
+      }
+    );
+  });
+});
+
+describe('flush work on nested act', () => {
+  describe('unmocked scheduler', () => {
+    beforeEach(() => {
+      jest.resetModules();
+      React = require('react');
+      ReactDOM = require('react-dom');
+      TestUtils = require('react-dom/test-utils');
+      TestRenderer = require('react-test-renderer');
+    });
+
+    it('flushes work only outside the outermost act(), even when nested from different renderers', () => {
+      const log = [];
+      function Effecty() {
+        React.useEffect(() => {
+          log.push('called');
+        }, []);
+        return null;
+      }
+      // in legacy mode, this tests whether an act only flushes its own effects
+      TestRenderer.act(() => {
+        TestUtils.act(() => {
+          TestRenderer.create(<Effecty />);
+        });
+        expect(log).toEqual([]);
+      });
+      expect(log).toEqual(['called']);
+
+      log.splice(0);
+      // for doublechecking, we flip it inside out, and assert on the outermost
       TestUtils.act(() => {
-        TestRenderer.create(<Effecty />);
+        TestRenderer.act(() => {
+          TestRenderer.create(<Effecty />);
+        });
       });
-      expect(log).toEqual([]);
+      expect(log).toEqual(['called']);
     });
-    expect(log).toEqual(['called']);
+  });
 
-    log.splice(0);
-    // for doublechecking, we flip it inside out, and assert on the outermost
-    TestUtils.act(() => {
+  describe('mocked scheduler', () => {
+    beforeEach(() => {
+      jest.resetModules();
+      jest.mock('scheduler', () =>
+        require.requireActual('scheduler/unstable_mock')
+      );
+      React = require('react');
+      ReactDOM = require('react-dom');
+      TestUtils = require('react-dom/test-utils');
+      TestRenderer = require('react-test-renderer');
+    });
+
+    afterEach(() => {
+      jest.unmock('scheduler');
+    });
+
+    it('flushes work only outside the outermost act(), even when nested from different renderers', () => {
+      const log = [];
+      function Effecty() {
+        React.useEffect(() => {
+          log.push('called');
+        }, []);
+        return null;
+      }
+      // with a mocked scheduler, this tests whether it flushes all work only on the outermost act
       TestRenderer.act(() => {
-        TestRenderer.create(<Effecty />);
+        TestUtils.act(() => {
+          TestRenderer.create(<Effecty />);
+        });
+        expect(log).toEqual([]);
+      });
+      expect(log).toEqual(['called']);
+
+      log.splice(0);
+      // for doublechecking, we flip it inside out, and assert on the outermost
+      TestUtils.act(() => {
+        TestRenderer.act(() => {
+          TestRenderer.create(<Effecty />);
+        });
       });
+      expect(log).toEqual(['called']);
     });
-    expect(log).toEqual(['called']);
   });
-}
+});
diff --git a/packages/react-dom/src/__tests__/ReactTestUtilsAct-test.internal.js b/packages/react-dom/src/__tests__/ReactTestUtilsAct-test.internal.js
@@ -7,22 +7,93 @@
  * @emails react-core
  */
 
+let React;
+let ReactDOM;
 let ReactFeatureFlags;
-let act;
-describe('mocked scheduler', () => {
-  beforeEach(() => {
-    jest.resetModules();
-    ReactFeatureFlags = require('shared/ReactFeatureFlags');
-    ReactFeatureFlags.warnAboutMissingMockScheduler = true;
-    jest.unmock('scheduler');
-    act = require('react-dom/test-utils').act;
+
+jest.unmock('scheduler');
+
+function App() {
+  return null;
+}
+
+describe('unmocked scheduler warning', () => {
+  describe('warns on mount', () => {
+    beforeEach(() => {
+      jest.resetModules();
+      React = require('react');
+      ReactDOM = require('react-dom');
+    });
+
+    it('does not warn when rendering in sync mode', () => {
+      expect(() => {
+        ReactDOM.render(<App />, document.createElement('div'));
+      }).toWarnDev([]);
+    });
+
+    it('should warn when rendering in concurrent mode', () => {
+      expect(() => {
+        ReactDOM.unstable_createRoot(document.createElement('div')).render(
+          <App />,
+        );
+      }).toWarnDev(
+        'In Concurrent or Sync modes, the "scheduler" module needs to be mocked ' +
+          'to guarantee consistent behaviour across tests and browsers.',
+        {withoutStack: true},
+      );
+      // does not warn twice
+      expect(() => {
+        ReactDOM.unstable_createRoot(document.createElement('div')).render(
+          <App />,
+        );
+      }).toWarnDev([]);
+    });
+
+    it('should warn when rendering in batched mode', () => {
+      expect(() => {
+        ReactDOM.unstable_createSyncRoot(document.createElement('div')).render(
+          <App />,
+        );
+      }).toWarnDev(
+        'In Concurrent or Sync modes, the "scheduler" module needs to be mocked ' +
+          'to guarantee consistent behaviour across tests and browsers.',
+        {withoutStack: true},
+      );
+      // does not warn twice
+      expect(() => {
+        ReactDOM.unstable_createSyncRoot(document.createElement('div')).render(
+          <App />,
+        );
+      }).toWarnDev([]);
+    });
   });
-  it("should warn when the scheduler isn't mocked", () => {
-    expect(() => act(() => {})).toWarnDev(
-      [
-        'Starting from React v17, the "scheduler" module will need to be mocked',
-      ],
-      {withoutStack: true},
-    );
+
+  describe('warns with the feature flag', () => {
+    beforeEach(() => {
+      jest.resetModules();
+      React = require('react');
+      ReactDOM = require('react-dom');
+      ReactFeatureFlags = require('shared/ReactFeatureFlags');
+      ReactFeatureFlags.warnAboutMissingMockScheduler = true;
+    });
+
+    afterEach(() => {
+      ReactFeatureFlags.warnAboutMissingMockScheduler = false;
+    });
+
+    it('should warn in sync mode when the feature flag is enabled', () => {
+      expect(() => {
+        ReactDOM.render(<App />, document.createElement('div'));
+      }).toWarnDev(
+        [
+          'Starting from React v17, the "scheduler" module will need to be mocked',
+        ],
+        {withoutStack: true},
+      );
+      // does not warn twice
+      expect(() => {
+        ReactDOM.render(<App />, document.createElement('div'));
+      }).toWarnDev([]);
+    });
   });
 });
diff --git a/packages/react-dom/src/test-utils/ReactTestUtilsAct.js b/packages/react-dom/src/test-utils/ReactTestUtilsAct.js
@@ -12,7 +12,6 @@ import type {Thenable} from 'react-reconciler/src/ReactFiberWorkLoop';
 import warningWithoutStack from 'shared/warningWithoutStack';
 import ReactDOM from 'react-dom';
 import ReactSharedInternals from 'shared/ReactSharedInternals';
-import {warnAboutMissingMockScheduler} from 'shared/ReactFeatureFlags';
 import enqueueTask from 'shared/enqueueTask';
 import * as Scheduler from 'scheduler';
 
@@ -43,26 +42,11 @@ const {IsSomeRendererActing} = ReactSharedInternals;
 // this implementation should be exactly the same in
 // ReactTestUtilsAct.js, ReactTestRendererAct.js, createReactNoop.js
 
-let hasWarnedAboutMissingMockScheduler = false;
 const isSchedulerMocked =
   typeof Scheduler.unstable_flushAllWithoutAsserting === 'function';
 const flushWork =
   Scheduler.unstable_flushAllWithoutAsserting ||
   function() {
-    if (warnAboutMissingMockScheduler === true) {
-      if (hasWarnedAboutMissingMockScheduler === false) {
-        warningWithoutStack(
-          null,
-          'Starting from React v17, the "scheduler" module will need to be mocked ' +
-            'to guarantee consistent behaviour across tests and browsers. To fix this, add the following ' +
-            "to the top of your tests, or in your framework's global config file -\n\n" +
-            'As an example, for jest - \n' +
-            "jest.mock('scheduler', () => require.requireActual('scheduler/unstable_mock'));\n\n" +
-            'For more info, visit https://fb.me/react-mock-scheduler',
-        );
-        hasWarnedAboutMissingMockScheduler = true;
-      }
-    }
     while (flushPassiveEffects()) {}
   };
 
diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js
@@ -27,7 +27,6 @@ import {REACT_FRAGMENT_TYPE, REACT_ELEMENT_TYPE} from 'shared/ReactSymbols';
 import enqueueTask from 'shared/enqueueTask';
 import ReactSharedInternals from 'shared/ReactSharedInternals';
 import warningWithoutStack from 'shared/warningWithoutStack';
-import {warnAboutMissingMockScheduler} from 'shared/ReactFeatureFlags';
 import {ConcurrentRoot, BatchedRoot, LegacyRoot} from 'shared/ReactRootTags';
 
 type Container = {
@@ -599,26 +598,11 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
   // this act() implementation should be exactly the same in
   // ReactTestUtilsAct.js, ReactTestRendererAct.js, createReactNoop.js
 
-  let hasWarnedAboutMissingMockScheduler = false;
   const isSchedulerMocked =
     typeof Scheduler.unstable_flushAllWithoutAsserting === 'function';
   const flushWork =
     Scheduler.unstable_flushAllWithoutAsserting ||
     function() {
-      if (warnAboutMissingMockScheduler === true) {
-        if (hasWarnedAboutMissingMockScheduler === false) {
-          warningWithoutStack(
-            null,
-            'Starting from React v17, the "scheduler" module will need to be mocked ' +
-              'to guarantee consistent behaviour across tests and browsers. To fix this, add the following ' +
-              "to the top of your tests, or in your framework's global config file -\n\n" +
-              'As an example, for jest - \n' +
-              "jest.mock('scheduler', () => require.requireActual('scheduler/unstable_mock'));\n\n" +
-              'For more info, visit https://fb.me/react-mock-scheduler',
-          );
-          hasWarnedAboutMissingMockScheduler = true;
-        }
-      }
       while (flushPassiveEffects()) {}
     };
 
diff --git a/packages/react-reconciler/src/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js
@@ -58,6 +58,7 @@ import {
   flushDiscreteUpdates,
   flushPassiveEffects,
   warnIfNotScopedWithMatchingAct,
+  warnIfNotMockedSchedulerInConcurrentOrBatchedMode,
   IsThisRendererActing,
 } from './ReactFiberWorkLoop';
 import {createUpdate, enqueueUpdate} from './ReactUpdateQueue';
@@ -314,6 +315,7 @@ export function updateContainer(
   if (__DEV__) {
     // $FlowExpectedError - jest isn't a global, and isn't recognized outside of tests
     if ('undefined' !== typeof jest) {
+      warnIfNotMockedSchedulerInConcurrentOrBatchedMode(current);
       warnIfNotScopedWithMatchingAct(current);
     }
   }
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js
diff --git a/packages/react-test-renderer/src/ReactTestRendererAct.js b/packages/react-test-renderer/src/ReactTestRendererAct.js
