Skip to content

Commit

Permalink
Use lanes to check if a render is a Suspense retry (#19307)
Browse files Browse the repository at this point in the history
Now that Suspense retries have their own dedicated set of lanes
(#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 #15769.
  • Loading branch information
acdlite committed Jul 10, 2020
1 parent 2663a12 commit 970fa12
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 31 deletions.
5 changes: 4 additions & 1 deletion packages/react-reconciler/src/ReactFiberLane.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
22 changes: 7 additions & 15 deletions packages/react-reconciler/src/ReactFiberWorkLoop.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,8 @@ import {
removeLanes,
pickArbitraryLane,
hasDiscreteLanes,
hasUpdatePriority,
includesNonIdleWork,
includesOnlyRetries,
getNextLanes,
returnNextLanesPriority,
setCurrentUpdateLanePriority,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
22 changes: 7 additions & 15 deletions packages/react-reconciler/src/ReactFiberWorkLoop.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,8 @@ import {
removeLanes,
pickArbitraryLane,
hasDiscreteLanes,
hasUpdatePriority,
includesNonIdleWork,
includesOnlyRetries,
getNextLanes,
returnNextLanesPriority,
setCurrentUpdateLanePriority,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 970fa12

Please sign in to comment.