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.

Sign up for GitHub

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

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion packages/react-reconciler/src/ReactFiberLane.js
View file
Open in desktop
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) {
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
View file
Open in desktop
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
View file
Open in desktop
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