Disable timeoutMs argument (#19703)

* Remove distinction between long, short transitions

We're removing the `timeoutMs` option, so there's no longer any
distinction between "short" and "long" transitions. They're all treated
the same.

This commit doesn't remove `timeoutMs` yet, only combines the internal
priority levels.

* Disable `timeoutMs` argument

tl;dr
-----

- We're removing the `timeoutMs` argument from `useTransition`.
- Transitions will either immediately switch to a skeleton/placeholder
  view (when loading new content) or wait indefinitely until the data
  resolves (when refreshing stale content).
- This commit disables the `timeoutMS` so that the API has the desired
  semantics. It doesn't yet update the types or migrate all the test
  callers. I'll do those steps in follow-up PRs.

Motivation
----------

Currently, transitions initiated by `startTransition` / `useTransition`
accept a `timeoutMs` option. You can use this to control the maximum
amount of time that a transition is allowed to delay before we give up
and show a placeholder.

What we've discovered is that, in practice, every transition falls into
one of two categories: a **load** or a **refresh**:

- **Loading a new screen**: show the next screen as soon as possible,
  even if the data hasn't finished loading. Use a skeleton/placeholder
  UI to show progress.
- **Refreshing a screen that's already visible**: keep showing the
  current screen indefinitely, for as long as it takes to load the fresh
  data, even if the current data is stale. Use a pending state (and
  maybe a busy indicator) to show progress.

In other words, transitions should either *delay indefinitely* (for a
refresh) or they should show a placeholder *instantly* (for a load).
There's not much use for transitions that are delayed for a
small-but-noticeable amount of time.

So, the plan is to remove the `timeoutMs` option. Instead, we'll assign
an effective timeout of `0` for loads, and `Infinity` for refreshes.

The mechanism for distinguishing a load from a refresh already exists in
the current model. If a component suspends, and the nearest Suspense
boundary hasn't already mounted, we treat that as a load, because
there's nothing on the screen. However, if the nearest boundary is
mounted, we treat that as a refresh, since it's already showing content.

If you need to fix a transition to be treated as a load instead of a
refresh, or vice versa, the solution will involve rearranging the
location of your Suspense boundaries. It may also involve adding a key.

We're still working on proper documentation for these patterns. In the
meantime, please reach out to us if you run into problems that you're
unsure how to fix.

We will remove `timeoutMs` from `useDeferredValue`, too, and apply the
same load versus refresh semantics to the update that spawns the
deferred value.

Note that there are other types of delays that are not related to
transitions; for example, we will still throttle the appearance of
nested placeholders (we refer to this as the placeholder "train model"),
and we may still apply a Just Noticeable Difference heuristic (JND) in
some cases. These aren't going anywhere. (Well, the JND heuristic might
but for different reasons than those discussed above.)

Author: acdlite

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

@@ -779,10 +779,19 @@ function runActTests(label, render, unmount, rerender) {
             },
             {timeout: 5000},
           );
