warn when using the wrong act() around create/updates

via #15319 This solves 2 specific problems - - using the 'wrong' act() doesn't silence the warning - using the wrong act logs a warning It does this by using an empty object on the reconciler as the identity of ReactShouldWarnActingUpdates.current. We also add this check when calling createContainer() to catch the common failure of this happening right in the beginning.
facebook · threepointone · Apr 3, 2019 · Apr 3, 2019 · Apr 3, 2019 · Apr 12, 2019
commit f0c3493de401eeefe225439c0425aaf3ea49ec4d
diff --git a/fixtures/dom/.gitignore b/fixtures/dom/.gitignore
@@ -16,6 +16,10 @@ public/react-dom-server.browser.development.js
 public/react-dom-server.browser.production.min.js
 public/react-dom-test-utils.development.js
 public/react-dom-test-utils.production.min.js
+public/react-test-renderer.development.js
+public/react-test-renderer.production.min.js
+public/scheduler.development.js
+public/scheduler.production.min.js
 
 # misc
 .DS_Store

diff --git a/fixtures/dom/package.json b/fixtures/dom/package.json
@@ -18,7 +18,7 @@
   },
   "scripts": {
     "start": "react-scripts start",
-    "prestart": "cp ../../build/node_modules/react/umd/react.development.js ../../build/node_modules/react-dom/umd/react-dom.development.js ../../build/node_modules/react/umd/react.production.min.js ../../build/node_modules/react-dom/umd/react-dom.production.min.js ../../build/node_modules/react-dom/umd/react-dom-server.browser.development.js ../../build/node_modules/react-dom/umd/react-dom-server.browser.production.min.js ../../build/node_modules/react-dom/umd/react-dom-test-utils.development.js ../../build/node_modules/react-dom/umd/react-dom-test-utils.production.min.js public/",
+    "prestart": "cp ../../build/node_modules/react/umd/react.development.js ../../build/node_modules/react-dom/umd/react-dom.development.js ../../build/node_modules/react/umd/react.production.min.js ../../build/node_modules/react-dom/umd/react-dom.production.min.js ../../build/node_modules/react-dom/umd/react-dom-server.browser.development.js ../../build/node_modules/react-dom/umd/react-dom-server.browser.production.min.js ../../build/node_modules/react-dom/umd/react-dom-test-utils.development.js ../../build/node_modules/react-dom/umd/react-dom-test-utils.production.min.js ../../build/node_modules/react-test-renderer/umd/react-test-renderer.development.js ../../build/node_modules/react-test-renderer/umd/react-test-renderer.production.min.js ../../build/node_modules/scheduler/umd/scheduler.development.js ../../build/node_modules/scheduler/umd/scheduler.production.min.js public/",
     "build": "react-scripts build && cp build/index.html build/200.html",
     "test": "react-scripts test --env=jsdom",
     "eject": "react-scripts eject"

diff --git a/fixtures/dom/public/act-dom.html b/fixtures/dom/public/act-dom.html
@@ -1,41 +1,58 @@
 <!DOCTYPE html>
 <html>
-<head>
-  <title>sanity test for ReactTestUtils.act</title>
-</head>
-<body>
-  this page tests whether act runs properly in a browser.
-  <br/>
-  your console should say "5"
-  <script src='react.development.js'></script>
-  <script src='react-dom.development.js'></script>
-  <script src='react-dom-test-utils.development.js'></script>
-  <script>
-    async function run(){
-      // from ReactTestUtilsAct-test.js
+  <head>
+    <title>sanity test for ReactTestUtils.act</title>
+  </head>
+  <body>
+    this page tests whether act runs properly in a browser.
+    <br />
+    your console should say "5", and a warning about not using the right act()
+    <script src="react.development.js"></script>
+    <script src="react-dom.development.js"></script>
+    <script src="react-dom-test-utils.development.js"></script>
+    <script src="scheduler.development.js"></script>
+    <script src="react-test-renderer.development.js"></script>
+    <script>
+      window.jest = {}; // to enable warnings
       function App() {
         let [state, setState] = React.useState(0);
         async function ticker() {
           await null;
           setState(x => x + 1);
         }
-        React.useEffect(
-          () => {
-            ticker();
-          },
-          [Math.min(state, 4)],
-        );
+        React.useEffect(() => {
+          ticker();
+        }, [Math.min(state, 4)]);
         return state;
       }
-      const el = document.createElement('div');
-      await ReactTestUtils.act(async () => {
-        ReactDOM.render(React.createElement(App), el);
-      });
-      // all 5 ticks present and accounted for
-      console.log(el.innerHTML);
-    }
-    run();
-
-  </script>
-</body>
-</html>
+
+      function sleep(period){
+        return new Promise(resolve => setTimeout(resolve, period))
+      }
+
+      async function testDOMAsynAct() {
+        // from ReactTestUtilsAct-test.js
+
+        const el = document.createElement("div");
+        await ReactTestUtils.act(async () => {
+          ReactDOM.render(React.createElement(App), el);
+        });
+        // all 5 ticks present and accounted for
+        console.log(el.innerHTML);
+      }
+
+      async function testMixRenderers() {
+        await ReactTestUtils.act(async () => {
+          ReactTestRenderer.create(React.createElement(App));
+        });
+      }
+
+      async function run() {
+        await testDOMAsynAct();
+        await testMixRenderers();
+      }
+
+      run();
+    </script>
+  </body>
+</html>
diff --git a/packages/react-dom/src/client/ReactDOM.js b/packages/react-dom/src/client/ReactDOM.js
@@ -36,6 +36,7 @@ import {
   findHostInstance,
   findHostInstanceWithWarning,
   flushPassiveEffects,
+  ReactActingUpdatesSigil,
 } from 'react-reconciler/inline.dom';
 import {createPortal as createPortalImpl} from 'shared/ReactPortal';
 import {canUseDOM} from 'shared/ExecutionEnvironment';
@@ -822,6 +823,7 @@ const ReactDOM: Object = {
       dispatchEvent,
       runEventsInBatch,
       flushPassiveEffects,
+      ReactActingUpdatesSigil,
     ],
   },
 };

