Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Lanes to check if a render is a Suspense retry #19307

Merged
merged 1 commit into from Jul 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion packages/react-reconciler/src/ReactFiberLane.js
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) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This rename is unrelated. Just felt slightly better than the old one.

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
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
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