-          // the spinner shows up regardless
-          expect(
-            document.querySelector('[data-test-id=spinner]'),
-          ).not.toBeNull();
+
+          if (label === 'concurrent mode') {
+            // In Concurrent Mode, refresh transitions delay indefinitely.
+            expect(document.querySelector('[data-test-id=spinner]')).toBeNull();
+          } else {
+            // In Legacy Mode and Blocking Mode, all fallbacks are forced to
+            // display, even during a refresh transition.
+            // TODO: Consider delaying indefinitely in Blocking Mode, to match
+            // Concurrent Mode semantics.
+            expect(
+              document.querySelector('[data-test-id=spinner]'),
+            ).not.toBeNull();
+          }
 
           // resolve the promise
           await act(async () => {

packages/react-reconciler/src/ReactFiberLane.js

@@ -43,23 +43,20 @@ import {
   NoPriority as NoSchedulerPriority,
 } from './SchedulerWithReactIntegration.new';
 
-export const SyncLanePriority: LanePriority = 17;
-export const SyncBatchedLanePriority: LanePriority = 16;
+export const SyncLanePriority: LanePriority = 15;
+export const SyncBatchedLanePriority: LanePriority = 14;
 
-const InputDiscreteHydrationLanePriority: LanePriority = 15;
-export const InputDiscreteLanePriority: LanePriority = 14;
+const InputDiscreteHydrationLanePriority: LanePriority = 13;
+export const InputDiscreteLanePriority: LanePriority = 12;
 
-const InputContinuousHydrationLanePriority: LanePriority = 13;
-export const InputContinuousLanePriority: LanePriority = 12;
+const InputContinuousHydrationLanePriority: LanePriority = 11;
+export const InputContinuousLanePriority: LanePriority = 10;
 
-const DefaultHydrationLanePriority: LanePriority = 11;
-export const DefaultLanePriority: LanePriority = 10;
+const DefaultHydrationLanePriority: LanePriority = 9;
+export const DefaultLanePriority: LanePriority = 8;
 
-const TransitionShortHydrationLanePriority: LanePriority = 9;
-export const TransitionShortLanePriority: LanePriority = 8;
-
-const TransitionLongHydrationLanePriority: LanePriority = 7;
-export const TransitionLongLanePriority: LanePriority = 6;
+const TransitionHydrationPriority: LanePriority = 7;
+export const TransitionPriority: LanePriority = 6;
 
 const RetryLanePriority: LanePriority = 5;
 
@@ -89,11 +86,8 @@ const InputContinuousLanes: Lanes = /*                  */ 0b0000000000000000000
 export const DefaultHydrationLane: Lane = /*            */ 0b0000000000000000000000100000000;
 export const DefaultLanes: Lanes = /*                   */ 0b0000000000000000000111000000000;
 
-const TransitionShortHydrationLane: Lane = /*           */ 0b0000000000000000001000000000000;
-const TransitionShortLanes: Lanes = /*                  */ 0b0000000000000011110000000000000;
-
-const TransitionLongHydrationLane: Lane = /*            */ 0b0000000000000100000000000000000;
-const TransitionLongLanes: Lanes = /*                   */ 0b0000000001111000000000000000000;
+const TransitionHydrationLane: Lane = /*                */ 0b0000000000000000001000000000000;
+const TransitionLanes: Lanes = /*                       */ 0b0000000001111111110000000000000;
 
 const RetryLanes: Lanes = /*                            */ 0b0000011110000000000000000000000;
 
@@ -160,23 +154,14 @@ function getHighestPriorityLanes(lanes: Lanes | Lane): Lanes {
     return_highestLanePriority = DefaultLanePriority;
     return defaultLanes;
   }
-  if ((lanes & TransitionShortHydrationLane) !== NoLanes) {
-    return_highestLanePriority = TransitionShortHydrationLanePriority;
-    return TransitionShortHydrationLane;
-  }
-  const transitionShortLanes = TransitionShortLanes & lanes;
-  if (transitionShortLanes !== NoLanes) {
-    return_highestLanePriority = TransitionShortLanePriority;
-    return transitionShortLanes;
+  if ((lanes & TransitionHydrationLane) !== NoLanes) {
+    return_highestLanePriority = TransitionHydrationPriority;
+    return TransitionHydrationLane;
   }
-  if ((lanes & TransitionLongHydrationLane) !== NoLanes) {
-    return_highestLanePriority = TransitionLongHydrationLanePriority;
-    return TransitionLongHydrationLane;
-  }
-  const transitionLongLanes = TransitionLongLanes & lanes;
-  if (transitionLongLanes !== NoLanes) {
-    return_highestLanePriority = TransitionLongLanePriority;
-    return transitionLongLanes;
+  const transitionLanes = TransitionLanes & lanes;
+  if (transitionLanes !== NoLanes) {
+    return_highestLanePriority = TransitionPriority;
+    return transitionLanes;
   }
   const retryLanes = RetryLanes & lanes;
   if (retryLanes !== NoLanes) {
@@ -241,10 +226,8 @@ export function lanePriorityToSchedulerPriority(
       return UserBlockingSchedulerPriority;
     case DefaultHydrationLanePriority:
     case DefaultLanePriority:
-    case TransitionShortHydrationLanePriority:
-    case TransitionShortLanePriority:
-    case TransitionLongHydrationLanePriority:
-    case TransitionLongLanePriority:
+    case TransitionHydrationPriority:
+    case TransitionPriority:
     case SelectiveHydrationLanePriority:
     case RetryLanePriority:
       return NormalSchedulerPriority;
@@ -402,7 +385,7 @@ function computeExpirationTime(lane: Lane, currentTime: number) {
   if (priority >= InputContinuousLanePriority) {
     // User interactions should expire slightly more quickly.
     return currentTime + 1000;
-  } else if (priority >= TransitionLongLanePriority) {
+  } else if (priority >= TransitionPriority) {
     return currentTime + 5000;
   } else {
     // Anything idle priority or lower should never expire.
@@ -513,9 +496,7 @@ export function findUpdateLane(
       if (lane === NoLane) {
         // If all the default lanes are already being worked on, look for a
         // lane in the transition range.
-        lane = pickArbitraryLane(
-          (TransitionShortLanes | TransitionLongLanes) & ~wipLanes,
-        );
+        lane = pickArbitraryLane(TransitionLanes & ~wipLanes);
         if (lane === NoLane) {
           // All the transition lanes are taken, too. This should be very
           // rare, but as a last resort, pick a default lane. This will have
@@ -525,8 +506,7 @@ export function findUpdateLane(
       }
       return lane;
     }
-    case TransitionShortLanePriority: // Should be handled by findTransitionLane instead
-    case TransitionLongLanePriority:
+    case TransitionPriority: // Should be handled by findTransitionLane instead
     case RetryLanePriority: // Should be handled by findRetryLane instead
       break;
     case IdleLanePriority:
@@ -548,48 +528,21 @@ export function findUpdateLane(
 
 // To ensure consistency across multiple updates in the same event, this should
 // be pure function, so that it always returns the same lane for given inputs.
-export function findTransitionLane(
-  lanePriority: LanePriority,
-  wipLanes: Lanes,
-  pendingLanes: Lanes,
-): Lane {
-  if (lanePriority === TransitionShortLanePriority) {
-    // First look for lanes that are completely unclaimed, i.e. have no
-    // pending work.
-    let lane = pickArbitraryLane(TransitionShortLanes & ~pendingLanes);
-    if (lane === NoLane) {
-      // If all lanes have pending work, look for a lane that isn't currently
-      // being worked on.
-      lane = pickArbitraryLane(TransitionShortLanes & ~wipLanes);
-      if (lane === NoLane) {
-        // If everything is being worked on, pick any lane. This has the
-        // effect of interrupting the current work-in-progress.
-        lane = pickArbitraryLane(TransitionShortLanes);
-      }
-    }
-    return lane;
-  }
-  if (lanePriority === TransitionLongLanePriority) {
-    // First look for lanes that are completely unclaimed, i.e. have no
-    // pending work.
-    let lane = pickArbitraryLane(TransitionLongLanes & ~pendingLanes);
+export function findTransitionLane(wipLanes: Lanes, pendingLanes: Lanes): Lane {
+  // First look for lanes that are completely unclaimed, i.e. have no
+  // pending work.
+  let lane = pickArbitraryLane(TransitionLanes & ~pendingLanes);
+  if (lane === NoLane) {
+    // If all lanes have pending work, look for a lane that isn't currently
+    // being worked on.
+    lane = pickArbitraryLane(TransitionLanes & ~wipLanes);
     if (lane === NoLane) {
-      // If all lanes have pending work, look for a lane that isn't currently
-      // being worked on.
-      lane = pickArbitraryLane(TransitionLongLanes & ~wipLanes);
-      if (lane === NoLane) {
-        // If everything is being worked on, pick any lane. This has the
-        // effect of interrupting the current work-in-progress.
-        lane = pickArbitraryLane(TransitionLongLanes);
-      }
+      // If everything is being worked on, pick any lane. This has the
+      // effect of interrupting the current work-in-progress.
+      lane = pickArbitraryLane(TransitionLanes);
     }
-    return lane;
   }
-  invariant(
-    false,
-    'Invalid transition priority: %s. This is a bug in React.',
-    lanePriority,
-  );
+  return lane;
 }
 
 // To ensure consistency across multiple updates in the same event, this should
@@ -816,18 +769,14 @@ export function getBumpedLaneForHydration(
     case DefaultLanePriority:
       lane = DefaultHydrationLane;
       break;
-    case TransitionShortHydrationLanePriority:
-    case TransitionShortLanePriority:
-      lane = TransitionShortHydrationLane;
-      break;
-    case TransitionLongHydrationLanePriority:
-    case TransitionLongLanePriority:
-      lane = TransitionLongHydrationLane;
+    case TransitionHydrationPriority:
+    case TransitionPriority:
+      lane = TransitionHydrationLane;
       break;
     case RetryLanePriority:
       // Shouldn't be reachable under normal circumstances, so there's no
       // dedicated lane for retry priority. Use the one for long transitions.
-      lane = TransitionLongHydrationLane;
+      lane = TransitionHydrationLane;
       break;
     case SelectiveHydrationLanePriority:
       lane = SelectiveHydrationLane;

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

@@ -153,8 +153,6 @@ import {
   SyncLanePriority,
   SyncBatchedLanePriority,
   InputDiscreteLanePriority,
-  TransitionShortLanePriority,
-  TransitionLongLanePriority,
   DefaultLanePriority,
   NoLanes,
   NoLane,
@@ -457,24 +455,13 @@ export function requestUpdateLane(
     // Use the size of the timeout as a heuristic to prioritize shorter
     // transitions over longer ones.
     // TODO: This will coerce numbers larger than 31 bits to 0.
-    const timeoutMs = suspenseConfig.timeoutMs;
-    const transitionLanePriority =
-      timeoutMs === undefined || (timeoutMs | 0) < 10000
-        ? TransitionShortLanePriority
-        : TransitionLongLanePriority;
-
     if (currentEventPendingLanes !== NoLanes) {
       currentEventPendingLanes =
         mostRecentlyUpdatedRoot !== null
           ? mostRecentlyUpdatedRoot.pendingLanes
           : NoLanes;
     }
-
-    return findTransitionLane(
-      transitionLanePriority,
-      currentEventWipLanes,
-      currentEventPendingLanes,
-    );
+    return findTransitionLane(currentEventWipLanes, currentEventPendingLanes);
   }
 
   // TODO: Remove this dependency on the Scheduler priority.
@@ -936,60 +923,41 @@ function finishConcurrentRender(root, exitStatus, lanes) {
     case RootSuspendedWithDelay: {
       markRootSuspended(root, lanes);
 
-      if (
-        // do not delay if we're inside an act() scope
-        !shouldForceFlushFallbacksInDEV()
-      ) {
-        // We're suspended in a state that should be avoided. We'll try to
-        // avoid committing it for as long as the timeouts let us.
-        const nextLanes = getNextLanes(root, NoLanes);
-        if (nextLanes !== NoLanes) {
-          // There's additional work on this root.
-          break;
-        }
-        const suspendedLanes = root.suspendedLanes;
-        if (!isSubsetOfLanes(suspendedLanes, lanes)) {
-          // We should prefer to render the fallback of at the last
-          // suspended level. Ping the last suspended level to try
-          // rendering it again.
-          // FIXME: What if the suspended lanes are Idle? Should not restart.
-          const eventTime = requestEventTime();
-          markRootPinged(root, suspendedLanes, eventTime);
-          break;
-        }
+      if (workInProgressRootLatestSuspenseTimeout !== NoTimestamp) {
+        // This is a transition, so we should exit without committing a
+        // placeholder and without scheduling a timeout. Delay indefinitely
+        // until we receive more data.
+        // TODO: Check the lanes to see if it's a transition, instead of
+        // tracking the latest timeout.
+        break;
+      }
+
+      if (!shouldForceFlushFallbacksInDEV()) {
+        // This is not a transition, but we did trigger an avoided state.
+        // Schedule a placeholder to display after a short delay, using the Just
+        // Noticable Difference.
+        // TODO: Is the JND optimization worth the added complexity? If this is
+        // the only reason we track the event time, then probably not.
+        // Consider removing.
 
         const mostRecentEventTime = getMostRecentEventTime(root, lanes);
-        let msUntilTimeout;
-        if (workInProgressRootLatestSuspenseTimeout !== NoTimestamp) {
-          // We have processed a suspense config whose expiration time we
-          // can use as the timeout.
-          msUntilTimeout = workInProgressRootLatestSuspenseTimeout - now();
-        } else if (mostRecentEventTime === NoTimestamp) {
-          // This should never normally happen because only new updates
-          // cause delayed states, so we should have processed something.
-          // However, this could also happen in an offscreen tree.
-          msUntilTimeout = 0;
-        } else {
-          // If we didn't process a suspense config, compute a JND based on
-          // the amount of time elapsed since the most recent event time.
-          const eventTimeMs = mostRecentEventTime;
-          const timeElapsedMs = now() - eventTimeMs;
-          msUntilTimeout = jnd(timeElapsedMs) - timeElapsedMs;
-        }
+        const eventTimeMs = mostRecentEventTime;
+        const timeElapsedMs = now() - eventTimeMs;
+        const msUntilTimeout = jnd(timeElapsedMs) - timeElapsedMs;
 
         // Don't bother with a very short suspense time.
         if (msUntilTimeout > 10) {
-          // The render is suspended, it hasn't timed out, and there's no
-          // lower priority work to do. Instead of committing the fallback
-          // immediately, wait for more data to arrive.
+          // Instead of committing the fallback immediately, wait for more data
+          // to arrive.
           root.timeoutHandle = scheduleTimeout(
             commitRoot.bind(null, root),
             msUntilTimeout,
           );
           break;
         }
       }
-      // The work expired. Commit immediately.
+
+      // Commit the placeholder.
       commitRoot(root);
       break;
     }