Don't prerender siblings of suspended component (#26380)

Today if something suspends, React will continue rendering the siblings
of that component.

Our original rationale for prerendering the siblings of a suspended
component was to initiate any lazy fetches that they might contain. This
was when we were more bullish about lazy fetching being a good idea some
of the time (when combined with prefetching), as opposed to our latest
thinking, which is that it's almost always a bad idea.

Another rationale for the original behavior was that the render was I/O
bound, anyway, so we might as do some extra work in the meantime. But
this was before we had the concept of instant loading states: when
navigating to a new screen, it's better to show a loading state as soon
as you can (often a skeleton UI), rather than delay the transition.
(There are still cases where we block the render, when a suitable
loading state is not available; it's just not _all_ cases where
something suspends.) So the biggest issue with our existing
implementation is that the prerendering of the siblings happens within
the same render pass as the one that suspended — _before_ the loading
state appears.

What we should do instead is immediately unwind the stack as soon as
something suspends, to unblock the loading state.

If we want to preserve the ability to prerender the siblings, what we
could do is schedule special render pass immediately after the fallback
is displayed. This is likely what we'll do in the future. However, in
the new implementation of `use`, there's another reason we don't
prerender siblings: so we can preserve the state of the stack when
something suspends, and resume where we left of when the promise
resolves without replaying the parents. The only way to do this
currently is to suspend the entire work loop. Fiber does not currently
support rendering multiple siblings in "parallel". Once you move onto
the next sibling, the stack of the previous sibling is discarded and
cannot be restored. We do plan to implement this feature, but it will
require a not-insignificant refactor.

Given that lazy data fetching is already bad for performance, the best
trade off for now seems to be to disable prerendering of siblings. This
gives us the best performance characteristics when you're following best
practices (i.e. hoist data fetches to Server Components or route
loaders), at the expense of making an already bad pattern a bit worse.

Later, when we implement resumable context stacks, we can reenable
sibling prerendering. Though even then the use case will mostly be to
prerender the CPU-bound work, not lazy fetches.

Author: acdlite

packages/internal-test-utils/__tests__/ReactInternalTestUtils-test.js

@@ -99,13 +99,9 @@ describe('ReactInternalTestUtils', () => {
     assertLog([
       'A',
       'B',
-      'C',
-      'D',
       // React will try one more time before giving up.
       'A',
       'B',
-      'C',
-      'D',
     ]);
   });
 

packages/react-cache/src/__tests__/ReactCacheOld-test.internal.js

@@ -208,19 +208,19 @@ describe('ReactCache', () => {
         unstable_isConcurrent: true,
       },
     );
-    await waitForAll([
-      'Suspend! [1]',
-      'Suspend! [2]',
-      'Suspend! [3]',
-      'Loading...',
-    ]);
+    await waitForAll(['Suspend! [1]', 'Loading...']);
     jest.advanceTimersByTime(100);
-    assertLog([
-      'Promise resolved [1]',
-      'Promise resolved [2]',
-      'Promise resolved [3]',
-    ]);
+    assertLog(['Promise resolved [1]']);
+    await waitForAll([1, 'Suspend! [2]']);
+
+    jest.advanceTimersByTime(100);
+    assertLog(['Promise resolved [2]']);
+    await waitForAll([1, 2, 'Suspend! [3]']);
+
+    jest.advanceTimersByTime(100);
+    assertLog(['Promise resolved [3]']);
     await waitForAll([1, 2, 3]);
+
     expect(root).toMatchRenderedOutput('123');
 
     // Render 1, 4, 5
@@ -232,10 +232,16 @@ describe('ReactCache', () => {
       </Suspense>,
     );
 
-    await waitForAll([1, 'Suspend! [4]', 'Suspend! [5]', 'Loading...']);
+    await waitForAll([1, 'Suspend! [4]', 'Loading...']);
+
+    jest.advanceTimersByTime(100);
+    assertLog(['Promise resolved [4]']);
+    await waitForAll([1, 4, 'Suspend! [5]', 'Loading...']);
+
     jest.advanceTimersByTime(100);
-    assertLog(['Promise resolved [4]', 'Promise resolved [5]']);
+    assertLog(['Promise resolved [5]']);
     await waitForAll([1, 4, 5]);
+
     expect(root).toMatchRenderedOutput('145');
 
     // We've now rendered values 1, 2, 3, 4, 5, over our limit of 3. The least
@@ -254,11 +260,14 @@ describe('ReactCache', () => {
       1,
       // 2 and 3 suspend because they were evicted from the cache
       'Suspend! [2]',
-      'Suspend! [3]',
       'Loading...',
     ]);
     jest.advanceTimersByTime(100);
-    assertLog(['Promise resolved [2]', 'Promise resolved [3]']);
+    assertLog(['Promise resolved [2]']);
+    await waitForAll([1, 2, 'Suspend! [3]', 'Loading...']);
+
+    jest.advanceTimersByTime(100);
+    assertLog(['Promise resolved [3]']);
     await waitForAll([1, 2, 3]);
     expect(root).toMatchRenderedOutput('123');
   });