[Suspense] Change Suspending and Restarting Heuristics

packages/react-reconciler/src/ReactFiberBeginWork.js

@@ -1413,6 +1413,9 @@ function validateFunctionComponentInDev(workInProgress: Fiber, Component: any) {
   }
 }
 
+// TODO: This is now an empty object. Should we just make it a boolean?
+const SUSPENDED_MARKER: SuspenseState = ({}: any);
+
 function updateSuspenseComponent(
   current,
   workInProgress,
@@ -1440,17 +1443,9 @@ function updateSuspenseComponent(
       (ForceSuspenseFallback: SuspenseContext),
     )
   ) {
-    // This either already captured or is a new mount that was forced into its fallback
-    // state by a parent.
-    const attemptedState: SuspenseState | null = workInProgress.memoizedState;
     // Something in this boundary's subtree already suspended. Switch to
     // rendering the fallback children.
-    nextState = {
-      fallbackExpirationTime:
-        attemptedState !== null
-          ? attemptedState.fallbackExpirationTime
-          : NoWork,
-    };
+    nextState = SUSPENDED_MARKER;
     nextDidTimeout = true;
     workInProgress.effectTag &= ~DidCapture;
   } else {

packages/react-reconciler/src/ReactFiberCommitWork.js

@@ -63,10 +63,6 @@ import invariant from 'shared/invariant';
 import warningWithoutStack from 'shared/warningWithoutStack';
 import warning from 'shared/warning';
 
-import {
-  NoWork,
-  computeAsyncExpirationNoBucket,
-} from './ReactFiberExpirationTime';
 import {onCommitUnmount} from './ReactFiberDevToolsHook';
 import {startPhaseTimer, stopPhaseTimer} from './ReactDebugFiberPerf';
 import {getStackByFiberInDevAndProd} from './ReactCurrentFiber';
@@ -102,8 +98,8 @@ import {
 } from './ReactFiberHostConfig';
 import {
   captureCommitPhaseError,
-  requestCurrentTime,
   resolveRetryThenable,
+  markCommitTimeOfFallback,
 } from './ReactFiberWorkLoop';
 import {
   NoEffect as NoHookEffect,
@@ -1288,16 +1284,7 @@ function commitSuspenseComponent(finishedWork: Fiber) {
   } else {
     newDidTimeout = true;
     primaryChildParent = finishedWork.child;
-    if (newState.fallbackExpirationTime === NoWork) {
-      // If the children had not already timed out, record the time.
-      // This is used to compute the elapsed time during subsequent
-      // attempts to render the children.
-      // We model this as a normal pri expiration time since that's
-      // how we infer start time for updates.
-      newState.fallbackExpirationTime = computeAsyncExpirationNoBucket(
-        requestCurrentTime(),
-      );
-    }
+    markCommitTimeOfFallback();
   }
 
   if (supportsMutation && primaryChildParent !== null) {

packages/react-reconciler/src/ReactFiberCompleteWork.js

@@ -101,7 +101,6 @@ import {
   enableEventAPI,
 } from 'shared/ReactFeatureFlags';
 import {
-  markRenderEventTimeAndConfig,
   renderDidSuspend,
   renderDidSuspendDelayIfPossible,
 } from './ReactFiberWorkLoop';
@@ -702,14 +701,6 @@ function completeWork(
         prevDidTimeout = prevState !== null;
         if (!nextDidTimeout && prevState !== null) {
           // We just switched from the fallback to the normal children.
-
-          // Mark the event time of the switching from fallback to normal children,
-          // based on the start of when we first showed the fallback. This time
-          // was given a normal pri expiration time at the time it was shown.
-          const fallbackExpirationTime: ExpirationTime =
-            prevState.fallbackExpirationTime;
-          markRenderEventTimeAndConfig(fallbackExpirationTime, null);
-
           // Delete the fallback.
           // TODO: Would it be better to store the fallback fragment on
           // the stateNode during the begin phase?
@@ -737,6 +728,13 @@ function completeWork(
         // in the concurrent tree already suspended during this render.
         // This is a known bug.
         if ((workInProgress.mode & BatchedMode) !== NoMode) {
+          // TODO: Move this back to throwException because this is too late
+          // if this is a large tree which is common for initial loads. We
+          // don't know if we should restart a render or not until we get
+          // this marker, and this is too late.
+          // If this render already had a ping or lower pri updates,
+          // and this is the first time we know we're going to suspend we
+          // should be able to immediately restart from within throwException.
           const hasInvisibleChildContext =
             current === null &&
             workInProgress.memoizedProps.unstable_avoidThisFallback !== true;

packages/react-reconciler/src/ReactFiberExpirationTime.js

@@ -83,14 +83,6 @@ export function computeSuspenseExpiration(
   );
 }
 
-// Same as computeAsyncExpiration but without the bucketing logic. This is
-// used to compute timestamps instead of actual expiration times.
-export function computeAsyncExpirationNoBucket(
-  currentTime: ExpirationTime,
-): ExpirationTime {
-  return currentTime - LOW_PRIORITY_EXPIRATION / UNIT_SIZE;
-}
-
 // We intentionally set a higher expiration time for interactive updates in
 // dev than in production.
 //

packages/react-reconciler/src/ReactFiberSuspenseComponent.js

@@ -8,11 +8,9 @@
  */
 
 import type {Fiber} from './ReactFiber';
-import type {ExpirationTime} from './ReactFiberExpirationTime';
 
-export type SuspenseState = {|
-  fallbackExpirationTime: ExpirationTime,
-|};
+// TODO: This is now an empty object. Should we switch this to a boolean?
+export type SuspenseState = {||};
 
 export function shouldCaptureSuspense(
   workInProgress: Fiber,

packages/react-reconciler/src/ReactFiberThrow.js

@@ -275,6 +275,45 @@ function throwException(
 
         // Confirmed that the boundary is in a concurrent mode tree. Continue
         // with the normal suspend path.
+        //
+        // After this we'll use a set of heuristics to determine whether this
+        // render pass will run to completion or restart or "suspend" the commit.
+        // The actual logic for this is spread out in different places.
+        //
+        // This first principle is that if we're going to suspend when we complete
+        // a root, then we should also restart if we get an update or ping that
+        // might unsuspend it, and vice versa. The only reason to suspend is
+        // because you think you might want to restart before committing. However,
+        // it doesn't make sense to restart only while in the period we're suspended.
+        //
+        // Restarting too aggressively is also not good because it starves out any
+        // intermediate loading state. So we use heuristics to determine when.
+
+        // Suspense Heuristics
+        //
+        // If nothing threw a Promise or all the same fallbacks are already showing,
+        // then don't suspend/restart.
+        //
+        // If this is an initial render of a new tree of Suspense boundaries and
+        // those trigger a fallback, then don't suspend/restart. We want to ensure
+        // that we can show the initial loading state as quickly as possible.
+        //
+        // If we hit a "Delayed" case, such as when we'd switch from content back into
+        // a fallback, then we should always suspend/restart. SuspenseConfig applies to
+        // this case. If none is defined, JND is used instead.
+        //
+        // If we're already showing a fallback and it gets "retried", allowing us to show
+        // another level, but there's still an inner boundary that would show a fallback,
+        // then we suspend/restart for 500ms since the last time we showed a fallback
+        // anywhere in the tree. This effectively throttles progressive loading into a
+        // consistent train of commits. This also gives us an opportunity to restart to
+        // get to the completed state slightly earlier.
+        //
+        // If there's ambiguity due to batching it's resolved in preference of:
+        // 1) "delayed", 2) "initial render", 3) "retry".
+        //
+        // We want to ensure that a "busy" state doesn't get force committed. We want to
+        // ensure that new initial loading states can commit as soon as possible.
 
         attachPingListener(root, renderExpirationTime, thenable);