Allow async blocks in to(Error|Warn)Dev (#25338)

facebook · Dec 1, 2022 · 3ba7add · 3ba7add
1 parent f197ca9
commit 3ba7add
Show file tree

Hide file tree

Showing 5 changed files with 113 additions and 52 deletions.
diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js
@@ -1777,7 +1777,7 @@ describe('ReactHooksWithNoopRenderer', () => {
         }, [props.count]);
         return <Text text={'Count: ' + count} />;
       }
-      expect(() =>
+      expect(() => {
         act(() => {
           ReactNoop.render(<Counter count={0} />, () =>
             Scheduler.unstable_yieldValue('Sync effect'),
@@ -1787,8 +1787,8 @@ describe('ReactHooksWithNoopRenderer', () => {
             'Sync effect',
           ]);
           expect(ReactNoop.getChildren()).toEqual([span('Count: (empty)')]);
-        }),
-      ).toErrorDev('flushSync was called from inside a lifecycle method');
+        });
+      }).toErrorDev('flushSync was called from inside a lifecycle method');
       expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]);
     });
 
@@ -2648,32 +2648,32 @@ describe('ReactHooksWithNoopRenderer', () => {
       }
 
       const root1 = ReactNoop.createRoot();
-      expect(() =>
+      expect(() => {
         act(() => {
           root1.render(<App return={17} />);
-        }),
-      ).toErrorDev([
+        });
+      }).toErrorDev([
         'Warning: useEffect must not return anything besides a ' +
           'function, which is used for clean-up. You returned: 17',
       ]);
 
       const root2 = ReactNoop.createRoot();
-      expect(() =>
+      expect(() => {
         act(() => {
           root2.render(<App return={null} />);
-        }),
-      ).toErrorDev([
+        });
+      }).toErrorDev([
         'Warning: useEffect must not return anything besides a ' +
           'function, which is used for clean-up. You returned null. If your ' +
           'effect does not require clean up, return undefined (or nothing).',
       ]);
 
       const root3 = ReactNoop.createRoot();
-      expect(() =>
+      expect(() => {
         act(() => {
           root3.render(<App return={Promise.resolve()} />);
-        }),
-      ).toErrorDev([
+        });
+      }).toErrorDev([
         'Warning: useEffect must not return anything besides a ' +
           'function, which is used for clean-up.\n\n' +
           'It looks like you wrote useEffect(async () => ...) or returned a Promise.',
@@ -3052,32 +3052,32 @@ describe('ReactHooksWithNoopRenderer', () => {
       }
 
       const root1 = ReactNoop.createRoot();
-      expect(() =>
+      expect(() => {
         act(() => {
           root1.render(<App return={17} />);
-        }),
-      ).toErrorDev([
+        });
+      }).toErrorDev([
         'Warning: useInsertionEffect must not return anything besides a ' +
           'function, which is used for clean-up. You returned: 17',
       ]);
 
       const root2 = ReactNoop.createRoot();
-      expect(() =>
+      expect(() => {
         act(() => {
           root2.render(<App return={null} />);
-        }),
-      ).toErrorDev([
+        });
+      }).toErrorDev([
         'Warning: useInsertionEffect must not return anything besides a ' +
           'function, which is used for clean-up. You returned null. If your ' +
           'effect does not require clean up, return undefined (or nothing).',
       ]);
 
       const root3 = ReactNoop.createRoot();
-      expect(() =>
+      expect(() => {
         act(() => {
           root3.render(<App return={Promise.resolve()} />);
-        }),
-      ).toErrorDev([
+        });
+      }).toErrorDev([
         'Warning: useInsertionEffect must not return anything besides a ' +
           'function, which is used for clean-up.\n\n' +
           'It looks like you wrote useInsertionEffect(async () => ...) or returned a Promise.',
@@ -3104,11 +3104,11 @@ describe('ReactHooksWithNoopRenderer', () => {
       }
 
       const root = ReactNoop.createRoot();
-      expect(() =>
+      expect(() => {
         act(() => {
           root.render(<App />);
-        }),
-      ).toErrorDev(['Warning: useInsertionEffect must not schedule updates.']);
+        });
+      }).toErrorDev(['Warning: useInsertionEffect must not schedule updates.']);
 
       expect(() => {
         act(() => {
@@ -3359,32 +3359,32 @@ describe('ReactHooksWithNoopRenderer', () => {
       }
 
       const root1 = ReactNoop.createRoot();
-      expect(() =>
+      expect(() => {
         act(() => {
           root1.render(<App return={17} />);
-        }),
-      ).toErrorDev([
+        });
+      }).toErrorDev([
         'Warning: useLayoutEffect must not return anything besides a ' +
           'function, which is used for clean-up. You returned: 17',
       ]);
 
       const root2 = ReactNoop.createRoot();
-      expect(() =>
+      expect(() => {
         act(() => {
           root2.render(<App return={null} />);
-        }),
-      ).toErrorDev([
+        });
+      }).toErrorDev([
         'Warning: useLayoutEffect must not return anything besides a ' +
           'function, which is used for clean-up. You returned null. If your ' +
           'effect does not require clean up, return undefined (or nothing).',
       ]);
 
       const root3 = ReactNoop.createRoot();
-      expect(() =>
+      expect(() => {
         act(() => {
           root3.render(<App return={Promise.resolve()} />);
-        }),
-      ).toErrorDev([
+        });
+      }).toErrorDev([
         'Warning: useLayoutEffect must not return anything besides a ' +
           'function, which is used for clean-up.\n\n' +
           'It looks like you wrote useLayoutEffect(async () => ...) or returned a Promise.',

diff --git a/packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee b/packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee
@@ -133,6 +133,7 @@ describe 'ReactCoffeeScriptClass', ->
     expect(->
       act ->
         root.render React.createElement(Foo, foo: 'foo')
+      return
     ).toErrorDev 'Foo: getDerivedStateFromProps() is defined as an instance method and will be ignored. Instead, declare it as a static method.'
 
   it 'warns if getDerivedStateFromError is not static', ->
@@ -144,6 +145,7 @@ describe 'ReactCoffeeScriptClass', ->
     expect(->
       act ->
         root.render React.createElement(Foo, foo: 'foo')
+      return
     ).toErrorDev 'Foo: getDerivedStateFromError() is defined as an instance method and will be ignored. Instead, declare it as a static method.'
 
   it 'warns if getSnapshotBeforeUpdate is static', ->
@@ -155,6 +157,7 @@ describe 'ReactCoffeeScriptClass', ->
     expect(->
       act ->
         root.render React.createElement(Foo, foo: 'foo')
+      return
     ).toErrorDev 'Foo: getSnapshotBeforeUpdate() is defined as a static method and will be ignored. Instead, declare it as an instance method.'
 
   it 'warns if state not initialized before static getDerivedStateFromProps', ->
@@ -171,6 +174,7 @@ describe 'ReactCoffeeScriptClass', ->
     expect(->
       act ->
         root.render React.createElement(Foo, foo: 'foo')
+      return
     ).toErrorDev (
       '`Foo` uses `getDerivedStateFromProps` but its initial state is ' +
       'undefined. This is not recommended. Instead, define the initial state by ' +

diff --git a/packages/react/src/__tests__/ReactES6Class-test.js b/packages/react/src/__tests__/ReactES6Class-test.js
@@ -149,7 +149,9 @@ describe('ReactES6Class', () => {
         return <div />;
       }
     }
-    expect(() => act(() => root.render(<Foo foo="foo" />))).toErrorDev(
+    expect(() => {
+      act(() => root.render(<Foo foo="foo" />));
+    }).toErrorDev(
       'Foo: getDerivedStateFromProps() is defined as an instance method ' +
         'and will be ignored. Instead, declare it as a static method.',
     );
@@ -164,7 +166,9 @@ describe('ReactES6Class', () => {
         return <div />;
       }
     }
-    expect(() => act(() => root.render(<Foo foo="foo" />))).toErrorDev(
+    expect(() => {
+      act(() => root.render(<Foo foo="foo" />));
+    }).toErrorDev(
       'Foo: getDerivedStateFromError() is defined as an instance method ' +
         'and will be ignored. Instead, declare it as a static method.',
     );
@@ -177,7 +181,9 @@ describe('ReactES6Class', () => {
         return <div />;
       }
     }
-    expect(() => act(() => root.render(<Foo foo="foo" />))).toErrorDev(
+    expect(() => {
+      act(() => root.render(<Foo foo="foo" />));
+    }).toErrorDev(
       'Foo: getSnapshotBeforeUpdate() is defined as a static method ' +
         'and will be ignored. Instead, declare it as an instance method.',
     );
@@ -195,7 +201,9 @@ describe('ReactES6Class', () => {
         return <div className={`${this.state.foo} ${this.state.bar}`} />;
       }
     }
-    expect(() => act(() => root.render(<Foo foo="foo" />))).toErrorDev(
+    expect(() => {
+      act(() => root.render(<Foo foo="foo" />));
+    }).toErrorDev(
       '`Foo` uses `getDerivedStateFromProps` but its initial state is ' +
         'undefined. This is not recommended. Instead, define the initial state by ' +
         'assigning an object to `this.state` in the constructor of `Foo`. ' +

diff --git a/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js b/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js
@@ -14,6 +14,7 @@ let useSyncExternalStoreWithSelector;
 let React;
 let ReactDOM;
 let ReactDOMClient;
+let ReactFeatureFlags;
 let Scheduler;
 let act;
 let useState;
@@ -48,6 +49,7 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
     React = require('react');
     ReactDOM = require('react-dom');
     ReactDOMClient = require('react-dom/client');
+    ReactFeatureFlags = require('shared/ReactFeatureFlags');
     Scheduler = require('scheduler');
     useState = React.useState;
     useEffect = React.useEffect;
@@ -882,8 +884,7 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
 
   describe('selector and isEqual error handling in extra', () => {
     let ErrorBoundary;
-    beforeAll(() => {
-      spyOnDev(console, 'warn');
+    beforeEach(() => {
       ErrorBoundary = class extends React.Component {
         state = {error: null};
         static getDerivedStateFromError(error) {
@@ -929,9 +930,15 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
 
       expect(container.textContent).toEqual('A');
 
-      await act(() => {
-        store.set({});
-      });
+      await expect(async () => {
+        await act(async () => {
+          store.set({});
+        });
+      }).toWarnDev(
+        ReactFeatureFlags.enableUseRefAccessWarning
+          ? ['Warning: App: Unsafe read of a mutable value during render.']
+          : [],
+      );
       expect(container.textContent).toEqual('Malformed state');
     });
 
@@ -968,9 +975,15 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
 
       expect(container.textContent).toEqual('A');
 
-      await act(() => {
-        store.set({});
-      });
+      await expect(async () => {
+        await act(() => {
+          store.set({});
+        });
+      }).toWarnDev(
+        ReactFeatureFlags.enableUseRefAccessWarning
+          ? ['Warning: App: Unsafe read of a mutable value during render.']
+          : [],
+      );
       expect(container.textContent).toEqual('Malformed state');
     });
   });

diff --git a/scripts/jest/matchers/toWarnDev.js b/scripts/jest/matchers/toWarnDev.js
@@ -152,11 +152,7 @@ const createMatcherFor = (consoleMethod, matcherName) =>
       // Avoid using Jest's built-in spy since it can't be removed.
       console[consoleMethod] = consoleSpy;
 
-      try {
-        callback();
-      } catch (error) {
-        caughtError = error;
-      } finally {
+      const onFinally = () => {
         // Restore the unspied method so that unexpected errors fail tests.
         console[consoleMethod] = originalMethod;
 
@@ -259,12 +255,52 @@ const createMatcherFor = (consoleMethod, matcherName) =>
         }
 
         return {pass: true};
+      };
+
+      let returnPromise = null;
+      try {
+        const result = callback();
+
+        if (
+          typeof result === 'object' &&
+          result !== null &&
+          typeof result.then === 'function'
+        ) {
+          // `act` returns a thenable that can't be chained.
+          // Once `act(async () => {}).then(() => {}).then(() => {})` works
+          // we can just return `result.then(onFinally, error => ...)`
+          returnPromise = new Promise((resolve, reject) => {
+            result.then(
+              () => {
+                resolve(onFinally());
+              },
+              error => {
+                caughtError = error;
+                return resolve(onFinally());
+              }
+            );
+          });
+        }
+      } catch (error) {
+        caughtError = error;
+      } finally {
+        return returnPromise === null ? onFinally() : returnPromise;
       }
     } else {
       // Any uncaught errors or warnings should fail tests in production mode.
-      callback();
+      const result = callback();
 
-      return {pass: true};
+      if (
+        typeof result === 'object' &&
+        result !== null &&
+        typeof result.then === 'function'
+      ) {
+        return result.then(() => {
+          return {pass: true};
+        });
+      } else {
+        return {pass: true};
+      }
     }
   };