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
 awaitact(async()=>{

packages/react-reconciler/src/ReactFiberLane.js

@@ -43,23 +43,20 @@ import {
 NoPriorityasNoSchedulerPriority,
 }from'./SchedulerWithReactIntegration.new';
 
-exportconstSyncLanePriority: LanePriority=17;
-exportconstSyncBatchedLanePriority: LanePriority=16;
+exportconstSyncLanePriority: LanePriority=15;
+exportconstSyncBatchedLanePriority: LanePriority=14;
 
-constInputDiscreteHydrationLanePriority: LanePriority=15;
-exportconstInputDiscreteLanePriority: LanePriority=14;
+constInputDiscreteHydrationLanePriority: LanePriority=13;
+exportconstInputDiscreteLanePriority: LanePriority=12;
 
-constInputContinuousHydrationLanePriority: LanePriority=13;
-exportconstInputContinuousLanePriority: LanePriority=12;
+constInputContinuousHydrationLanePriority: LanePriority=11;
+exportconstInputContinuousLanePriority: LanePriority=10;
 
-constDefaultHydrationLanePriority: LanePriority=11;
-exportconstDefaultLanePriority: LanePriority=10;
+constDefaultHydrationLanePriority: LanePriority=9;
+exportconstDefaultLanePriority: LanePriority=8;
 
-constTransitionShortHydrationLanePriority: LanePriority=9;
-exportconstTransitionShortLanePriority: LanePriority=8;
-
-constTransitionLongHydrationLanePriority: LanePriority=7;
-exportconstTransitionLongLanePriority: LanePriority=6;
+constTransitionHydrationPriority: LanePriority=7;
+exportconstTransitionPriority: LanePriority=6;
 
 constRetryLanePriority: LanePriority=5;
 
@@ -89,11 +86,8 @@ const InputContinuousLanes: Lanes = /* */ 0b0000000000000000000
 exportconstDefaultHydrationLane: Lane=/* */0b0000000000000000000000100000000;
 exportconstDefaultLanes: Lanes=/* */0b0000000000000000000111000000000;
 
-constTransitionShortHydrationLane: Lane=/* */0b0000000000000000001000000000000;
-constTransitionShortLanes: Lanes=/* */0b0000000000000011110000000000000;
-
-constTransitionLongHydrationLane: Lane=/* */0b0000000000000100000000000000000;
-constTransitionLongLanes: Lanes=/* */0b0000000001111000000000000000000;
+constTransitionHydrationLane: Lane=/* */0b0000000000000000001000000000000;
+constTransitionLanes: Lanes=/* */0b0000000001111111110000000000000;
 
 constRetryLanes: Lanes=/* */0b0000011110000000000000000000000;
 
@@ -160,23 +154,14 @@ function getHighestPriorityLanes(lanes: Lanes | Lane): Lanes {
  return_highestLanePriority =DefaultLanePriority;
 returndefaultLanes;
 }
-if((lanes&TransitionShortHydrationLane)!==NoLanes){
- return_highestLanePriority =TransitionShortHydrationLanePriority;
-returnTransitionShortHydrationLane;
-}
-consttransitionShortLanes=TransitionShortLanes&lanes;
-if(transitionShortLanes!==NoLanes){
- return_highestLanePriority =TransitionShortLanePriority;
-returntransitionShortLanes;
+if((lanes&TransitionHydrationLane)!==NoLanes){
+ return_highestLanePriority =TransitionHydrationPriority;
+returnTransitionHydrationLane;
 }
-if((lanes&TransitionLongHydrationLane)!==NoLanes){
- return_highestLanePriority =TransitionLongHydrationLanePriority;
-returnTransitionLongHydrationLane;
-}
-consttransitionLongLanes=TransitionLongLanes&lanes;
-if(transitionLongLanes!==NoLanes){
- return_highestLanePriority =TransitionLongLanePriority;
-returntransitionLongLanes;
+consttransitionLanes=TransitionLanes&lanes;
+if(transitionLanes!==NoLanes){
+ return_highestLanePriority =TransitionPriority;
+returntransitionLanes;
 }
 constretryLanes=RetryLanes&lanes;
 if(retryLanes!==NoLanes){
@@ -241,10 +226,8 @@ export function lanePriorityToSchedulerPriority(
 returnUserBlockingSchedulerPriority;
 caseDefaultHydrationLanePriority:
 caseDefaultLanePriority:
-caseTransitionShortHydrationLanePriority:
-caseTransitionShortLanePriority:
-caseTransitionLongHydrationLanePriority:
-caseTransitionLongLanePriority:
+caseTransitionHydrationPriority:
+caseTransitionPriority:
 caseSelectiveHydrationLanePriority:
 caseRetryLanePriority:
 returnNormalSchedulerPriority;
@@ -402,7 +385,7 @@ function computeExpirationTime(lane: Lane, currentTime: number) {
 if(priority>=InputContinuousLanePriority){
 // User interactions should expire slightly more quickly.
 returncurrentTime+1000;
-}elseif(priority>=TransitionLongLanePriority){
+}elseif(priority>=TransitionPriority){
 returncurrentTime+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(
 }
 returnlane;
 }
-caseTransitionShortLanePriority: // Should be handled by findTransitionLane instead
-caseTransitionLongLanePriority:
+caseTransitionPriority: // Should be handled by findTransitionLane instead
 caseRetryLanePriority: // Should be handled by findRetryLane instead
 break;
 caseIdleLanePriority:
@@ -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.
-exportfunctionfindTransitionLane(
-lanePriority: LanePriority,
-wipLanes: Lanes,
-pendingLanes: Lanes,
-): Lane{
-if(lanePriority===TransitionShortLanePriority){
-// First look for lanes that are completely unclaimed, i.e. have no
-// pending work.
-letlane=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);
-}
-}
-returnlane;
-}
-if(lanePriority===TransitionLongLanePriority){
-// First look for lanes that are completely unclaimed, i.e. have no
-// pending work.
-letlane=pickArbitraryLane(TransitionLongLanes&~pendingLanes);
+exportfunctionfindTransitionLane(wipLanes: Lanes,pendingLanes: Lanes): Lane{
+// First look for lanes that are completely unclaimed, i.e. have no
+// pending work.
+letlane=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);
 }
-returnlane;
 }
-invariant(
-false,
-'Invalid transition priority: %s. This is a bug in React.',
-lanePriority,
-);
+returnlane;
 }
 
 // To ensure consistency across multiple updates in the same event, this should
@@ -816,18 +769,14 @@ export function getBumpedLaneForHydration(
 caseDefaultLanePriority:
 lane=DefaultHydrationLane;
 break;
-caseTransitionShortHydrationLanePriority:
-caseTransitionShortLanePriority:
-lane=TransitionShortHydrationLane;
-break;
-caseTransitionLongHydrationLanePriority:
-caseTransitionLongLanePriority:
-lane=TransitionLongHydrationLane;
+caseTransitionHydrationPriority:
+caseTransitionPriority:
+lane=TransitionHydrationLane;
 break;
 caseRetryLanePriority:
 // 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;
 caseSelectiveHydrationLanePriority:
 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.
-consttimeoutMs=suspenseConfig.timeoutMs;
-consttransitionLanePriority=
-timeoutMs===undefined||(timeoutMs|0)<10000
- ? TransitionShortLanePriority
- : TransitionLongLanePriority;
-
 if(currentEventPendingLanes!==NoLanes){
 currentEventPendingLanes=
 mostRecentlyUpdatedRoot!==null
  ? mostRecentlyUpdatedRoot.pendingLanes
  : NoLanes;
 }
-
-returnfindTransitionLane(
-transitionLanePriority,
-currentEventWipLanes,
-currentEventPendingLanes,
-);
+ return findTransitionLane(currentEventWipLanes, currentEventPendingLanes);
 }
 
 // TODO: Remove this dependency on the Scheduler priority.
@@ -936,60 +923,41 @@ function finishConcurrentRender(root, exitStatus, lanes) {
 caseRootSuspendedWithDelay: {
 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.
-constnextLanes=getNextLanes(root,NoLanes);
-if(nextLanes!==NoLanes){
-// There's additional work on this root.
-break;
-}
-constsuspendedLanes=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.
-consteventTime=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.
 
 constmostRecentEventTime=getMostRecentEventTime(root,lanes);
-letmsUntilTimeout;
-if(workInProgressRootLatestSuspenseTimeout!==NoTimestamp){
-// We have processed a suspense config whose expiration time we
-// can use as the timeout.
-msUntilTimeout=workInProgressRootLatestSuspenseTimeout-now();
-}elseif(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.
-consteventTimeMs=mostRecentEventTime;
-consttimeElapsedMs=now()-eventTimeMs;
-msUntilTimeout=jnd(timeElapsedMs)-timeElapsedMs;
-}
+consteventTimeMs=mostRecentEventTime;
+consttimeElapsedMs=now()-eventTimeMs;
+constmsUntilTimeout=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;
 }