using the wrong renderer's act() should warn

fixtures/dom/package.json

@@ -18,7 +18,7 @@
   },
   "scripts": {
     "start": "react-scripts start",
-    "prestart": "cp ../../build/node_modules/scheduler/umd/scheduler-unstable_mock.development.js ../../build/node_modules/scheduler/umd/scheduler-unstable_mock.production.min.js ../../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/scheduler/umd/scheduler-unstable_mock.development.js ../../build/node_modules/scheduler/umd/scheduler-unstable_mock.production.min.js ../../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/ && cp -a ../../build/node_modules/. node_modules",
     "build": "react-scripts build && cp build/index.html build/200.html",
     "test": "react-scripts test --env=jsdom",
     "eject": "react-scripts eject"

fixtures/dom/public/act-dom.html

@@ -15,31 +15,29 @@
   <script src='react-dom.development.js'></script>
   <script src='react-dom-test-utils.development.js'></script>
   <script>
-    async function run(){
       // from ReactTestUtilsAct-test.js
       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>
+
+      async function testAsyncAct() {
+        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);
+      }
+
+      testAsyncAct();
+    </script>
+  </body>
 </html>

fixtures/dom/src/index.test.js

@@ -0,0 +1,98 @@
+import React from 'react';
+import ReactDOM from 'react-dom';
+import TestUtils from 'react-dom/test-utils';
+import TestRenderer from 'react-test-renderer';
+
+let spy;
+beforeEach(() => {
+  spy = jest.spyOn(console, 'error').mockImplementation(() => {});
+});
+
+function confirmWarning() {
+  expect(spy).toHaveBeenCalledWith(
+    expect.stringContaining(
+      "It looks like you're using the wrong act() around your test interactions."
+    ),
+    ''
+  );
+}
+
+function App(props) {
+  return 'hello world';
+}
+
+it("doesn't warn when you use the right act + renderer: dom", () => {
+  TestUtils.act(() => {
+    TestUtils.renderIntoDocument(<App />);
+  });
+  expect(spy).not.toHaveBeenCalled();
+});
+
+it("doesn't warn when you use the right act + renderer: test", () => {
+  TestRenderer.act(() => {
+    TestRenderer.create(<App />);
+  });
+  expect(spy).not.toHaveBeenCalled();
+});
+
+it('works with createRoot().render combo', () => {
+  const root = ReactDOM.unstable_createRoot(document.createElement('div'));
+  TestRenderer.act(() => {
+    root.render(<App />);
+  });
+  confirmWarning();
+});
+
+it('warns when using the wrong act version - test + dom: render', () => {
+  TestRenderer.act(() => {
+    TestUtils.renderIntoDocument(<App />);
+  });
+  confirmWarning();
+});
+
+it('warns when using the wrong act version - test + dom: updates', () => {
+  let setCtr;
+  function Counter(props) {
+    const [ctr, _setCtr] = React.useState(0);
+    setCtr = _setCtr;
+    return ctr;
+  }
+  TestUtils.renderIntoDocument(<Counter />);
+  TestRenderer.act(() => {
+    setCtr(1);
+  });
+  confirmWarning();
+});
+
+it('warns when using the wrong act version - dom + test: .create()', () => {
+  TestUtils.act(() => {
+    TestRenderer.create(<App />);
+  });
+  confirmWarning();
+});
+
+it('warns when using the wrong act version - dom + test: .update()', () => {
+  let root;
+  // use the right one here so we don't get the first warning
+  TestRenderer.act(() => {
+    root = TestRenderer.create(<App key="one" />);
+  });
+  TestUtils.act(() => {
+    root.update(<App key="two" />);
+  });
+  confirmWarning();
+});
+
+it('warns when using the wrong act version - dom + test: updates', () => {
+  let setCtr;
+  function Counter(props) {
+    const [ctr, _setCtr] = React.useState(0);
+    setCtr = _setCtr;
+    return ctr;
+  }
+  const root = TestRenderer.create(<Counter />);
+  TestUtils.act(() => {
+    setCtr(1);
+  });
+  confirmWarning();
+});

package.json

@@ -107,6 +107,7 @@
     "test-prod-build": "yarn test-build-prod",
     "test-build": "cross-env NODE_ENV=development jest --config ./scripts/jest/config.build.js",
     "test-build-prod": "cross-env NODE_ENV=production jest --config ./scripts/jest/config.build.js",
+    "test-dom-fixture": "cd fixtures/dom && yarn && yarn prestart && yarn test",
     "flow": "node ./scripts/tasks/flow.js",
     "flow-ci": "node ./scripts/tasks/flow-ci.js",
     "prettier": "node ./scripts/prettier/index.js write-changed",

packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js

@@ -259,13 +259,13 @@ function runActTests(label, render, unmount) {
       it('warns if you return a value inside act', () => {
         expect(() => act(() => null)).toWarnDev(
           [
-            'The callback passed to act(...) function must return undefined, or a Promise.',
+            'The callback passed to act(...) function must return undefined or a Promise.',
           ],
           {withoutStack: true},
         );
         expect(() => act(() => 123)).toWarnDev(
           [
-            'The callback passed to act(...) function must return undefined, or a Promise.',
+            'The callback passed to act(...) function must return undefined or a Promise.',
           ],
           {withoutStack: true},
         );
@@ -274,7 +274,7 @@ function runActTests(label, render, unmount) {
       it('warns if you try to await a sync .act call', () => {
         expect(() => act(() => {}).then(() => {})).toWarnDev(
           [
-            'Do not await the result of calling act(...) with sync logic, it is not a Promise.',
+            'Do not await the result of calling a synchronous act(...), it is not a Promise.',
           ],
           {withoutStack: true},
         );
@@ -337,7 +337,7 @@ function runActTests(label, render, unmount) {
         if (__DEV__) {
           expect(console.error.calls.count()).toEqual(1);
           expect(console.error.calls.argsFor(0)[0]).toMatch(
-            'You called act(async () => ...) without await.',
+            'You called act(async () => ...) without awaiting its result.',
           );
         }
       });

packages/react-dom/src/test-utils/ReactTestUtils.js

@@ -42,8 +42,9 @@ const [
   restoreStateIfNeeded,
   dispatchEvent,
   runEventsInBatch,
-  // eslint-disable-next-line no-unused-vars
+  /* eslint-disable no-unused-vars */
   flushPassiveEffects,
+  /* eslint-enable no-unused-vars */
 ] = ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Events;
 
 function Event(suffix) {}

packages/react-dom/src/test-utils/ReactTestUtilsAct.js

@@ -37,7 +37,7 @@ const [
 
 const batchedUpdates = ReactDOM.unstable_batchedUpdates;
 
-const {ReactShouldWarnActingUpdates} = ReactSharedInternals;
+const {ReactActingRendererSigil} = ReactSharedInternals;
 
 // this implementation should be exactly the same in
 // ReactTestUtilsAct.js, ReactTestRendererAct.js, createReactNoop.js
@@ -85,18 +85,20 @@ let actingUpdatesScopeDepth = 0;
 
 function act(callback: () => Thenable) {
   let previousActingUpdatesScopeDepth;
+  let previousActingUpdatesSigil;
   if (__DEV__) {
     previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
+    previousActingUpdatesSigil = ReactActingRendererSigil.current;
     actingUpdatesScopeDepth++;
-    ReactShouldWarnActingUpdates.current = true;
+    // we use the function flushPassiveEffects directly as the sigil,
+    // since it's unique to a renderer
+    ReactActingRendererSigil.current = flushPassiveEffects;
   }
 
   function onDone() {
     if (__DEV__) {
       actingUpdatesScopeDepth--;
-      if (actingUpdatesScopeDepth === 0) {
-        ReactShouldWarnActingUpdates.current = false;
-      }
+      ReactActingRendererSigil.current = previousActingUpdatesSigil;
       if (actingUpdatesScopeDepth > previousActingUpdatesScopeDepth) {
         // if it's _less than_ previousActingUpdatesScopeDepth, then we can assume the 'other' one has warned
         warningWithoutStack(
@@ -126,9 +128,10 @@ function act(callback: () => Thenable) {
             if (called === false) {
               warningWithoutStack(
                 null,
-                'You called act(async () => ...) without await. ' +
+                'You called act(async () => ...) without awaiting its result. ' +
                   'This could lead to unexpected testing behaviour, interleaving multiple act ' +
-                  'calls and mixing their scopes. You should - await act(async () => ...);',
+                  'calls and mixing their scopes. You should await asynchronous act() calls:\n' +
+                  'await act(async () => ...);\n',
               );
             }
           });
@@ -164,7 +167,7 @@ function act(callback: () => Thenable) {
       warningWithoutStack(
         result === undefined,
         'The callback passed to act(...) function ' +
-          'must return undefined, or a Promise. You returned %s',
+          'must return undefined or a Promise. You returned %s',
         result,
       );
     }
@@ -184,7 +187,8 @@ function act(callback: () => Thenable) {
         if (__DEV__) {
           warningWithoutStack(
             false,
-            'Do not await the result of calling act(...) with sync logic, it is not a Promise.',
+            'Do not await the result of calling a synchronous act(...), it is not a Promise. \n' +
+              'Remove the `await` statement before this act() call.',
           );
         }
         resolve();

packages/react-noop-renderer/src/createReactNoop.js

@@ -81,7 +81,7 @@ type TextInstance = {|
 |};
 type HostContext = Object;
 
-const {ReactShouldWarnActingUpdates} = ReactSharedInternals;
+const {ReactActingRendererSigil} = ReactSharedInternals;
 
 const NO_CONTEXT = {};
 const UPPERCASE_CONTEXT = {};
@@ -698,18 +698,20 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
 
   function act(callback: () => Thenable) {
     let previousActingUpdatesScopeDepth;
+    let previousActingUpdatesSigil;
     if (__DEV__) {
       previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
+      previousActingUpdatesSigil = ReactActingRendererSigil.current;
       actingUpdatesScopeDepth++;
-      ReactShouldWarnActingUpdates.current = true;
+      // we use the function flushPassiveEffects directly as the sigil,
+      // since it's unique to a renderer
+      ReactActingRendererSigil.current = flushPassiveEffects;
     }
 
     function onDone() {
       if (__DEV__) {
         actingUpdatesScopeDepth--;
-        if (actingUpdatesScopeDepth === 0) {
-          ReactShouldWarnActingUpdates.current = false;
-        }
+        ReactActingRendererSigil.current = previousActingUpdatesSigil;
         if (actingUpdatesScopeDepth > previousActingUpdatesScopeDepth) {
           // if it's _less than_ previousActingUpdatesScopeDepth, then we can assume the 'other' one has warned
           warningWithoutStack(
@@ -739,9 +741,10 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
               if (called === false) {
                 warningWithoutStack(
                   null,
-                  'You called act(async () => ...) without await. ' +
+                  'You called act(async () => ...) without awaiting its result. ' +
                     'This could lead to unexpected testing behaviour, interleaving multiple act ' +
-                    'calls and mixing their scopes. You should - await act(async () => ...);',
+                    'calls and mixing their scopes. You should await asynchronous act() calls:\n' +
+                    'await act(async () => ...);\n',
                 );
               }
             });
@@ -777,7 +780,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
         warningWithoutStack(
           result === undefined,
           'The callback passed to act(...) function ' +
-            'must return undefined, or a Promise. You returned %s',
+            'must return undefined or a Promise. You returned %s',
           result,
         );
       }
@@ -797,7 +800,8 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
           if (__DEV__) {
             warningWithoutStack(
               false,
-              'Do not await the result of calling act(...) with sync logic, it is not a Promise.',
+              'Do not await the result of calling a synchronous act(...), it is not a Promise. \n' +
+                'Remove the `await` statement before this act() call.',
             );
           }
           resolve();

packages/react-reconciler/src/ReactFiberHooks.js

@@ -36,6 +36,7 @@ import {
   requestCurrentTime,
   warnIfNotCurrentlyActingUpdatesInDev,
   markRenderEventTimeAndConfig,
+  warnIfNotScopedWithMatchingAct,
 } from './ReactFiberScheduler';
 
 import invariant from 'shared/invariant';
@@ -1211,6 +1212,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);
       }
     }

packages/react-reconciler/src/ReactFiberReconciler.js

@@ -55,6 +55,7 @@ import {
   interactiveUpdates,
   flushInteractiveUpdates,
   flushPassiveEffects,
+  warnIfNotScopedWithMatchingAct,
 } from './ReactFiberScheduler';
 import {createUpdate, enqueueUpdate} from './ReactUpdateQueue';
 import ReactFiberInstrumentation from './ReactFiberInstrumentation';
@@ -302,6 +303,14 @@ export function updateContainer(
 ): ExpirationTime {
   const current = container.current;
   const currentTime = requestCurrentTime();
+  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(current);
+    }
+  }
   const suspenseConfig = requestCurrentSuspenseConfig();
   const expirationTime = computeExpirationForFiber(
     currentTime,