await act(async () => ...) (#14853)

This took a while, but I'm happy I went through it. Some key moments - recursively flushing effects, flushing microtasks on each async turn, and my team's uncompromising philosophy on code reuse. Really happy with this. I still want to expand test coverage, and I have some more small related todos, but this is good to land. On to the next one. 

Soundtrack to landing this - https://open.spotify.com/track/0MF8I8OUo8kytiOo8aSHYq?si=gSWqUheKQbiQDXzptCXHTg

* hacked up act(async () => {...})

* move stuff around

* merge changes

* abstract .act warnings and stuff. all renderers. pass all tests.

* move testutils.act back into testutils

* move into scheduler, rename some bits

* smaller bundle

* a comment for why we don't do typeof === 'function'

* fix test

* pass tests - fire, prod

* lose actContainerElement

* tighter

* write a test for TestRenderer

it's an odd one, because not only does sync act not flush effects correctly, but the async one does (wut). verified it's fine with the dom version.

* lint

* rewrote to move flushing logic closer to the renderer

the scheduler's `flushPassiveEffects` didn't work as expected for the test renderer, so I decided to go back to the hack (rendering a dumb container) This also makes reactdom not as heavy (by a few bytes, but still).

* move it around so the delta isn't too bad

* cleanups

fix promise chaining
propagate errors correctly
test for thenable the 'right' way
more tests!
tidier!
ponies!

* Stray comment

* recursively flush effects

* fixed tests

* lint, move noop.act into react-reconciler

* microtasks when checking if called, s/called/calledLog, cleanup

* pass fb lint

we could have globally changed our eslint config to assume Promise is available, but that means we expect a promise polyfill on the page, and we don't yet. this code is triggered only in jest anyway, and we're fairly certain Promise will be available there. hence, the once-off disable for the check

* shorter timers, fix a test, test for Promise

* use global.Promise for existence check

* flush microtasks

* a version that works in browsers (that support postMessage)

I also added a sanity fixture inside fixtures/dom/ mostly for me.

* hoist flushEffectsAndMicroTasks

* pull out tick logic from ReactFiberScheduler

* fix await act (...sync) hanging

- fix a hang when awaiting sync logic
- a better async/await test for test renderer

* feedback changes

- use node's setImmediate if available
- a warning if MessageChannel isn't available
- rename some functions

* pass lint/flow checks (without requiring a Promise polyfill/exclusion)

* prettier

the prettiest, even.

* use globalPromise for the missed await warning

* __DEV__ check for didWarnAboutMessageChannel

* thenables and callbacks instead of promises, pass flow/lint

* tinier. better.

- pulled most bits out of FiberScheduler
- actedUpdates uses callbacks now

* pass build validation

* augh prettier

* golfing 7 more chars

* Test that effects are not flushed without also flushing microtasks

* export doesHavePendingPassiveEffects, nits

* createAct()

* dead code

* missed in merge?

* lose the preflushing bits

* ugh prettier

* removed `actedUpdates()`, created shared/actingUpdatesScopeDepth

* rearrange imports so builds work, remove the hack versions of flushPassiveEffects

* represent actingUpdatesScopeDepth as a tuple [number]

* use a shared flag on React.__SECRET...

* remove createAct, setup act for all relevant renderers

* review feedback

shared/enqueueTask

import ReactSharedInternals from 'shared/ReactSharedInternals';

simpler act() internals

ReactSharedInternals.ReactShouldWarnActingUpdates

* move act() implementation into createReactNoop

* warnIfNotCurrentlyActingUpdatesInDev condition check order

Author: Sunil Pai

fixtures/dom/.gitignore

@@ -14,6 +14,8 @@ public/react-dom.development.js
 public/react-dom.production.min.js
 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
 
 # misc
 .DS_Store

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 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 public/",
     "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

@@ -0,0 +1,41 @@
+<!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
+      function App() {
+        let [state, setState] = React.useState(0);
+        async function ticker() {
+          await null;
+          setState(x => x + 1);
+        }
+        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>

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

@@ -14,7 +14,6 @@ let React;
 let ReactDOM;
 let ReactDOMServer;
 let ReactTestUtils;
-let act;
 
 function getTestDocument(markup) {
   const doc = document.implementation.createHTMLDocument('');
@@ -34,7 +33,6 @@ describe('ReactTestUtils', () => {
     ReactDOM = require('react-dom');
     ReactDOMServer = require('react-dom/server');
     ReactTestUtils = require('react-dom/test-utils');
-    act = ReactTestUtils.act;
   });
 
   it('Simulate should have locally attached media events', () => {
@@ -517,173 +515,4 @@ describe('ReactTestUtils', () => {
     ReactTestUtils.renderIntoDocument(<Component />);
     expect(mockArgs.length).toEqual(0);
   });
-
-  it('can use act to batch effects', () => {
-    function App(props) {
-      React.useEffect(props.callback);
-      return null;
-    }
-    const container = document.createElement('div');
-    document.body.appendChild(container);
-
-    try {
-      let called = false;
-      act(() => {
-        ReactDOM.render(
-          <App
-            callback={() => {
-              called = true;
-            }}
-          />,
-          container,
-        );
-      });
-
-      expect(called).toBe(true);
-    } finally {
-      document.body.removeChild(container);
-    }
-  });
-
-  it('flushes effects on every call', () => {
-    function App(props) {
-      let [ctr, setCtr] = React.useState(0);
-      React.useEffect(() => {
-        props.callback(ctr);
-      });
-      return (
-        <button id="button" onClick={() => setCtr(x => x + 1)}>
-          click me!
-        </button>
-      );
-    }
-
-    const container = document.createElement('div');
-    document.body.appendChild(container);
-    let calledCtr = 0;
-    act(() => {
-      ReactDOM.render(
-        <App
-          callback={val => {
-            calledCtr = val;
-          }}
-        />,
-        container,
-      );
-    });
-    const button = document.getElementById('button');
-    function click() {
-      button.dispatchEvent(new MouseEvent('click', {bubbles: true}));
-    }
-
-    act(() => {
-      click();
-      click();
-      click();
-    });
-    expect(calledCtr).toBe(3);
-    act(click);
-    expect(calledCtr).toBe(4);
-    act(click);
-    expect(calledCtr).toBe(5);
-
-    document.body.removeChild(container);
-  });
-
-  it('can use act to batch effects on updates too', () => {
-    function App() {
-      let [ctr, setCtr] = React.useState(0);
-      return (
-        <button id="button" onClick={() => setCtr(x => x + 1)}>
-          {ctr}
-        </button>
-      );
-    }
-    const container = document.createElement('div');
-    document.body.appendChild(container);
-    let button;
-    act(() => {
-      ReactDOM.render(<App />, container);
-    });
-    button = document.getElementById('button');
-    expect(button.innerHTML).toBe('0');
-    act(() => {
-      button.dispatchEvent(new MouseEvent('click', {bubbles: true}));
-    });
-    expect(button.innerHTML).toBe('1');
-    document.body.removeChild(container);
-  });
-
-  it('detects setState being called outside of act(...)', () => {
-    let setValueRef = null;
-    function App() {
-      let [value, setValue] = React.useState(0);
-      setValueRef = setValue;
-      return (
-        <button id="button" onClick={() => setValue(2)}>
-          {value}
-        </button>
-      );
-    }
-    const container = document.createElement('div');
-    document.body.appendChild(container);
-    let button;
-    act(() => {
-      ReactDOM.render(<App />, container);
-      button = container.querySelector('#button');
-      button.dispatchEvent(new MouseEvent('click', {bubbles: true}));
-    });
-    expect(button.innerHTML).toBe('2');
-    expect(() => setValueRef(1)).toWarnDev([
-      'An update to App inside a test was not wrapped in act(...).',
-    ]);
-    document.body.removeChild(container);
-  });
-
-  it('lets a ticker update', () => {
-    function App() {
-      let [toggle, setToggle] = React.useState(0);
-      React.useEffect(() => {
-        let timeout = setTimeout(() => {
-          setToggle(1);
-        }, 200);
-        return () => clearTimeout(timeout);
-      });
-      return toggle;
-    }
-    const container = document.createElement('div');
-
-    act(() => {
-      act(() => {
-        ReactDOM.render(<App />, container);
-      });
-      jest.advanceTimersByTime(250);
-    });
-
-    expect(container.innerHTML).toBe('1');
-  });
-
-  it('warns if you return a value inside act', () => {
-    expect(() => act(() => null)).toWarnDev(
-      [
-        'The callback passed to ReactTestUtils.act(...) function must not return anything.',
-      ],
-      {withoutStack: true},
-    );
-    expect(() => act(() => 123)).toWarnDev(
-      [
-        'The callback passed to ReactTestUtils.act(...) function must not return anything.',
-      ],
-      {withoutStack: true},
-    );
-  });
-
-  it('warns if you try to await an .act call', () => {
-    expect(act(() => {}).then).toWarnDev(
-      [
-        'Do not await the result of calling ReactTestUtils.act(...), it is not a Promise.',
-      ],
-      {withoutStack: true},
-    );
-  });
 });