Use lanes to check if a render is a Suspense retry (facebook#19307)

acdlite · web-flow · commit 970fa122d818 · 2020-07-10T08:58:05.000-05:00
Now that Suspense retries have their own dedicated set of lanes (facebook#19287), we can determine if a render includes only retries by checking if its lanes are a subset of the retry lanes. Previously we inferred this by checking `workInProgressRootLatestProcessedEventTime`. If it's not set, that implies that no updates were processed in the current render, which implies it must be a Suspense retry. The eventual plan is to get rid of `workInProgressRootLatestProcessedEventTime` and instead track event times on the root; this change is one the steps toward that goal. The relevant tests were originally added in facebook#15769.
diff --git a/packages/react-reconciler/src/ReactFiberLane.js b/packages/react-reconciler/src/ReactFiberLane.js
@@ -451,9 +451,12 @@ export function getLanesToRetrySynchronouslyOnError(root: FiberRoot): Lanes {
 export function returnNextLanesPriority() {
   return return_highestLanePriority;
 }
-export function hasUpdatePriority(lanes: Lanes) {
+export function includesNonIdleWork(lanes: Lanes) {
   return (lanes & NonIdleLanes) !== NoLanes;
 }
+export function includesOnlyRetries(lanes: Lanes) {
+  return (lanes & RetryLanes) === lanes;
+}
 
 // To ensure consistency across multiple updates in the same event, this should
 // be a pure function, so that it always returns the same lane for given inputs.
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
@@ -132,7 +132,8 @@ import {
   removeLanes,
   pickArbitraryLane,
   hasDiscreteLanes,
-  hasUpdatePriority,
+  includesNonIdleWork,
+  includesOnlyRetries,
   getNextLanes,
   returnNextLanesPriority,
   setCurrentUpdateLanePriority,
@@ -847,22 +848,13 @@ function finishConcurrentRender(root, finishedWork, exitStatus, lanes) {
       // We have an acceptable loading state. We need to figure out if we
       // should immediately commit it or wait a bit.
 
-      // If we have processed new updates during this render, we may now
-      // have a new loading state ready. We want to ensure that we commit
-      // that as soon as possible.
-      const hasNotProcessedNewUpdates =
-        workInProgressRootLatestProcessedEventTime === NoTimestamp;
       if (
-        hasNotProcessedNewUpdates &&
+        includesOnlyRetries(lanes) &&
         // do not delay if we're inside an act() scope
         !shouldForceFlushFallbacksInDEV()
       ) {
-        // If we have not processed any new updates during this pass, then
-        // this is either a retry of an existing fallback state or a
-        // hidden tree. Hidden trees shouldn't be batched with other work
-        // and after that's fixed it can only be a retry. We're going to
-        // throttle committing retries so that we don't show too many
-        // loading states too quickly.
+        // This render only included retries, no updates. Throttle committing
+        // retries so that we don't show too many loading states too quickly.
         const msUntilTimeout =
           globalMostRecentFallbackTime + FALLBACK_THROTTLE_MS - now();
         // Don't bother with a very short suspense time.
@@ -1475,8 +1467,8 @@ export function renderDidSuspendDelayIfPossible(): void {
   // this render.
   if (
     workInProgressRoot !== null &&
-    (hasUpdatePriority(workInProgressRootSkippedLanes) ||
-      hasUpdatePriority(workInProgressRootUpdatedLanes))
+    (includesNonIdleWork(workInProgressRootSkippedLanes) ||
+      includesNonIdleWork(workInProgressRootUpdatedLanes))
   ) {
     // Mark the current render as suspended so that we switch to working on
     // the updates that were skipped. Usually we only suspend at the end of
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
@@ -155,7 +155,8 @@ import {
   removeLanes,
   pickArbitraryLane,
   hasDiscreteLanes,
-  hasUpdatePriority,
+  includesNonIdleWork,
+  includesOnlyRetries,
   getNextLanes,
   returnNextLanesPriority,
   setCurrentUpdateLanePriority,
@@ -870,22 +871,13 @@ function finishConcurrentRender(root, finishedWork, exitStatus, lanes) {
       // We have an acceptable loading state. We need to figure out if we
       // should immediately commit it or wait a bit.
 
-      // If we have processed new updates during this render, we may now
-      // have a new loading state ready. We want to ensure that we commit
-      // that as soon as possible.
-      const hasNotProcessedNewUpdates =
-        workInProgressRootLatestProcessedEventTime === NoTimestamp;
       if (
-        hasNotProcessedNewUpdates &&
+        includesOnlyRetries(lanes) &&
         // do not delay if we're inside an act() scope
         !shouldForceFlushFallbacksInDEV()
       ) {
-        // If we have not processed any new updates during this pass, then
-        // this is either a retry of an existing fallback state or a
-        // hidden tree. Hidden trees shouldn't be batched with other work
-        // and after that's fixed it can only be a retry. We're going to
-        // throttle committing retries so that we don't show too many
-        // loading states too quickly.
+        // This render only included retries, no updates. Throttle committing
+        // retries so that we don't show too many loading states too quickly.
         const msUntilTimeout =
           globalMostRecentFallbackTime + FALLBACK_THROTTLE_MS - now();
         // Don't bother with a very short suspense time.
@@ -1498,8 +1490,8 @@ export function renderDidSuspendDelayIfPossible(): void {
   // this render.
   if (
     workInProgressRoot !== null &&
-    (hasUpdatePriority(workInProgressRootSkippedLanes) ||
-      hasUpdatePriority(workInProgressRootUpdatedLanes))
+    (includesNonIdleWork(workInProgressRootSkippedLanes) ||
+      includesNonIdleWork(workInProgressRootUpdatedLanes))
   ) {
     // Mark the current render as suspended so that we switch to working on
     // the updates that were skipped. Usually we only suspend at the end of

Original file line number	Diff line number	Diff line change
`@@ -451,9 +451,12 @@ export function getLanesToRetrySynchronouslyOnError(root: FiberRoot): Lanes {`
`451`	`451`	`export function returnNextLanesPriority() {`
`452`	`452`	`return return_highestLanePriority;`
`453`	`453`	`}`
`454`		`-export function hasUpdatePriority(lanes: Lanes) {`
	`454`	`+export function includesNonIdleWork(lanes: Lanes) {`
`455`	`455`	`return (lanes & NonIdleLanes) !== NoLanes;`
`456`	`456`	`}`
	`457`	`+export function includesOnlyRetries(lanes: Lanes) {`
	`458`	`+ return (lanes & RetryLanes) === lanes;`
	`459`	`+}`
`457`	`460`
`458`	`461`	`// To ensure consistency across multiple updates in the same event, this should`
`459`	`462`	`// be a pure function, so that it always returns the same lane for given inputs.`