Repository: facebook/react
Pull request: Convert more Suspense tests to use `act`

Many of our Suspense-related tests were written before the `act` API was introduced, and use the lower level `waitFor` helpers instead. So they are less resilient to changes in implementation details than they could be.

This converts some of our test suite to use `act` in more places. I found these while working on a PR to expand our fallback throttling mechanism to include all renders that result from a promise resolving, even if there are no more fallbacks in the tree. This isn't all the affected tests, just some of them — I'll be sharding the changes across multiple PRs.

Changed files:
filename: packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js
patch: @@ -194,7 +194,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
   }
 
   // @gate enableLegacyCache
-  it('does not restart rendering for initial render', async () => {
+  it("does not restart if there's a ping during initial render", async () => {
     function Bar(props) {
       Scheduler.log('Bar');
       return props.children;
@@ -216,13 +216,9 @@ describe('ReactSuspenseWithNoopRenderer', () => {
       );
     }
 
-    if (gate(flags => flags.enableSyncDefaultUpdates)) {
-      React.startTransition(() => {
-        ReactNoop.render(<Foo />);
-      });
-    } else {
+    React.startTransition(() => {
       ReactNoop.render(<Foo />);
-    }
+    });
     await waitFor([
       'Foo',
       'Bar',
@@ -237,21 +233,21 @@ describe('ReactSuspenseWithNoopRenderer', () => {
     expect(ReactNoop).toMatchRenderedOutput(null);
 
     // Flush the promise completely
-    await resolveText('A');
-
-    // Even though the promise has resolved, we should now flush
-    // and commit the in progress render instead of restarting.
-    await waitForPaint(['D']);
-    expect(ReactNoop).toMatchRenderedOutput(
-      <>
-        <span prop="Loading..." />
-        <span prop="C" />
-        <span prop="D" />
-      </>,
-    );
-
-    // Next, we'll flush the complete content.
-    await waitForAll(['Bar', 'A', 'B']);
+    await act(async () => {
+      await resolveText('A');
+      // Even though the promise has resolved, we should now flush
+      // and commit the in progress render instead of restarting.
+      await waitForPaint(['D']);
+      expect(ReactNoop).toMatchRenderedOutput(
+        <>
+          <span prop="Loading..." />
+          <span prop="C" />
+          <span prop="D" />
+        </>,
+      );
+      // Next, we'll flush the complete content.
+      await waitForAll(['Bar', 'A', 'B']);
+    });
 
     expect(ReactNoop).toMatchRenderedOutput(
       <>
@@ -343,9 +339,8 @@ describe('ReactSuspenseWithNoopRenderer', () => {
 
     // Resolve first Suspense's promise so that it switches switches back to the
     // normal view. The second Suspense should still show the placeholder.
-    await resolveText('A');
-
-    await waitForAll(['A']);
+    await act(() => resolveText('A'));
+    assertLog(['A']);
     expect(ReactNoop).toMatchRenderedOutput(
       <>
         <span prop="A" />
@@ -355,9 +350,8 @@ describe('ReactSuspenseWithNoopRenderer', () => {
 
     // Resolve the second Suspense's promise so that it switches back to the
     // normal view.
-    await resolveText('B');
-
-    await waitForAll(['B']);
+    await act(() => resolveText('B'));
+    assertLog(['B']);
     expect(ReactNoop).toMatchRenderedOutput(
       <>
         <span prop="A" />
@@ -515,20 +509,37 @@ describe('ReactSuspenseWithNoopRenderer', () => {
 
   // @gate enableLegacyCache
   it('can update at a higher priority while in a suspended state', async () => {
-    function App(props) {
+    let setHighPri;
+    function HighPri() {
+      const [text, setText] = React.useState('A');
+      setHighPri = setText;
+      return <Text text={text} />;
+    }
+
+    let setLowPri;
+    function LowPri() {
+      const [text, setText] = React.useState('1');
+      setLowPri = setText;
+      return <AsyncText text={text} />;
+    }
+
+    function App() {
       return (
-        <Suspense fallback={<Text text="Loading..." />}>
-          <Text text={props.highPri} />
-          <AsyncText text={props.lowPri} />
-        </Suspense>
+        <>
+          <HighPri />
+          <Suspense fallback={<Text text="Loading..." />}>
+            <LowPri />
+          </Suspense>
+        </>
       );
     }
 
     // Initial mount
-    ReactNoop.render(<App highPri="A" lowPri="1" />);
-    await waitForAll(['A', 'Suspend! [1]', 'Loading...']);
-    await resolveText('1');
-    await waitForAll(['A', '1']);
+    await act(() => ReactNoop.render(<App />));
+    assertLog(['A', 'Suspend! [1]', 'Loading...']);
+
+    await act(() => resolveText('1'));
+    assertLog(['1']);
     expect(ReactNoop).toMatchRenderedOutput(
       <>
         <span prop="A" />
@@ -537,33 +548,31 @@ describe('ReactSuspenseWithNoopRenderer', () => {
     );
 
     // Update the low-pri text
-    ReactNoop.render(<App highPri="A" lowPri="2" />);
-    await waitForAll([
-      'A',
-      // Suspends
-      'Suspend! [2]',
-      'Loading...',
-    ]);
+    await act(() => startTransition(() => setLowPri('2')));
+    // Suspends
+    assertLog(['Suspend! [2]', 'Loading...']);
 
     // While we're still waiting for the low-pri update to complete, update the
     // high-pri text at high priority.
     ReactNoop.flushSync(() => {
-      ReactNoop.render(<App highPri="B" lowPri="1" />);
+      setHighPri('B');
     });
-    assertLog(['B', '1']);
+    assertLog(['B']);
     expect(ReactNoop).toMatchRenderedOutput(
       <>
         <span prop="B" />
         <span prop="1" />
       </>,
     );
 
-    // Unblock the low-pri text and finish
-    await resolveText('2');
+    // Unblock the low-pri text and finish. Nothing in the UI changes because
+    // the update was overriden
+    await act(() => resolveText('2'));
+    assertLog(['2']);
     expect(ReactNoop).toMatchRenderedOutput(
       <>
         <span prop="B" />
-        <span prop="1" />
+        <span prop="2" />
       </>,
     );
   });
@@ -772,8 +781,8 @@ describe('ReactSuspenseWithNoopRenderer', () => {
     );
 
     // Finally, flush the inner promise. We should see the complete screen.
-    await resolveText('Inner content');
-    await waitForAll(['Inner content']);
+    await act(() => resolveText('Inner content'));
+    assertLog(['Inner content']);
     expect(ReactNoop).toMatchRenderedOutput(
       <>
         <span prop="Sync" />
@@ -784,7 +793,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
   });
 
   // @gate enableLegacyCache
-  it('renders an expiration boundary synchronously', async () => {
+  it('renders an Suspense boundary synchronously', async () => {
     spyOnDev(console, 'error');
     // Synchronously render a tree that suspends
     ReactNoop.flushSync(() =>
@@ -814,8 +823,8 @@ describe('ReactSuspenseWithNoopRenderer', () => {
     );
 
     // Once the promise resolves, we render the suspended view
-    await resolveText('Async');
-    await waitForAll(['Async']);
+    await act(() => resolveText('Async'));
+    assertLog(['Async']);
     expect(ReactNoop).toMatchRenderedOutput(
       <>
         <span prop="Async" />
@@ -922,10 +931,11 @@ describe('ReactSuspenseWithNoopRenderer', () => {
     await waitForAll(['Suspend! [A]', 'Loading...']);
     expect(ReactNoop).toMatchRenderedOutput(<span prop="Loading..." />);
 
-    await resolveText('A');
-    await resolveText('B');
-
-    await waitForAll(['A', 'B']);
+    await act(() => {
+      resolveText('A');
+      resolveText('B');
+    });
+    assertLog(['A', 'B']);
     expect(ReactNoop).toMatchRenderedOutput(
       <>
         <span prop="A" />
@@ -1736,12 +1746,12 @@ describe('ReactSuspenseWithNoopRenderer', () => {
     ]);
     expect(ReactNoop).toMatchRenderedOutput(<span prop="Loading..." />);
 
-    await resolveText('A');
     // Wait a long time.
     Scheduler.unstable_advanceTime(5000);
     await advanceTimers(5000);
 
     // Retry with the new content.
+    await resolveText('A');
     await waitForAll([
       'A',
       // B suspends
@@ -1759,9 +1769,9 @@ describe('ReactSuspenseWithNoopRenderer', () => {
     );
 
     // Flush the last promise completely
-    await resolveText('B');
+    await act(() => resolveText('B'));
     // Renders successfully
-    await waitForAll(['B']);
+    assertLog(['B']);
     expect(ReactNoop).toMatchRenderedOutput(
       <>
         <span prop="A" />
@@ -1794,26 +1804,44 @@ describe('ReactSuspenseWithNoopRenderer', () => {
     ]);
     expect(ReactNoop).toMatchRenderedOutput(<span prop="Loading..." />);
 
-    await resolveText('A');
+    await act(async () => {
+      await resolveText('A');
 
-    // Retry with the new content.
-    await waitForAll([
-      'A',
-      // B suspends
-      'Suspend! [B]',
-      'Loading more...',
-    ]);
-    // Because we've already been waiting for so long we can
-    // wait a bit longer. Still nothing...
-    expect(ReactNoop).toMatchRenderedOutput(<span prop="Loading..." />);
+      // Retry with the new content.
+      await waitForAll([
+        'A',
+        // B suspends
+        'Suspend! [B]',
+        'Loading more...',
+      ]);
+      // Because we've already been waiting for so long we can
+      // wait a bit longer. Still nothing...
+      expect(ReactNoop).toMatchRenderedOutput(<span prop="Loading..." />);
 
-    await resolveText('B');
+      // Before we commit another Promise resolves.
+      // We're still showing the first loading state.
+      await resolveText('B');
+      expect(ReactNoop).toMatchRenderedOutput(<span prop="Loading..." />);
 
-    // Before we commit another Promise resolves.
-    // We're still showing the first loading state.
-    expect(ReactNoop).toMatchRenderedOutput(<span prop="Loading..." />);
-    // Restart and render the complete content.
-    await waitForAll(['A', 'B']);
+      // Restart and render the complete content.
+      await waitForAll(['A', 'B']);
+      // TODO: Because this render was the result of a retry, and a fallback
+      // was shown recently, we should suspend and remain on the fallback
+      // for little bit longer. We currently only do this if there's still
+      // remaining fallbacks in the tree, but we should do it for all retries.
+      //
+      // Correct output:
+      // expect(ReactNoop).toMatchRenderedOutput(<span prop="Loading..." />);
+      //
+      // Actual output:
+      expect(ReactNoop).toMatchRenderedOutput(
+        <>
+          <span prop="A" />
+          <span prop="B" />
+        </>,
+      );
+    });
+    assertLog([]);
     expect(ReactNoop).toMatchRenderedOutput(
       <>
         <span prop="A" />
@@ -2011,8 +2039,8 @@ describe('ReactSuspenseWithNoopRenderer', () => {
     expect(ReactNoop).toMatchRenderedOutput(<span prop="Initial load..." />);
 
     // Eventually we resolve and show the data.
-    await resolveText('A');
-    await waitForAll(['A', 'B']);
+    await act(() => resolveText('A'));
+    assertLog(['A', 'B']);
     expect(ReactNoop).toMatchRenderedOutput(
       <>
         <span prop="A" />
@@ -2037,8 +2065,8 @@ describe('ReactSuspenseWithNoopRenderer', () => {
     );
 
     // Later we load the data.
-    await resolveText('C');
-    await waitForAll(['A', 'C']);
+    await act(() => resolveText('C'));
+    assertLog(['A', 'C']);
     expect(ReactNoop).toMatchRenderedOutput(
       <>
         <span prop="A" />
@@ -2073,8 +2101,8 @@ describe('ReactSuspenseWithNoopRenderer', () => {
     expect(ReactNoop).toMatchRenderedOutput(<span prop="B" />);
 
     // Eventually we resolve and show the data.
-    await resolveText('A');
-    await waitForAll(['A']);
+    await act(() => resolveText('A'));
+    assertLog(['A']);
     expect(ReactNoop).toMatchRenderedOutput(
       <>
         <span prop="A" />
@@ -2102,8 +2130,8 @@ describe('ReactSuspenseWithNoopRenderer', () => {
     );
 
     // Later we load the data.
-    await resolveText('C');
-    await waitForAll(['A', 'C']);
+    await act(() => resolveText('C'));
+    assertLog(['A', 'C']);
     expect(ReactNoop).toMatchRenderedOutput(
       <>
         <span prop="A" />
@@ -2266,8 +2294,8 @@ describe('ReactSuspenseWithNoopRenderer', () => {
       expect(ReactNoop).toMatchRenderedOutput(<span prop="Loading..." />);
 
       // Later we load the data.
-      await resolveText('A');
-      await waitForAll(['A']);
+      await act(() => resolveText('A'));
+      assertLog(['A']);
       expect(ReactNoop).toMatchRenderedOutput(<span prop="A" />);
 
       // Start transition.
@@ -2280,8 +2308,8 @@ describe('ReactSuspenseWithNoopRenderer', () => {
       // loading state.
       expect(ReactNoop).toMatchRenderedOutput(<span prop="A" />);
       // Later we load the data.
-      await resolveText('B');
-      await waitForAll(['B']);
+      await act(() => resolveText('B'));
+      assertLog(['B']);
       expect(ReactNoop).toMatchRenderedOutput(<span prop="B" />);
     });
 
@@ -2316,8 +2344,8 @@ describe('ReactSuspenseWithNoopRenderer', () => {
       });
 
       // Later we load the data.
-      await resolveText('A');
-      await waitForAll(['A']);
+      await act(() => resolveText('A'));
+      assertLog(['A']);
       expect(ReactNoop).toMatchRenderedOutput(<span prop="A" />);
 
       // Start transition.
@@ -2332,8 +2360,8 @@ describe('ReactSuspenseWithNoopRenderer', () => {
         expect(ReactNoop).toMatchRenderedOutput(<span prop="A" />);
       });
       // Later we load the data.
-      await resolveText('B');
-      await waitForAll(['B']);
+      await act(() => resolveText('B'));
+      assertLog(['B']);
       expect(ReactNoop).toMatchRenderedOutput(<span prop="B" />);
     });
 
@@ -2371,8 +2399,8 @@ describe('ReactSuspenseWithNoopRenderer', () => {
       });
 
       // Later we load the data.
-      await resolveText('A');
-      await waitForAll(['A']);
+      await act(() => resolveText('A'));
+      assertLog(['A']);
       expect(ReactNoop).toMatchRenderedOutput(<span prop="A" />);
 
       // Start transition.
@@ -2387,8 +2415,8 @@ describe('ReactSuspenseWithNoopRenderer', () => {
         expect(ReactNoop).toMatchRenderedOutput(<span prop="A" />);
       });
       // Later we load the data.
-      await resolveText('B');
-      await waitForAll(['B']);
+      await act(() => resolveText('B'));
+      assertLog(['B']);
       expect(ReactNoop).toMatchRenderedOutput(<span prop="B" />);
     });
   });
@@ -2414,8 +2442,8 @@ describe('ReactSuspenseWithNoopRenderer', () => {
       expect(ReactNoop).toMatchRenderedOutput(<span prop="Loading..." />);
 
       // Later we load the data.
-      await resolveText('A');
-      await waitForAll(['A']);
+      await act(() => resolveText('A'));
+      assertLog(['A']);
       expect(ReactNoop).toMatchRenderedOutput(<span prop="A" />);
 
       // Start transition.
@@ -2429,8 +2457,8 @@ describe('ReactSuspenseWithNoopRenderer', () => {
       expect(ReactNoop).toMatchRenderedOutput(<span prop="A" />);
 
       // Later we load the data.
-      await resolveText('B');
-      await waitForAll(['B']);
+      await act(() => resolveText('B'));
+      assertLog(['B']);
       expect(ReactNoop).toMatchRenderedOutput(<span prop="B" />);
 
       // Start a long (infinite) transition.
@@ -2475,8 +2503,8 @@ describe('ReactSuspenseWithNoopRenderer', () => {
       });
 
       // Later we load the data.
-      await resolveText('A');
-      await waitForAll(['A']);
+      await act(() => resolveText('A'));
+      assertLog(['A']);
       expect(ReactNoop).toMatchRenderedOutput(<span prop="A" />);
 
       // Start transition.
@@ -2493,8 +2521,8 @@ describe('ReactSuspenseWithNoopRenderer', () => {
       });
 
       // Later we load the data.
-      await resolveText('B');
-      await waitForAll(['B']);
+      await act(() => resolveText('B'));
+      assertLog(['B']);
       expect(ReactNoop).toMatchRenderedOutput(<span prop="B" />);
 
       // Start a long (infinite) transition.
@@ -2545,8 +2573,8 @@ describe('ReactSuspenseWithNoopRenderer', () => {
       });
 
       // Later we load the data.
-      await resolveText('A');
-      await waitForAll(['A']);
+      await act(() => resolveText('A'));
+      assertLog(['A']);
       expect(ReactNoop).toMatchRenderedOutput(<span prop="A" />);
 
       // Start transition.
@@ -2562,8 +2590,8 @@ describe('ReactSuspenseWithNoopRenderer', () => {
       });
 
       // Later we load the data.
-      await resolveText('B');
-      await waitForAll(['B']);
+      await act(() => resolveText('B'));
+      assertLog(['B']);
       expect(ReactNoop).toMatchRenderedOutput(<span prop="B" />);
 
       // Start a long (infinite) transition.
@@ -2599,8 +2627,8 @@ describe('ReactSuspenseWithNoopRenderer', () => {
     // Initial render.
     ReactNoop.render(<App page="A" />);
     await waitForAll(['Hi!', 'Suspend! [A]', 'Loading...']);
-    await resolveText('A');
-    await waitForAll(['Hi!', 'A']);
+    await act(() => resolveText('A'));
+    assertLog(['Hi!', 'A']);
     expect(ReactNoop).toMatchRenderedOutput(
       <>
         <span prop="Hi!" />