diff --git a/packages/react-dom/src/fire/ReactFire.js b/packages/react-dom/src/fire/ReactFire.js
@@ -41,6 +41,7 @@ import {
   findHostInstance,
   findHostInstanceWithWarning,
   flushPassiveEffects,
+  ReactActingUpdatesSigil,
 } from 'react-reconciler/inline.fire';
 import {createPortal as createPortalImpl} from 'shared/ReactPortal';
 import {canUseDOM} from 'shared/ExecutionEnvironment';
@@ -828,6 +829,7 @@ const ReactDOM: Object = {
       dispatchEvent,
       runEventsInBatch,
       flushPassiveEffects,
+      ReactActingUpdatesSigil,
     ],
   },
 };

diff --git a/packages/react-dom/src/test-utils/ReactTestUtils.js b/packages/react-dom/src/test-utils/ReactTestUtils.js
@@ -42,8 +42,10 @@ const [
   restoreStateIfNeeded,
   dispatchEvent,
   runEventsInBatch,
-  // eslint-disable-next-line no-unused-vars
+  /* eslint-disable no-unused-vars */
   flushPassiveEffects,
+  ReactActingUpdatesSigil,
+  /* eslint-enable no-unused-vars */
 ] = ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Events;
 
 function Event(suffix) {}

diff --git a/packages/react-dom/src/test-utils/ReactTestUtilsAct.js b/packages/react-dom/src/test-utils/ReactTestUtilsAct.js
@@ -31,6 +31,7 @@ const [
   runEventsInBatch,
   /* eslint-enable no-unused-vars */
   flushPassiveEffects,
+  ReactActingUpdatesSigil,
 ] = ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Events;
 
 const batchedUpdates = ReactDOM.unstable_batchedUpdates;
