Revert yieldy behavior for non-use Suspense (#25537)

To derisk the rollout of `use`, and simplify the implementation, this
reverts the yield-to-microtasks behavior for promises that are thrown
directly (as opposed to being unwrapped by `use`).

We may add this back later. However, the plan is to deprecate throwing a
promise directly and migrate all existing Suspense code to `use`, so the
extra code probably isn't worth it.

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

@@ -136,7 +136,7 @@ import {now} from './Scheduler';
 import {
   trackUsedThenable,
   getPreviouslyUsedThenableAtIndex,
-} from './ReactFiberWakeable.new';
+} from './ReactFiberThenable.new';
 
 const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals;
 
@@ -783,6 +783,7 @@ function use<T>(usable: Usable<T>): T {
       const index = thenableIndexCounter;
       thenableIndexCounter += 1;
 
+      // TODO: Unify this switch statement with the one in trackUsedThenable.
       switch (thenable.status) {
         case 'fulfilled': {
           const fulfilledValue: T = thenable.value;

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

@@ -136,7 +136,7 @@ import {now} from './Scheduler';
 import {
   trackUsedThenable,
   getPreviouslyUsedThenableAtIndex,
-} from './ReactFiberWakeable.old';
+} from './ReactFiberThenable.old';
 
 const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals;
 
@@ -783,6 +783,7 @@ function use<T>(usable: Usable<T>): T {
       const index = thenableIndexCounter;
       thenableIndexCounter += 1;
 
+      // TODO: Unify this switch statement with the one in trackUsedThenable.
       switch (thenable.status) {
         case 'fulfilled': {
           const fulfilledValue: T = thenable.value;

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

@@ -8,7 +8,6 @@
  */
 
 import type {
-  Wakeable,
   Thenable,
   PendingThenable,
   FulfilledThenable,
@@ -18,14 +17,8 @@ import type {
 import ReactSharedInternals from 'shared/ReactSharedInternals';
 const {ReactCurrentActQueue} = ReactSharedInternals;
 
-let suspendedThenable: Thenable<mixed> | null = null;
-let adHocSuspendCount: number = 0;
-
-// TODO: Sparse arrays are bad for performance.
+let suspendedThenable: Thenable<any> | null = null;
 let usedThenables: Array<Thenable<any> | void> | null = null;
-let lastUsedThenable: Thenable<any> | null = null;
-
-const MAX_AD_HOC_SUSPEND_COUNT = 50;
 
 export function isTrackingSuspendedThenable(): boolean {
   return suspendedThenable !== null;
@@ -39,22 +32,17 @@ export function suspendedThenableDidResolve(): boolean {
   return false;
 }
 
-export function trackSuspendedWakeable(wakeable: Wakeable) {
-  // If this wakeable isn't already a thenable, turn it into one now. Then,
-  // when we resume the work loop, we can check if its status is
-  // still pending.
-  // TODO: Get rid of the Wakeable type? It's superseded by UntrackedThenable.
-  const thenable: Thenable<mixed> = (wakeable: any);
-
-  if (thenable !== lastUsedThenable) {
-    // If this wakeable was not just `use`-d, it must be an ad hoc wakeable
-    // that was thrown by an older Suspense implementation. Keep a count of
-    // these so that we can detect an infinite ping loop.
-    // TODO: Once `use` throws an opaque signal instead of the actual thenable,
-    // a better way to count ad hoc suspends is whether an actual thenable
-    // is caught by the work loop.
-    adHocSuspendCount++;
+export function trackUsedThenable<T>(thenable: Thenable<T>, index: number) {
+  if (__DEV__ && ReactCurrentActQueue.current !== null) {
+    ReactCurrentActQueue.didUsePromise = true;
   }
+
+  if (usedThenables === null) {
+    usedThenables = [thenable];
+  } else {
+    usedThenables[index] = thenable;
+  }
+
   suspendedThenable = thenable;
 
   // We use an expando to track the status and result of a thenable so that we
@@ -105,34 +93,12 @@ export function trackSuspendedWakeable(wakeable: Wakeable) {
 
 export function resetWakeableStateAfterEachAttempt() {
   suspendedThenable = null;
-  adHocSuspendCount = 0;
-  lastUsedThenable = null;
 }
 
 export function resetThenableStateOnCompletion() {
   usedThenables = null;
 }
 
-export function throwIfInfinitePingLoopDetected() {
-  if (adHocSuspendCount > MAX_AD_HOC_SUSPEND_COUNT) {
-    // TODO: Guard against an infinite loop by throwing an error if the same
-    // component suspends too many times in a row. This should be thrown from
-    // the render phase so that it gets the component stack.
-  }
-}
-
-export function trackUsedThenable<T>(thenable: Thenable<T>, index: number) {
-  if (usedThenables === null) {
-    usedThenables = [];
-  }
-  usedThenables[index] = thenable;
-  lastUsedThenable = thenable;
-
-  if (__DEV__ && ReactCurrentActQueue.current !== null) {
-    ReactCurrentActQueue.didUsePromise = true;
-  }
-}
-
 export function getPreviouslyUsedThenableAtIndex<T>(
   index: number,
 ): Thenable<T> | null {

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

@@ -8,7 +8,6 @@
  */
 
 import type {
-  Wakeable,
   Thenable,
   PendingThenable,
   FulfilledThenable,
@@ -18,14 +17,8 @@ import type {
 import ReactSharedInternals from 'shared/ReactSharedInternals';
 const {ReactCurrentActQueue} = ReactSharedInternals;
 
-let suspendedThenable: Thenable<mixed> | null = null;
-let adHocSuspendCount: number = 0;
-
-// TODO: Sparse arrays are bad for performance.
+let suspendedThenable: Thenable<any> | null = null;
 let usedThenables: Array<Thenable<any> | void> | null = null;
-let lastUsedThenable: Thenable<any> | null = null;
-
-const MAX_AD_HOC_SUSPEND_COUNT = 50;
 
 export function isTrackingSuspendedThenable(): boolean {
   return suspendedThenable !== null;
@@ -39,22 +32,17 @@ export function suspendedThenableDidResolve(): boolean {
   return false;
 }
 
-export function trackSuspendedWakeable(wakeable: Wakeable) {
-  // If this wakeable isn't already a thenable, turn it into one now. Then,
-  // when we resume the work loop, we can check if its status is
-  // still pending.
-  // TODO: Get rid of the Wakeable type? It's superseded by UntrackedThenable.
-  const thenable: Thenable<mixed> = (wakeable: any);
-
-  if (thenable !== lastUsedThenable) {
-    // If this wakeable was not just `use`-d, it must be an ad hoc wakeable
-    // that was thrown by an older Suspense implementation. Keep a count of
-    // these so that we can detect an infinite ping loop.
-    // TODO: Once `use` throws an opaque signal instead of the actual thenable,
-    // a better way to count ad hoc suspends is whether an actual thenable
-    // is caught by the work loop.
-    adHocSuspendCount++;
+export function trackUsedThenable<T>(thenable: Thenable<T>, index: number) {
+  if (__DEV__ && ReactCurrentActQueue.current !== null) {
+    ReactCurrentActQueue.didUsePromise = true;
   }
+
+  if (usedThenables === null) {
+    usedThenables = [thenable];
+  } else {
+    usedThenables[index] = thenable;
+  }
+
   suspendedThenable = thenable;
 
   // We use an expando to track the status and result of a thenable so that we
@@ -105,34 +93,12 @@ export function trackSuspendedWakeable(wakeable: Wakeable) {
 
 export function resetWakeableStateAfterEachAttempt() {
   suspendedThenable = null;
-  adHocSuspendCount = 0;
-  lastUsedThenable = null;
 }
 
 export function resetThenableStateOnCompletion() {
   usedThenables = null;
 }
 
-export function throwIfInfinitePingLoopDetected() {
-  if (adHocSuspendCount > MAX_AD_HOC_SUSPEND_COUNT) {
-    // TODO: Guard against an infinite loop by throwing an error if the same
-    // component suspends too many times in a row. This should be thrown from
-    // the render phase so that it gets the component stack.
-  }
-}
-
-export function trackUsedThenable<T>(thenable: Thenable<T>, index: number) {
-  if (usedThenables === null) {
-    usedThenables = [];
-  }
-  usedThenables[index] = thenable;
-  lastUsedThenable = thenable;
-
-  if (__DEV__ && ReactCurrentActQueue.current !== null) {
-    ReactCurrentActQueue.didUsePromise = true;
-  }
-}
-
 export function getPreviouslyUsedThenableAtIndex<T>(
   index: number,
 ): Thenable<T> | null {

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

@@ -267,10 +267,9 @@ import {processTransitionCallbacks} from './ReactFiberTracingMarkerComponent.new
 import {
   resetWakeableStateAfterEachAttempt,
   resetThenableStateOnCompletion,
-  trackSuspendedWakeable,
   suspendedThenableDidResolve,
   isTrackingSuspendedThenable,
-} from './ReactFiberWakeable.new';
+} from './ReactFiberThenable.new';
 import {schedulePostPaintCallback} from './ReactPostPaintCallback';
 
 const ceil = Math.ceil;
@@ -1739,11 +1738,6 @@ function handleThrow(root, thrownValue): void {
     return;
   }
 
-  const isWakeable =
-    thrownValue !== null &&
-    typeof thrownValue === 'object' &&
-    typeof thrownValue.then === 'function';
-
   if (enableProfilerTimer && erroredWork.mode & ProfileMode) {
     // Record the time spent rendering before an error was thrown. This
     // avoids inaccurate Profiler durations in the case of a
@@ -1753,7 +1747,11 @@ function handleThrow(root, thrownValue): void {
 
   if (enableSchedulingProfiler) {
     markComponentRenderStopped();
-    if (isWakeable) {
+    if (
+      thrownValue !== null &&
+      typeof thrownValue === 'object' &&
+      typeof thrownValue.then === 'function'
+    ) {
       const wakeable: Wakeable = (thrownValue: any);
       markComponentSuspended(
         erroredWork,
@@ -1768,12 +1766,6 @@ function handleThrow(root, thrownValue): void {
       );
     }
   }
-
-  if (isWakeable) {
-    const wakeable: Wakeable = (thrownValue: any);
-
-    trackSuspendedWakeable(wakeable);
-  }
 }
 
 function pushDispatcher(container) {

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

@@ -267,10 +267,9 @@ import {processTransitionCallbacks} from './ReactFiberTracingMarkerComponent.old
 import {
   resetWakeableStateAfterEachAttempt,
   resetThenableStateOnCompletion,
-  trackSuspendedWakeable,
   suspendedThenableDidResolve,
   isTrackingSuspendedThenable,
-} from './ReactFiberWakeable.old';
+} from './ReactFiberThenable.old';
 import {schedulePostPaintCallback} from './ReactPostPaintCallback';
 
 const ceil = Math.ceil;
@@ -1739,11 +1738,6 @@ function handleThrow(root, thrownValue): void {
     return;
   }
 
-  const isWakeable =
-    thrownValue !== null &&
-    typeof thrownValue === 'object' &&
-    typeof thrownValue.then === 'function';
-
   if (enableProfilerTimer && erroredWork.mode & ProfileMode) {
     // Record the time spent rendering before an error was thrown. This
     // avoids inaccurate Profiler durations in the case of a
@@ -1753,7 +1747,11 @@ function handleThrow(root, thrownValue): void {
 
   if (enableSchedulingProfiler) {
     markComponentRenderStopped();
-    if (isWakeable) {
+    if (
+      thrownValue !== null &&
+      typeof thrownValue === 'object' &&
+      typeof thrownValue.then === 'function'
+    ) {
       const wakeable: Wakeable = (thrownValue: any);
       markComponentSuspended(
         erroredWork,
@@ -1768,12 +1766,6 @@ function handleThrow(root, thrownValue): void {
       );
     }
   }
-
-  if (isWakeable) {
-    const wakeable: Wakeable = (thrownValue: any);
-
-    trackSuspendedWakeable(wakeable);
-  }
 }
 
 function pushDispatcher(container) {

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

@@ -485,32 +485,22 @@ describe('ReactOffscreen', () => {
       // In the same render, also hide the offscreen tree.
       root.render(<App show={false} />);
 
-      if (gate(flags => flags.enableSyncDefaultUpdates)) {
-        expect(Scheduler).toFlushUntilNextPaint([
-          // The outer update will commit, but the inner update is deferred until
-          // a later render.
-          'Outer: 1',
+      expect(Scheduler).toFlushUntilNextPaint([
+        // The outer update will commit, but the inner update is deferred until
+        // a later render.
+        'Outer: 1',
 
-          // Something suspended. This means we won't commit immediately; there
-          // will be an async gap between render and commit. In this test, we will
-          // use this property to schedule a concurrent update. The fact that
-          // we're using Suspense to schedule a concurrent update is not directly
-          // relevant to the test — we could also use time slicing, but I've
-          // chosen to use Suspense the because implementation details of time
-          // slicing are more volatile.
-          'Suspend! [Async: 1]',
+        // Something suspended. This means we won't commit immediately; there
+        // will be an async gap between render and commit. In this test, we will
+        // use this property to schedule a concurrent update. The fact that
+        // we're using Suspense to schedule a concurrent update is not directly
+        // relevant to the test — we could also use time slicing, but I've
+        // chosen to use Suspense the because implementation details of time
+        // slicing are more volatile.
+        'Suspend! [Async: 1]',
 
-          'Loading...',
-        ]);
-      } else {
-        // When default updates are time sliced, React yields before preparing
-        // the fallback.
-        expect(Scheduler).toFlushUntilNextPaint([
-          'Outer: 1',
-          'Suspend! [Async: 1]',
-        ]);
-        expect(Scheduler).toFlushUntilNextPaint(['Loading...']);
-      }
+        'Loading...',
+      ]);
 
       // Assert that we haven't committed quite yet
       expect(root).toMatchRenderedOutput(

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

@@ -3874,6 +3874,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
         'Suspend! [A2]',
         'Loading...',
         'Suspend! [B2]',
+        'Loading...',
       ]);
       expect(root).toMatchRenderedOutput(
         <>

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

@@ -26,6 +26,11 @@ describe('ReactWakeable', () => {
     return props.text;
   }
 
+  // This behavior was intentionally disabled to derisk the rollout of `use`.
+  // It changes the behavior of old, pre-`use` Suspense implementations. We may
+  // add this back; however, the plan is to migrate all existing Suspense code
+  // to `use`, so the extra code probably isn't worth it.
+  // @gate TODO
   test('if suspended fiber is pinged in a microtask, retry immediately without unwinding the stack', async () => {
     let resolved = false;
     function Async() {

packages/react-server/src/ReactFizzHooks.js

@@ -21,7 +21,7 @@ import type {
 
 import type {ResponseState} from './ReactServerFormatConfig';
 import type {Task} from './ReactFizzServer';
-import type {ThenableState} from './ReactFizzWakeable';
+import type {ThenableState} from './ReactFizzThenable';
 
 import {readContext as readContextImpl} from './ReactFizzNewContext';
 import {getTreeId} from './ReactFizzTreeContext';
@@ -29,7 +29,7 @@ import {
   getPreviouslyUsedThenableAtIndex,
   createThenableState,
   trackUsedThenable,
-} from './ReactFizzWakeable';
+} from './ReactFizzThenable';
 
 import {makeId} from './ReactServerFormatConfig';
 
@@ -593,6 +593,7 @@ function use<T>(usable: Usable<T>): T {
       const index = thenableIndexCounter;
       thenableIndexCounter += 1;
 
+      // TODO: Unify this switch statement with the one in trackUsedThenable.
       switch (thenable.status) {
         case 'fulfilled': {
           const fulfilledValue: T = thenable.value;

packages/react-server/src/ReactFizzServer.js

@@ -17,7 +17,6 @@ import type {
   ReactContext,
   ReactProviderType,
   OffscreenMode,
-  Wakeable,
 } from 'shared/ReactTypes';
 import type {LazyComponent as LazyComponentType} from 'react/src/ReactLazy';
 import type {
@@ -30,7 +29,7 @@ import type {
 import type {ContextSnapshot} from './ReactFizzNewContext';
 import type {ComponentStackNode} from './ReactFizzComponentStack';
 import type {TreeContext} from './ReactFizzTreeContext';
-import type {ThenableState} from './ReactFizzWakeable';
+import type {ThenableState} from './ReactFizzThenable';
 
 import {
   scheduleWork,
@@ -139,7 +138,6 @@ import {
 import assign from 'shared/assign';
 import getComponentNameFromType from 'shared/getComponentNameFromType';
 import isArray from 'shared/isArray';
-import {trackSuspendedWakeable} from './ReactFizzWakeable';
 
 const ReactCurrentDispatcher = ReactSharedInternals.ReactCurrentDispatcher;
 const ReactCurrentCache = ReactSharedInternals.ReactCurrentCache;
@@ -1554,8 +1552,6 @@ function spawnNewSuspendedTask(
     task.treeContext,
   );
 
-  trackSuspendedWakeable(x);
-
   if (__DEV__) {
     if (task.componentStack !== null) {
       // We pop one task off the stack because the node that suspended will be tried again,
@@ -1879,9 +1875,6 @@ function retryTask(request: Request, task: Task): void {
       // Something suspended again, let's pick it back up later.
       const ping = task.ping;
       x.then(ping, ping);
-
-      const wakeable: Wakeable = x;
-      trackSuspendedWakeable(wakeable);
       task.thenableState = getThenableStateAfterSuspending();
     } else {
       task.abortSet.delete(task);

packages/react-server/src/ReactFizzThenable.js

@@ -14,7 +14,6 @@
 // instead of "Wakeable". Or some other more appropriate name.
 
 import type {
-  Wakeable,
   Thenable,
   PendingThenable,
   FulfilledThenable,
@@ -30,12 +29,12 @@ export function createThenableState(): ThenableState {
   return [];
 }
 
-export function trackSuspendedWakeable(wakeable: Wakeable) {
-  // If this wakeable isn't already a thenable, turn it into one now. Then,
-  // when we resume the work loop, we can check if its status is
-  // still pending.
-  // TODO: Get rid of the Wakeable type? It's superseded by UntrackedThenable.
-  const thenable: Thenable<mixed> = (wakeable: any);
+export function trackUsedThenable<T>(
+  thenableState: ThenableState,
+  thenable: Thenable<T>,
+  index: number,
+) {
+  thenableState[index] = thenable;
 
   // We use an expando to track the status and result of a thenable so that we
   // can synchronously unwrap the value. Think of this as an extension of the
@@ -82,16 +81,6 @@ export function trackSuspendedWakeable(wakeable: Wakeable) {
   }
 }
 
-export function trackUsedThenable<T>(
-  thenableState: ThenableState,
-  thenable: Thenable<T>,
-  index: number,
-) {
-  // This is only a separate function from trackSuspendedWakeable for symmetry
-  // with Fiber.
-  thenableState[index] = thenable;
-}
-
 export function getPreviouslyUsedThenableAtIndex<T>(
   thenableState: ThenableState | null,
   index: number,

packages/react-server/src/ReactFlightHooks.js

@@ -10,7 +10,7 @@
 import type {Dispatcher} from 'react-reconciler/src/ReactInternalTypes';
 import type {Request} from './ReactFlightServer';
 import type {ReactServerContext, Thenable, Usable} from 'shared/ReactTypes';
-import type {ThenableState} from './ReactFlightWakeable';
+import type {ThenableState} from './ReactFlightThenable';
 import {
   REACT_SERVER_CONTEXT_TYPE,
   REACT_MEMO_CACHE_SENTINEL,
@@ -21,7 +21,7 @@ import {
   getPreviouslyUsedThenableAtIndex,
   createThenableState,
   trackUsedThenable,
-} from './ReactFlightWakeable';
+} from './ReactFlightThenable';
 
 let currentRequest = null;
 let thenableIndexCounter = 0;

packages/react-server/src/ReactFlightServer.js

@@ -16,7 +16,7 @@ import type {
   ModuleKey,
 } from './ReactFlightServerConfig';
 import type {ContextSnapshot} from './ReactFlightNewContext';
-import type {ThenableState} from './ReactFlightWakeable';
+import type {ThenableState} from './ReactFlightThenable';
 import type {
   ReactProviderType,
   ServerContextJSONValue,
@@ -64,7 +64,7 @@ import {
   getActiveContext,
   rootContextSnapshot,
 } from './ReactFlightNewContext';
-import {trackSuspendedWakeable} from './ReactFlightWakeable';
+import {trackSuspendedWakeable} from './ReactFlightThenable';
 
 import {
   REACT_ELEMENT_TYPE,

packages/react-server/src/ReactFlightThenable.js

@@ -30,6 +30,8 @@ export function createThenableState(): ThenableState {
   return [];
 }
 
+// TODO: Unify this with trackSuspendedThenable. It needs to support not only
+// `use`, but async components, too.
 export function trackSuspendedWakeable(wakeable: Wakeable) {
   // If this wakeable isn't already a thenable, turn it into one now. Then,
   // when we resume the work loop, we can check if its status is

scripts/jest/TestFlags.js

@@ -48,6 +48,7 @@ const environmentFlags = {
 
   // Use this for tests that are known to be broken.
   FIXME: false,
+  TODO: false,
 
   // Turn these flags back on (or delete) once the effect list is removed in
   // favor of a depth-first traversal using `subtreeTags`.