Allow suspending in shell for all non-sync updates

Instead of erroring, we can delay the commit.

The only time we'll continue to error when there's no parent Suspense
boundary is during sync/discrete updates, because those are expected to
produce a complete tree synchronously to maintain consistency with
external state.

Author: acdlite

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

@@ -155,7 +155,7 @@ describe('useId', () => {
     textCache = new Map();
   }
 
-  test('suspending in the shell', async () => {
+  test('suspending in the shell during hydration', async () => {
     const div = React.createRef(null);
 
     function App() {
@@ -194,4 +194,23 @@ describe('useId', () => {
     expect(div.current).toBe(dehydratedDiv);
     expect(container.textContent).toBe('Shell');
   });
+
+  test('suspending in the shell during a normal client render', async () => {
+    // Same as previous test but during a normal client render, no hydration
+    function App() {
+      return <AsyncText text="Shell" />;
+    }
+
+    const root = ReactDOM.createRoot(container);
+    await clientAct(async () => {
+      root.render(<App />);
+    });
+    expect(Scheduler).toHaveYielded(['Suspend! [Shell]']);
+
+    await clientAct(async () => {
+      await resolveText('Shell');
+    });
+    expect(Scheduler).toHaveYielded(['Shell']);
+    expect(container.textContent).toBe('Shell');
+  });
 });

packages/react-reconciler/src/ReactFiberThrow.new.js

@@ -79,7 +79,6 @@ import {
   includesSomeLane,
   mergeLanes,
   pickArbitraryLane,
-  includesOnlyTransitions,
   includesSyncLane,
 } from './ReactFiberLane.new';
 import {
@@ -481,33 +480,25 @@ function throwException(
       attachRetryListener(suspenseBoundary, root, wakeable, rootRenderLanes);
       return;
     } else {
-      // No boundary was found. If we're inside startTransition, this is OK.
+      // No boundary was found. Unless this is a sync update, this is OK.
       // We can suspend and wait for more data to arrive.
 
-      if (
-        includesOnlyTransitions(rootRenderLanes) ||
-        (getIsHydrating() && !includesSyncLane(rootRenderLanes))
-      ) {
-        // This is a transition. Suspend. Since we're not activating a Suspense
-        // boundary, this will unwind all the way to the root without performing
-        // a second pass to render a fallback. (This is arguably how refresh
-        // transitions should work, too, since we're not going to commit the
-        // fallbacks anyway.)
+      if (!includesSyncLane(rootRenderLanes)) {
+        // This is not a sync update. Suspend. Since we're not activating a
+        // Suspense boundary, this will unwind all the way to the root without
+        // performing a second pass to render a fallback. (This is arguably how
+        // refresh transitions should work, too, since we're not going to commit
+        // the fallbacks anyway.)
         //
         // This case also applies to initial hydration.
-        //
-        // TODO: Maybe we should expand this branch to cover all non-sync
-        // updates, including default.
         attachPingListener(root, wakeable, rootRenderLanes);
         renderDidSuspendDelayIfPossible();
         return;
       }
 
-      // We're not in a transition. We treat this case like an error because
-      // discrete renders are expected to finish synchronously to maintain
-      // consistency with external state.
-      // TODO: This will error during non-transition concurrent renders, too.
-      // But maybe it shouldn't?
+      // This is a sync/discrete update. We treat this case like an error
+      // because discrete renders are expected to produce a complete tree
+      // synchronously to maintain consistency with external state.
 
       // TODO: We should never call getComponentNameFromFiber in production.
       // Log a warning or something to prevent us from accidentally bundling it.

packages/react-reconciler/src/ReactFiberThrow.old.js

@@ -79,7 +79,6 @@ import {
   includesSomeLane,
   mergeLanes,
   pickArbitraryLane,
-  includesOnlyTransitions,
   includesSyncLane,
 } from './ReactFiberLane.old';
 import {
@@ -481,33 +480,25 @@ function throwException(
       attachRetryListener(suspenseBoundary, root, wakeable, rootRenderLanes);
       return;
     } else {
-      // No boundary was found. If we're inside startTransition, this is OK.
+      // No boundary was found. Unless this is a sync update, this is OK.
       // We can suspend and wait for more data to arrive.
 
-      if (
-        includesOnlyTransitions(rootRenderLanes) ||
-        (getIsHydrating() && !includesSyncLane(rootRenderLanes))
-      ) {
-        // This is a transition. Suspend. Since we're not activating a Suspense
-        // boundary, this will unwind all the way to the root without performing
-        // a second pass to render a fallback. (This is arguably how refresh
-        // transitions should work, too, since we're not going to commit the
-        // fallbacks anyway.)
+      if (!includesSyncLane(rootRenderLanes)) {
+        // This is not a sync update. Suspend. Since we're not activating a
+        // Suspense boundary, this will unwind all the way to the root without
+        // performing a second pass to render a fallback. (This is arguably how
+        // refresh transitions should work, too, since we're not going to commit
+        // the fallbacks anyway.)
         //
         // This case also applies to initial hydration.
-        //
-        // TODO: Maybe we should expand this branch to cover all non-sync
-        // updates, including default.
         attachPingListener(root, wakeable, rootRenderLanes);
         renderDidSuspendDelayIfPossible();
         return;
       }
 
-      // We're not in a transition. We treat this case like an error because
-      // discrete renders are expected to finish synchronously to maintain
-      // consistency with external state.
-      // TODO: This will error during non-transition concurrent renders, too.
-      // But maybe it shouldn't?
+      // This is a sync/discrete update. We treat this case like an error
+      // because discrete renders are expected to produce a complete tree
+      // synchronously to maintain consistency with external state.
 
       // TODO: We should never call getComponentNameFromFiber in production.
       // Log a warning or something to prevent us from accidentally bundling it.

packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js

@@ -389,17 +389,6 @@ describe('ReactSuspense', () => {
     expect(root).toMatchRenderedOutput('Hi');
   });
 
-  it('throws if tree suspends and none of the Suspense ancestors have a boundary', () => {
-    ReactTestRenderer.create(<AsyncText text="Hi" ms={1000} />, {
-      unstable_isConcurrent: true,
-    });
-
-    expect(Scheduler).toFlushAndThrow(
-      'AsyncText suspended while rendering, but no fallback UI was specified.',
-    );
-    expect(Scheduler).toHaveYielded(['Suspend! [Hi]', 'Suspend! [Hi]']);
-  });
-
   it('updates memoized child of suspense component when context updates (simple memo)', () => {
     const {useContext, createContext, useState, memo} = React;
 

packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js

@@ -1003,9 +1003,15 @@ describe('ReactSuspenseWithNoopRenderer', () => {
   });
 
   // @gate enableCache
-  it('throws a helpful error when an update is suspends without a placeholder', () => {
-    ReactNoop.render(<AsyncText text="Async" />);
-    expect(Scheduler).toFlushAndThrow(
+  it('errors when an update suspends without a placeholder during a sync update', () => {
+    // This is an error because sync/discrete updates are expected to produce
+    // a complete tree immediately to maintain consistency with external state
+    // — we can't delay the commit.
+    expect(() => {
+      ReactNoop.flushSync(() => {
+        ReactNoop.render(<AsyncText text="Async" />);
+      });
+    }).toThrow(
       'AsyncText suspended while rendering, but no fallback UI was specified.',
     );
   });