@@ -61,18 +62,18 @@ function flushEffectsAndMicroTasks(onDone: (err: ?Error) => void) {
 
 function act(callback: () => Thenable) {
   let previousActingUpdatesScopeDepth;
+  let previousActingUpdatesSigil;
   if (__DEV__) {
     previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
+    previousActingUpdatesSigil = ReactShouldWarnActingUpdates.current;
     actingUpdatesScopeDepth++;
-    ReactShouldWarnActingUpdates.current = true;
+    ReactShouldWarnActingUpdates.current = ReactActingUpdatesSigil;
   }
 
   function onDone() {
     if (__DEV__) {
       actingUpdatesScopeDepth--;
-      if (actingUpdatesScopeDepth === 0) {
-        ReactShouldWarnActingUpdates.current = false;
-      }
+      ReactShouldWarnActingUpdates.current = previousActingUpdatesSigil;
       if (actingUpdatesScopeDepth > previousActingUpdatesScopeDepth) {
         // if it's _less than_ previousActingUpdatesScopeDepth, then we can assume the 'other' one has warned
         warningWithoutStack(

diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js
@@ -645,11 +645,17 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
   const roots = new Map();
   const DEFAULT_ROOT_ID = '<default>';
 
-  const {flushPassiveEffects, batchedUpdates} = NoopRenderer;
+  const {
+    flushPassiveEffects,
+    batchedUpdates,
+    ReactActingUpdatesSigil,
+  } = NoopRenderer;
 
   // this act() implementation should be exactly the same in
   // ReactTestUtilsAct.js, ReactTestRendererAct.js, createReactNoop.js
 
+  // we track the 'depth' of the act() calls with this counter,
+  // so we can tell if any async act() calls try to run in parallel.
   let actingUpdatesScopeDepth = 0;
 
   function flushEffectsAndMicroTasks(onDone: (err: ?Error) => void) {
@@ -669,18 +675,18 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
 
   function act(callback: () => Thenable) {
     let previousActingUpdatesScopeDepth;
+    let previousActingUpdatesSigil;
     if (__DEV__) {
       previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
+      previousActingUpdatesSigil = ReactShouldWarnActingUpdates.current;
       actingUpdatesScopeDepth++;
-      ReactShouldWarnActingUpdates.current = true;
+      ReactShouldWarnActingUpdates.current = ReactActingUpdatesSigil;
     }
 
     function onDone() {
       if (__DEV__) {
         actingUpdatesScopeDepth--;
-        if (actingUpdatesScopeDepth === 0) {
-          ReactShouldWarnActingUpdates.current = false;
-        }
+        ReactShouldWarnActingUpdates.current = previousActingUpdatesSigil;
         if (actingUpdatesScopeDepth > previousActingUpdatesScopeDepth) {
           // if it's _less than_ previousActingUpdatesScopeDepth, then we can assume the 'other' one has warned
           warningWithoutStack(

diff --git a/packages/react-reconciler/src/ReactActingUpdatesSigil.js b/packages/react-reconciler/src/ReactActingUpdatesSigil.js
@@ -0,0 +1,10 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow
+ */
+
+export default {};
diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js
@@ -35,6 +35,7 @@ import {
   requestCurrentTime,
   warnIfNotCurrentlyActingUpdatesInDev,
   markRenderEventTime,
+  warnIfNotScopedWithMatchingAct,
 } from './ReactFiberScheduler';
 
 import invariant from 'shared/invariant';
@@ -1180,6 +1181,7 @@ function dispatchAction<S, A>(
       // further, this isn't a test file, so flow doesn't recognize the symbol. So...
       // $FlowExpectedError - because requirements don't give a damn about your type sigs.
       if ('undefined' !== typeof jest) {
+        warnIfNotScopedWithMatchingAct(fiber);
         warnIfNotCurrentlyActingUpdatesInDev(fiber);
       }
     }

diff --git a/packages/react-reconciler/src/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js
@@ -53,6 +53,7 @@ import {
   interactiveUpdates,
   flushInteractiveUpdates,
   flushPassiveEffects,
+  warnIfNotScopedWithMatchingAct,
 } from './ReactFiberScheduler';
 import {createUpdate, enqueueUpdate} from './ReactUpdateQueue';
 import ReactFiberInstrumentation from './ReactFiberInstrumentation';
@@ -63,6 +64,7 @@ import {
 } from './ReactCurrentFiber';
 import {StrictMode} from './ReactTypeOfMode';
 import {Sync} from './ReactFiberExpirationTime';
+import ReactActingUpdatesSigil from './ReactActingUpdatesSigil';
 
 type OpaqueRoot = FiberRoot;
 
@@ -276,7 +278,16 @@ export function createContainer(
   isConcurrent: boolean,
   hydrate: boolean,
 ): OpaqueRoot {
-  return createFiberRoot(containerInfo, isConcurrent, hydrate);
+  const fiberRoot = createFiberRoot(containerInfo, isConcurrent, hydrate);
+  if (__DEV__) {
+    // jest isn't a 'global', it's just exposed to tests via a wrapped function
+    // further, this isn't a test file, so flow doesn't recognize the symbol. So...
+    // $FlowExpectedError - because requirements don't give a damn about your type sigs.
+    if ('undefined' !== typeof jest) {
+      warnIfNotScopedWithMatchingAct(fiberRoot.current);
+    }
+  }
+  return fiberRoot;
 }
 
 export function updateContainer(
@@ -455,3 +466,5 @@ export function injectIntoDevTools(devToolsConfig: DevToolsConfig): boolean {
     },
   });
 }
+
+export {ReactActingUpdatesSigil};
diff --git a/packages/react-reconciler/src/ReactFiberScheduler.js b/packages/react-reconciler/src/ReactFiberScheduler.js
@@ -159,6 +159,7 @@ import {
   clearCaughtError,
 } from 'shared/ReactErrorUtils';
 import {onCommitRoot} from './ReactFiberDevToolsHook';
+import ReactActingUpdatesSigil from './ReactActingUpdatesSigil';
 
 const ceil = Math.ceil;
 
@@ -2077,11 +2078,38 @@ function warnAboutInvalidUpdatesOnClassComponentsInDEV(fiber) {
   }
 }
 
+export function warnIfNotScopedWithMatchingAct(fiber: Fiber): void {
+  if (__DEV__) {
+    if (
+      ReactShouldWarnActingUpdates.current !== null &&
+      ReactShouldWarnActingUpdates.current !== ReactActingUpdatesSigil
+    ) {
+      // it looks like we're using the wrong matching act(), so log a warning
+      warningWithoutStack(
+        false,
+        "It looks like you're using the wrong act() around your interactions.\n" +
+          'Be sure to use the matching version of act() corresponding to your renderer. e.g. -\n' +
+          "for react-dom, import {act} from 'react-test-utils';\n" +
+          'for react-test-renderer, const {act} = TestRenderer.' +
+          '%s',
+        getStackByFiberInDevAndProd(fiber),
+      );
+    }
+  }
+}
+
+// in a test-like environment, we want to warn if dispatchAction() is
+// called outside of a TestUtils.act(...)/batchedUpdates/render call.
+// so we have a a step counter for when we descend/ascend from
+// act() calls, and test on it for when to warn
+// It's a tuple with a single value. Look into ReactTestUtilsAct as an
+// example of how we change the value
+
 function warnIfNotCurrentlyActingUpdatesInDEV(fiber: Fiber): void {
   if (__DEV__) {
     if (
       workPhase === NotWorking &&
-      ReactShouldWarnActingUpdates.current === false
+      ReactShouldWarnActingUpdates.current !== ReactActingUpdatesSigil
     ) {
       warningWithoutStack(
         false,

diff --git a/packages/react-test-renderer/src/ReactTestRendererAct.js b/packages/react-test-renderer/src/ReactTestRendererAct.js
@@ -11,6 +11,7 @@ import type {Thenable} from 'react-reconciler/src/ReactFiberScheduler';
 import {
   batchedUpdates,
   flushPassiveEffects,
+  ReactActingUpdatesSigil,
 } from 'react-reconciler/inline.test';
 import ReactSharedInternals from 'shared/ReactSharedInternals';
 import warningWithoutStack from 'shared/warningWithoutStack';
@@ -42,18 +43,18 @@ function flushEffectsAndMicroTasks(onDone: (err: ?Error) => void) {
 
 function act(callback: () => Thenable) {
   let previousActingUpdatesScopeDepth;
+  let previousActingUpdatesSigil;
   if (__DEV__) {
     previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
+    previousActingUpdatesSigil = ReactShouldWarnActingUpdates.current;
     actingUpdatesScopeDepth++;
-    ReactShouldWarnActingUpdates.current = true;
+    ReactShouldWarnActingUpdates.current = ReactActingUpdatesSigil;
   }
 
   function onDone() {
     if (__DEV__) {
       actingUpdatesScopeDepth--;
-      if (actingUpdatesScopeDepth === 0) {
-        ReactShouldWarnActingUpdates.current = false;
-      }
+      ReactShouldWarnActingUpdates.current = previousActingUpdatesSigil;
       if (actingUpdatesScopeDepth > previousActingUpdatesScopeDepth) {
         // if it's _less than_ previousActingUpdatesScopeDepth, then we can assume the 'other' one has warned
         warningWithoutStack(

diff --git a/packages/react/src/ReactSharedInternals.js b/packages/react/src/ReactSharedInternals.js
@@ -11,12 +11,12 @@ import * as SchedulerTracing from 'scheduler/tracing';
 import ReactCurrentDispatcher from './ReactCurrentDispatcher';
 import ReactCurrentOwner from './ReactCurrentOwner';
 import ReactDebugCurrentFrame from './ReactDebugCurrentFrame';
+import ReactShouldWarnActingUpdates from './ReactShouldWarnActingUpdates';
 
 const ReactSharedInternals = {
   ReactCurrentDispatcher,
   ReactCurrentOwner,
-  // used by act()
-  ReactShouldWarnActingUpdates: {current: false},
+  ReactShouldWarnActingUpdates,
   // Used by renderers to avoid bundling object-assign twice in UMD bundles:
   assign,
 };