From cd3884ddd2d7b32fbcc124fcd6bfb0262facf6af Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Thu, 12 Sep 2019 14:21:57 -0700 Subject: [PATCH] Re-enable risky work loop changes The stack of PRs in #16743 was reverted. This adds them back. --- .../src/ReactFiberBeginWork.js | 6 + .../src/ReactFiberExpirationTime.js | 3 + .../react-reconciler/src/ReactFiberHooks.js | 3 + .../react-reconciler/src/ReactFiberRoot.js | 132 +++- .../src/ReactFiberWorkLoop.js | 571 +++++++++++------- .../react-reconciler/src/ReactUpdateQueue.js | 6 +- ...tIncrementalErrorHandling-test.internal.js | 121 ++-- .../__tests__/ReactSuspense-test.internal.js | 268 ++++++++ ...tSuspenseWithNoopRenderer-test.internal.js | 67 +- ...ReactIncrementalPerf-test.internal.js.snap | 5 +- 10 files changed, 900 insertions(+), 282 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index eb4234d4aef07..f86a7baaff4a6 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -177,6 +177,7 @@ import { retryDehydratedSuspenseBoundary, scheduleWork, renderDidSuspendDelayIfPossible, + markUnprocessedUpdateTime, } from './ReactFiberWorkLoop'; const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner; @@ -2709,6 +2710,11 @@ function bailoutOnAlreadyFinishedWork( stopProfilerTimerIfRunning(workInProgress); } + const updateExpirationTime = workInProgress.expirationTime; + if (updateExpirationTime !== NoWork) { + markUnprocessedUpdateTime(updateExpirationTime); + } + // Check if the children have any pending work. const childExpirationTime = workInProgress.childExpirationTime; if (childExpirationTime < renderExpirationTime) { diff --git a/packages/react-reconciler/src/ReactFiberExpirationTime.js b/packages/react-reconciler/src/ReactFiberExpirationTime.js index 652ca93aed3be..e61ed56079e17 100644 --- a/packages/react-reconciler/src/ReactFiberExpirationTime.js +++ b/packages/react-reconciler/src/ReactFiberExpirationTime.js @@ -21,7 +21,10 @@ import { export type ExpirationTime = number; export const NoWork = 0; +// TODO: Think of a better name for Never. export const Never = 1; +// TODO: Use the Idle expiration time for idle state updates +export const Idle = 2; export const Sync = MAX_SIGNED_31_BIT_INT; export const Batched = Sync - 1; diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 9834ac842c767..3f0552ba30210 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -43,6 +43,7 @@ import { warnIfNotCurrentlyActingUpdatesInDev, warnIfNotScopedWithMatchingAct, markRenderEventTimeAndConfig, + markUnprocessedUpdateTime, } from './ReactFiberWorkLoop'; import invariant from 'shared/invariant'; @@ -531,6 +532,7 @@ export function resetHooks(): void { // This is used to reset the state of this module when a component throws. // It's also called inside mountIndeterminateComponent if we determine the // component is a module-style component. + renderExpirationTime = NoWork; currentlyRenderingFiber = null; @@ -755,6 +757,7 @@ function updateReducer( // Update the remaining priority in the queue. if (updateExpirationTime > remainingExpirationTime) { remainingExpirationTime = updateExpirationTime; + markUnprocessedUpdateTime(remainingExpirationTime); } } else { // This update does have sufficient priority. diff --git a/packages/react-reconciler/src/ReactFiberRoot.js b/packages/react-reconciler/src/ReactFiberRoot.js index 7ef93739039e0..72dba87355d72 100644 --- a/packages/react-reconciler/src/ReactFiberRoot.js +++ b/packages/react-reconciler/src/ReactFiberRoot.js @@ -14,6 +14,7 @@ import type {TimeoutHandle, NoTimeout} from './ReactFiberHostConfig'; import type {Thenable} from './ReactFiberWorkLoop'; import type {Interaction} from 'scheduler/src/Tracing'; import type {SuspenseHydrationCallbacks} from './ReactFiberSuspenseComponent'; +import type {ReactPriorityLevel} from './SchedulerWithReactIntegration'; import {noTimeout} from './ReactFiberHostConfig'; import {createHostRootFiber} from './ReactFiber'; @@ -23,6 +24,7 @@ import { enableSuspenseCallback, } from 'shared/ReactFeatureFlags'; import {unstable_getThreadID} from 'scheduler/tracing'; +import {NoPriority} from './SchedulerWithReactIntegration'; // TODO: This should be lifted into the renderer. export type Batch = { @@ -69,12 +71,20 @@ type BaseFiberRootProperties = {| callbackNode: *, // Expiration of the callback associated with this root callbackExpirationTime: ExpirationTime, + // Priority of the callback associated with this root + callbackPriority: ReactPriorityLevel, // The earliest pending expiration time that exists in the tree firstPendingTime: ExpirationTime, - // The latest pending expiration time that exists in the tree - lastPendingTime: ExpirationTime, - // The time at which a suspended component pinged the root to render again - pingTime: ExpirationTime, + // The earliest suspended expiration time that exists in the tree + firstSuspendedTime: ExpirationTime, + // The latest suspended expiration time that exists in the tree + lastSuspendedTime: ExpirationTime, + // The next known expiration time after the suspended range + nextKnownPendingLevel: ExpirationTime, + // The latest time at which a suspended component pinged the root to + // render again + lastPingedTime: ExpirationTime, + lastExpiredTime: ExpirationTime, |}; // The following attributes are only used by interaction tracing builds. @@ -117,10 +127,13 @@ function FiberRootNode(containerInfo, tag, hydrate) { this.hydrate = hydrate; this.firstBatch = null; this.callbackNode = null; - this.callbackExpirationTime = NoWork; + this.callbackPriority = NoPriority; this.firstPendingTime = NoWork; - this.lastPendingTime = NoWork; - this.pingTime = NoWork; + this.firstSuspendedTime = NoWork; + this.lastSuspendedTime = NoWork; + this.nextKnownPendingLevel = NoWork; + this.lastPingedTime = NoWork; + this.lastExpiredTime = NoWork; if (enableSchedulerTracing) { this.interactionThreadID = unstable_getThreadID(); @@ -151,3 +164,108 @@ export function createFiberRoot( return root; } + +export function isRootSuspendedAtTime( + root: FiberRoot, + expirationTime: ExpirationTime, +): boolean { + const firstSuspendedTime = root.firstSuspendedTime; + const lastSuspendedTime = root.lastSuspendedTime; + return ( + firstSuspendedTime !== NoWork && + (firstSuspendedTime >= expirationTime && + lastSuspendedTime <= expirationTime) + ); +} + +export function markRootSuspendedAtTime( + root: FiberRoot, + expirationTime: ExpirationTime, +): void { + const firstSuspendedTime = root.firstSuspendedTime; + const lastSuspendedTime = root.lastSuspendedTime; + if (firstSuspendedTime < expirationTime) { + root.firstSuspendedTime = expirationTime; + } + if (lastSuspendedTime > expirationTime || firstSuspendedTime === NoWork) { + root.lastSuspendedTime = expirationTime; + } + + if (expirationTime <= root.lastPingedTime) { + root.lastPingedTime = NoWork; + } + + if (expirationTime <= root.lastExpiredTime) { + root.lastExpiredTime = NoWork; + } +} + +export function markRootUpdatedAtTime( + root: FiberRoot, + expirationTime: ExpirationTime, +): void { + // Update the range of pending times + const firstPendingTime = root.firstPendingTime; + if (expirationTime > firstPendingTime) { + root.firstPendingTime = expirationTime; + } + + // Update the range of suspended times. Treat everything lower priority or + // equal to this update as unsuspended. + const firstSuspendedTime = root.firstSuspendedTime; + if (firstSuspendedTime !== NoWork) { + if (expirationTime >= firstSuspendedTime) { + // The entire suspended range is now unsuspended. + root.firstSuspendedTime = root.lastSuspendedTime = root.nextKnownPendingLevel = NoWork; + } else if (expirationTime >= root.lastSuspendedTime) { + root.lastSuspendedTime = expirationTime + 1; + } + + // This is a pending level. Check if it's higher priority than the next + // known pending level. + if (expirationTime > root.nextKnownPendingLevel) { + root.nextKnownPendingLevel = expirationTime; + } + } +} + +export function markRootFinishedAtTime( + root: FiberRoot, + finishedExpirationTime: ExpirationTime, + remainingExpirationTime: ExpirationTime, +): void { + // Update the range of pending times + root.firstPendingTime = remainingExpirationTime; + + // Update the range of suspended times. Treat everything higher priority or + // equal to this update as unsuspended. + if (finishedExpirationTime <= root.lastSuspendedTime) { + // The entire suspended range is now unsuspended. + root.firstSuspendedTime = root.lastSuspendedTime = root.nextKnownPendingLevel = NoWork; + } else if (finishedExpirationTime <= root.firstSuspendedTime) { + // Part of the suspended range is now unsuspended. Narrow the range to + // include everything between the unsuspended time (non-inclusive) and the + // last suspended time. + root.firstSuspendedTime = finishedExpirationTime - 1; + } + + if (finishedExpirationTime <= root.lastPingedTime) { + // Clear the pinged time + root.lastPingedTime = NoWork; + } + + if (finishedExpirationTime <= root.lastExpiredTime) { + // Clear the expired time + root.lastExpiredTime = NoWork; + } +} + +export function markRootExpiredAtTime( + root: FiberRoot, + expirationTime: ExpirationTime, +): void { + const lastExpiredTime = root.lastExpiredTime; + if (lastExpiredTime === NoWork || lastExpiredTime > expirationTime) { + root.lastExpiredTime = expirationTime; + } +} diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index cfa2f3e0e959e..19557350c92e1 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -10,10 +10,7 @@ import type {Fiber} from './ReactFiber'; import type {FiberRoot} from './ReactFiberRoot'; import type {ExpirationTime} from './ReactFiberExpirationTime'; -import type { - ReactPriorityLevel, - SchedulerCallback, -} from './SchedulerWithReactIntegration'; +import type {ReactPriorityLevel} from './SchedulerWithReactIntegration'; import type {Interaction} from 'scheduler/src/Tracing'; import type {SuspenseConfig} from './ReactFiberSuspenseConfig'; import type {SuspenseState} from './ReactFiberSuspenseComponent'; @@ -66,6 +63,13 @@ import { } from './ReactFiberHostConfig'; import {createWorkInProgress, assignFiberPropertiesInDEV} from './ReactFiber'; +import { + isRootSuspendedAtTime, + markRootSuspendedAtTime, + markRootFinishedAtTime, + markRootUpdatedAtTime, + markRootExpiredAtTime, +} from './ReactFiberRoot'; import { NoMode, StrictMode, @@ -112,6 +116,7 @@ import { inferPriorityFromExpirationTime, LOW_PRIORITY_EXPIRATION, Batched, + Idle, } from './ReactFiberExpirationTime'; import {beginWork as originalBeginWork} from './ReactFiberBeginWork'; import {completeWork} from './ReactFiberCompleteWork'; @@ -223,6 +228,10 @@ let workInProgressRootExitStatus: RootExitStatus = RootIncomplete; let workInProgressRootLatestProcessedExpirationTime: ExpirationTime = Sync; let workInProgressRootLatestSuspenseTimeout: ExpirationTime = Sync; let workInProgressRootCanSuspendUsingConfig: null | SuspenseConfig = null; +// The work left over by components that were visited during this render. Only +// includes unprocessed updates, not work in bailed out children. +let workInProgressRootNextUnprocessedUpdateTime: ExpirationTime = NoWork; + // If we're pinged while rendering we don't always restart immediately. // This flag determines if it might be worthwhile to restart if an opportunity // happens latere. @@ -378,8 +387,6 @@ export function scheduleUpdateOnFiber( return; } - root.pingTime = NoWork; - checkForInterruption(fiber, expirationTime); recordScheduleUpdate(); @@ -400,12 +407,10 @@ export function scheduleUpdateOnFiber( // This is a legacy edge case. The initial mount of a ReactDOM.render-ed // root inside of batchedUpdates should be synchronous, but layout updates // should be deferred until the end of the batch. - let callback = renderRoot(root, Sync, true); - while (callback !== null) { - callback = callback(true); - } + performSyncWorkOnRoot(root); } else { - scheduleCallbackForRoot(root, ImmediatePriority, Sync); + ensureRootIsScheduled(root); + schedulePendingInteractions(root, expirationTime); if (executionContext === NoContext) { // Flush the synchronous work now, unless we're already working or inside // a batch. This is intentionally inside scheduleUpdateOnFiber instead of @@ -416,7 +421,8 @@ export function scheduleUpdateOnFiber( } } } else { - scheduleCallbackForRoot(root, priorityLevel, expirationTime); + ensureRootIsScheduled(root); + schedulePendingInteractions(root, expirationTime); } if ( @@ -484,97 +490,193 @@ function markUpdateTimeFromFiberToRoot(fiber, expirationTime) { } if (root !== null) { - // Update the first and last pending expiration times in this root - const firstPendingTime = root.firstPendingTime; - if (expirationTime > firstPendingTime) { - root.firstPendingTime = expirationTime; - } - const lastPendingTime = root.lastPendingTime; - if (lastPendingTime === NoWork || expirationTime < lastPendingTime) { - root.lastPendingTime = expirationTime; + if (workInProgressRoot === root) { + // Received an update to a tree that's in the middle of rendering. Mark + // that's unprocessed work on this root. + markUnprocessedUpdateTime(expirationTime); + + if (workInProgressRootExitStatus === RootSuspendedWithDelay) { + // The root already suspended with a delay, which means this render + // definitely won't finish. Since we have a new update, let's mark it as + // suspended now, right before marking the incoming update. This has the + // effect of interrupting the current render and switching to the update. + // TODO: This happens to work when receiving an update during the render + // phase, because of the trick inside computeExpirationForFiber to + // subtract 1 from `renderExpirationTime` to move it into a + // separate bucket. But we should probably model it with an exception, + // using the same mechanism we use to force hydration of a subtree. + // TODO: This does not account for low pri updates that were already + // scheduled before the root started rendering. Need to track the next + // pending expiration time (perhaps by backtracking the return path) and + // then trigger a restart in the `renderDidSuspendDelayIfPossible` path. + markRootSuspendedAtTime(root, renderExpirationTime); + } } + // Mark that the root has a pending update. + markRootUpdatedAtTime(root, expirationTime); } return root; } -// Use this function, along with runRootCallback, to ensure that only a single -// callback per root is scheduled. It's still possible to call renderRoot -// directly, but scheduling via this function helps avoid excessive callbacks. -// It works by storing the callback node and expiration time on the root. When a -// new callback comes in, it compares the expiration time to determine if it -// should cancel the previous one. It also relies on commitRoot scheduling a -// callback to render the next level, because that means we don't need a -// separate callback per expiration time. -function scheduleCallbackForRoot( - root: FiberRoot, - priorityLevel: ReactPriorityLevel, - expirationTime: ExpirationTime, -) { - const existingCallbackExpirationTime = root.callbackExpirationTime; - if (existingCallbackExpirationTime < expirationTime) { - // New callback has higher priority than the existing one. - const existingCallbackNode = root.callbackNode; +function getNextRootExpirationTimeToWorkOn(root: FiberRoot): ExpirationTime { + // Determines the next expiration time that the root should render, taking + // into account levels that may be suspended, or levels that may have + // received a ping. + + const lastExpiredTime = root.lastExpiredTime; + if (lastExpiredTime !== NoWork) { + return lastExpiredTime; + } + + // "Pending" refers to any update that hasn't committed yet, including if it + // suspended. The "suspended" range is therefore a subset. + const firstPendingTime = root.firstPendingTime; + if (!isRootSuspendedAtTime(root, firstPendingTime)) { + // The highest priority pending time is not suspended. Let's work on that. + return firstPendingTime; + } + + // If the first pending time is suspended, check if there's a lower priority + // pending level that we know about. Or check if we received a ping. Work + // on whichever is higher priority. + const lastPingedTime = root.lastPingedTime; + const nextKnownPendingLevel = root.nextKnownPendingLevel; + return lastPingedTime > nextKnownPendingLevel + ? lastPingedTime + : nextKnownPendingLevel; +} + +// Use this function to schedule a task for a root. There's only one task per +// root; if a task was already scheduled, we'll check to make sure the +// expiration time of the existing task is the same as the expiration time of +// the next level that the root has work on. This function is called on every +// update, and right before exiting a task. +function ensureRootIsScheduled(root: FiberRoot) { + const lastExpiredTime = root.lastExpiredTime; + if (lastExpiredTime !== NoWork) { + // Special case: Expired work should flush synchronously. + root.callbackExpirationTime = Sync; + root.callbackPriority = ImmediatePriority; + root.callbackNode = scheduleSyncCallback( + performSyncWorkOnRoot.bind(null, root), + ); + return; + } + + const expirationTime = getNextRootExpirationTimeToWorkOn(root); + const existingCallbackNode = root.callbackNode; + if (expirationTime === NoWork) { + // There's nothing to work on. if (existingCallbackNode !== null) { - cancelCallback(existingCallbackNode); + root.callbackNode = null; + root.callbackExpirationTime = NoWork; + root.callbackPriority = NoPriority; } - root.callbackExpirationTime = expirationTime; + return; + } - if (expirationTime === Sync) { - // Sync React callbacks are scheduled on a special internal queue - root.callbackNode = scheduleSyncCallback( - runRootCallback.bind( - null, - root, - renderRoot.bind(null, root, expirationTime), - ), - ); - } else { - let options = null; - if ( - !disableSchedulerTimeoutBasedOnReactExpirationTime && - expirationTime !== Never - ) { - let timeout = expirationTimeToMs(expirationTime) - now(); - options = {timeout}; - } + // TODO: If this is an update, we already read the current time. Pass the + // time as an argument. + const currentTime = requestCurrentTime(); + const priorityLevel = inferPriorityFromExpirationTime( + currentTime, + expirationTime, + ); - root.callbackNode = scheduleCallback( - priorityLevel, - runRootCallback.bind( - null, - root, - renderRoot.bind(null, root, expirationTime), - ), - options, - ); + // If there's an existing render task, confirm it has the correct priority and + // expiration time. Otherwise, we'll cancel it and schedule a new one. + if (existingCallbackNode !== null) { + const existingCallbackPriority = root.callbackPriority; + const existingCallbackExpirationTime = root.callbackExpirationTime; + if ( + // Callback must have the exact same expiration time. + existingCallbackExpirationTime === expirationTime && + // Callback must have greater or equal priority. + existingCallbackPriority >= priorityLevel + ) { + // Existing callback is sufficient. + return; } + // Need to schedule a new task. + // TODO: Instead of scheduling a new task, we should be able to change the + // priority of the existing one. + cancelCallback(existingCallbackNode); + } + + root.callbackExpirationTime = expirationTime; + root.callbackPriority = priorityLevel; + + let callbackNode; + if (expirationTime === Sync) { + // Sync React callbacks are scheduled on a special internal queue + callbackNode = scheduleSyncCallback(performSyncWorkOnRoot.bind(null, root)); + } else if (disableSchedulerTimeoutBasedOnReactExpirationTime) { + callbackNode = scheduleCallback( + priorityLevel, + performConcurrentWorkOnRoot.bind(null, root), + ); + } else { + callbackNode = scheduleCallback( + priorityLevel, + performConcurrentWorkOnRoot.bind(null, root), + // Compute a task timeout based on the expiration time. This also affects + // ordering because tasks are processed in timeout order. + {timeout: expirationTimeToMs(expirationTime) - now()}, + ); } - // Associate the current interactions with this new root+priority. - schedulePendingInteractions(root, expirationTime); + root.callbackNode = callbackNode; } -function runRootCallback(root, callback, isSync) { - const prevCallbackNode = root.callbackNode; - let continuation = null; - try { - continuation = callback(isSync); - if (continuation !== null) { - return runRootCallback.bind(null, root, continuation); - } else { - return null; +// This is the entry point for every concurrent task, i.e. anything that +// goes through Scheduler. +function performConcurrentWorkOnRoot(root, didTimeout) { + // Since we know we're in a React event, we can clear the current + // event time. The next update will compute a new event time. + currentEventTime = NoWork; + + if (didTimeout) { + // An async update expired. + const currentTime = requestCurrentTime(); + markRootExpiredAtTime(root, currentTime); + } + + // Determine the next expiration time to work on, using the fields stored + // on the root. + const expirationTime = getNextRootExpirationTimeToWorkOn(root); + if (expirationTime !== NoWork) { + const originalCallbackNode = root.callbackNode; + try { + renderRoot(root, expirationTime, didTimeout); + if (root.callbackNode === originalCallbackNode) { + // The task node scheduled for this root is the same one that's + // currently executed. Need to return a continuation. + return performConcurrentWorkOnRoot.bind(null, root); + } + } finally { + // Before exiting, make sure there's a callback scheduled for the + // pending level. + ensureRootIsScheduled(root); } + } + return null; +} + +// This is the entry point for synchronous tasks that don't go +// through Scheduler +function performSyncWorkOnRoot(root) { + // Check if there's expired work on this root. Otherwise, render at Sync. + const lastExpiredTime = root.lastExpiredTime; + const expirationTime = lastExpiredTime !== NoWork ? lastExpiredTime : Sync; + try { + renderRoot(root, expirationTime, true); } finally { - // If the callback exits without returning a continuation, remove the - // corresponding callback node from the root. Unless the callback node - // has changed, which implies that it was already cancelled by a high - // priority update. - if (continuation === null && prevCallbackNode === root.callbackNode) { - root.callbackNode = null; - root.callbackExpirationTime = NoWork; - } + // Before exiting, make sure there's a callback scheduled for the + // pending level. + ensureRootIsScheduled(root); } + return null; } export function flushRoot(root: FiberRoot, expirationTime: ExpirationTime) { @@ -585,7 +687,8 @@ export function flushRoot(root: FiberRoot, expirationTime: ExpirationTime) { 'means you attempted to commit from inside a lifecycle method.', ); } - scheduleSyncCallback(renderRoot.bind(null, root, expirationTime)); + markRootExpiredAtTime(root, expirationTime); + ensureRootIsScheduled(root); flushSyncCallbackQueue(); } @@ -654,7 +757,8 @@ function flushPendingDiscreteUpdates() { const roots = rootsWithPendingDiscreteUpdates; rootsWithPendingDiscreteUpdates = null; roots.forEach((expirationTime, root) => { - scheduleSyncCallback(renderRoot.bind(null, root, expirationTime)); + markRootExpiredAtTime(root, expirationTime); + ensureRootIsScheduled(root); }); // Now flush the immediate queue. flushSyncCallbackQueue(); @@ -786,6 +890,7 @@ function prepareFreshStack(root, expirationTime) { workInProgressRootLatestProcessedExpirationTime = Sync; workInProgressRootLatestSuspenseTimeout = Sync; workInProgressRootCanSuspendUsingConfig = null; + workInProgressRootNextUnprocessedUpdateTime = NoWork; workInProgressRootHasPendingPing = false; if (enableSchedulerTracing) { @@ -798,28 +903,24 @@ function prepareFreshStack(root, expirationTime) { } } +// renderRoot should only be called from inside either +// `performConcurrentWorkOnRoot` or `performSyncWorkOnRoot`. function renderRoot( root: FiberRoot, expirationTime: ExpirationTime, isSync: boolean, -): SchedulerCallback | null { +): void { invariant( (executionContext & (RenderContext | CommitContext)) === NoContext, 'Should not already be working.', ); - if (root.firstPendingTime < expirationTime) { - // If there's no work left at this expiration time, exit immediately. This - // happens when multiple callbacks are scheduled for a single root, but an - // earlier callback flushes the work of a later one. - return null; - } - if (isSync && root.finishedExpirationTime === expirationTime) { // There's already a pending commit at this expiration time. // TODO: This is poorly factored. This case only exists for the // batch.commit() API. - return commitRoot.bind(null, root); + commitRoot(root); + return; } flushPassiveEffects(); @@ -829,26 +930,6 @@ function renderRoot( if (root !== workInProgressRoot || expirationTime !== renderExpirationTime) { prepareFreshStack(root, expirationTime); startWorkOnPendingInteractions(root, expirationTime); - } else if (workInProgressRootExitStatus === RootSuspendedWithDelay) { - // We could've received an update at a lower priority while we yielded. - // We're suspended in a delayed state. Once we complete this render we're - // just going to try to recover at the last pending time anyway so we might - // as well start doing that eagerly. - // Ideally we should be able to do this even for retries but we don't yet - // know if we're going to process an update which wants to commit earlier, - // and this path happens very early so it would happen too often. Instead, - // for that case, we'll wait until we complete. - if (workInProgressRootHasPendingPing) { - // We have a ping at this expiration. Let's restart to see if we get unblocked. - prepareFreshStack(root, expirationTime); - } else { - const lastPendingTime = root.lastPendingTime; - if (lastPendingTime < expirationTime) { - // There's lower priority work. It might be unsuspended. Try rendering - // at that level immediately, while preserving the position in the queue. - return renderRoot.bind(null, root, lastPendingTime); - } - } } // If we have a work-in-progress fiber, it means there's still work to do @@ -872,32 +953,6 @@ function renderRoot( startWorkLoopTimer(workInProgress); - // TODO: Fork renderRoot into renderRootSync and renderRootAsync - if (isSync) { - if (expirationTime !== Sync) { - // An async update expired. There may be other expired updates on - // this root. We should render all the expired work in a - // single batch. - const currentTime = requestCurrentTime(); - if (currentTime < expirationTime) { - // Restart at the current time. - executionContext = prevExecutionContext; - resetContextDependencies(); - ReactCurrentDispatcher.current = prevDispatcher; - if (enableSchedulerTracing) { - __interactionsRef.current = ((prevInteractions: any): Set< - Interaction, - >); - } - return renderRoot.bind(null, root, currentTime); - } - } - } else { - // Since we know we're in a React event, we can clear the current - // event time. The next update will compute a new event time. - currentEventTime = NoWork; - } - do { try { if (isSync) { @@ -919,6 +974,7 @@ function renderRoot( // boundary. prepareFreshStack(root, expirationTime); executionContext = prevExecutionContext; + markRootSuspendedAtTime(root, expirationTime); throw thrownValue; } @@ -937,6 +993,8 @@ function renderRoot( thrownValue, renderExpirationTime, ); + // TODO: This is not wrapped in a try-catch, so if the complete phase + // throws, we won't capture it. workInProgress = completeUnitOfWork(sourceFiber); } } while (true); @@ -949,9 +1007,9 @@ function renderRoot( } if (workInProgress !== null) { - // There's still work left over. Return a continuation. + // There's still work left over. Exit without committing. stopInterruptedWorkLoopTimer(); - return renderRoot.bind(null, root, expirationTime); + return; } } @@ -959,7 +1017,8 @@ function renderRoot( // something suspended, wait to commit it after a timeout. stopFinishedWorkLoopTimer(); - root.finishedWork = root.current.alternate; + const finishedWork: Fiber = ((root.finishedWork = + root.current.alternate): any); root.finishedExpirationTime = expirationTime; const isLocked = resolveLocksOnRoot(root, expirationTime); @@ -967,7 +1026,8 @@ function renderRoot( // This root has a lock that prevents it from committing. Exit. If we begin // work on the root again, without any intervening updates, it will finish // without doing additional work. - return null; + markRootSuspendedAtTime(root, expirationTime); + return; } // Set this to null to indicate there's no in-progress render. @@ -981,28 +1041,25 @@ function renderRoot( // but eslint doesn't know about invariant, so it complains if I do. // eslint-disable-next-line no-fallthrough case RootErrored: { - // An error was thrown. First check if there is lower priority work - // scheduled on this root. - const lastPendingTime = root.lastPendingTime; - if (lastPendingTime < expirationTime) { - // There's lower priority work. Before raising the error, try rendering - // at the lower priority to see if it fixes it. Use a continuation to - // maintain the existing priority and position in the queue. - return renderRoot.bind(null, root, lastPendingTime); - } - if (!isSync) { - // If we're rendering asynchronously, it's possible the error was - // caused by tearing due to a mutation during an event. Try rendering - // one more time without yiedling to events. - prepareFreshStack(root, expirationTime); - scheduleSyncCallback(renderRoot.bind(null, root, expirationTime)); - return null; + if (!isSync && expirationTime !== Idle) { + // If this was an async render, the error may have happened due to + // a mutation in a concurrent event. Try rendering one more time, + // synchronously, to see if the error goes away. If there are lower + // priority updates, let's include those, too, in case they fix the + // inconsistency. Render at Idle to include all updates. + markRootExpiredAtTime(root, Idle); + return; } - // If we're already rendering synchronously, commit the root in its - // errored state. - return commitRoot.bind(null, root); + // Commit the root in its errored state. + commitRoot(root); + return; } case RootSuspended: { + markRootSuspendedAtTime(root, expirationTime); + const lastSuspendedTime = root.lastSuspendedTime; + if (expirationTime === lastSuspendedTime) { + root.nextKnownPendingLevel = getRemainingExpirationTime(finishedWork); + } flushSuspensePriorityWarningInDEV(); // We have an acceptable loading state. We need to figure out if we should @@ -1034,17 +1091,31 @@ function renderRoot( // Don't bother with a very short suspense time. if (msUntilTimeout > 10) { if (workInProgressRootHasPendingPing) { - // This render was pinged but we didn't get to restart earlier so try - // restarting now instead. - prepareFreshStack(root, expirationTime); - return renderRoot.bind(null, root, expirationTime); + const lastPingedTime = root.lastPingedTime; + if (lastPingedTime === NoWork || lastPingedTime >= expirationTime) { + // This render was pinged but we didn't get to restart earlier so + // try restarting now instead. + root.lastPingedTime = expirationTime; + prepareFreshStack(root, expirationTime); + return; + } } - const lastPendingTime = root.lastPendingTime; - if (lastPendingTime < expirationTime) { - // There's lower priority work. It might be unsuspended. Try rendering - // at that level. - return renderRoot.bind(null, root, lastPendingTime); + + const nextTime = getNextRootExpirationTimeToWorkOn(root); + if (nextTime !== NoWork && nextTime !== expirationTime) { + // There's additional work on this root. + return; } + if ( + lastSuspendedTime !== NoWork && + lastSuspendedTime !== expirationTime + ) { + // We should prefer to render the fallback of at the last suspended + // level. Ping the last suspended level to try rendering it again. + root.lastPingedTime = lastSuspendedTime; + return; + } + // The render is suspended, it hasn't timed out, and there's no lower // priority work to do. Instead of committing the fallback // immediately, wait for more data to arrive. @@ -1052,13 +1123,19 @@ function renderRoot( commitRoot.bind(null, root), msUntilTimeout, ); - return null; + return; } } // The work expired. Commit immediately. - return commitRoot.bind(null, root); + commitRoot(root); + return; } case RootSuspendedWithDelay: { + markRootSuspendedAtTime(root, expirationTime); + const lastSuspendedTime = root.lastSuspendedTime; + if (expirationTime === lastSuspendedTime) { + root.nextKnownPendingLevel = getRemainingExpirationTime(finishedWork); + } flushSuspensePriorityWarningInDEV(); if ( @@ -1073,16 +1150,29 @@ function renderRoot( // We're suspended in a state that should be avoided. We'll try to avoid committing // it for as long as the timeouts let us. if (workInProgressRootHasPendingPing) { - // This render was pinged but we didn't get to restart earlier so try - // restarting now instead. - prepareFreshStack(root, expirationTime); - return renderRoot.bind(null, root, expirationTime); + const lastPingedTime = root.lastPingedTime; + if (lastPingedTime === NoWork || lastPingedTime >= expirationTime) { + // This render was pinged but we didn't get to restart earlier so + // try restarting now instead. + root.lastPingedTime = expirationTime; + prepareFreshStack(root, expirationTime); + return; + } + } + + const nextTime = getNextRootExpirationTimeToWorkOn(root); + if (nextTime !== NoWork && nextTime !== expirationTime) { + // There's additional work on this root. + return; } - const lastPendingTime = root.lastPendingTime; - if (lastPendingTime < expirationTime) { - // There's lower priority work. It might be unsuspended. Try rendering - // at that level immediately. - return renderRoot.bind(null, root, lastPendingTime); + if ( + lastSuspendedTime !== NoWork && + lastSuspendedTime !== expirationTime + ) { + // We should prefer to render the fallback of at the last suspended + // level. Ping the last suspended level to try rendering it again. + root.lastPingedTime = lastSuspendedTime; + return; } let msUntilTimeout; @@ -1130,11 +1220,12 @@ function renderRoot( commitRoot.bind(null, root), msUntilTimeout, ); - return null; + return; } } // The work expired. Commit immediately. - return commitRoot.bind(null, root); + commitRoot(root); + return; } case RootCompleted: { // The work completed. Ready to commit. @@ -1158,14 +1249,16 @@ function renderRoot( workInProgressRootCanSuspendUsingConfig, ); if (msUntilTimeout > 10) { + markRootSuspendedAtTime(root, expirationTime); root.timeoutHandle = scheduleTimeout( commitRoot.bind(null, root), msUntilTimeout, ); - return null; + return; } } - return commitRoot.bind(null, root); + commitRoot(root); + return; } default: { invariant(false, 'Unknown root exit status.'); @@ -1199,6 +1292,14 @@ export function markRenderEventTimeAndConfig( } } +export function markUnprocessedUpdateTime( + expirationTime: ExpirationTime, +): void { + if (expirationTime > workInProgressRootNextUnprocessedUpdateTime) { + workInProgressRootNextUnprocessedUpdateTime = expirationTime; + } +} + export function renderDidSuspend(): void { if (workInProgressRootExitStatus === RootIncomplete) { workInProgressRootExitStatus = RootSuspended; @@ -1212,6 +1313,22 @@ export function renderDidSuspendDelayIfPossible(): void { ) { workInProgressRootExitStatus = RootSuspendedWithDelay; } + + // Check if there's a lower priority update somewhere else in the tree. + if ( + workInProgressRootNextUnprocessedUpdateTime !== NoWork && + workInProgressRoot !== null + ) { + // Mark the current render as suspended, and then mark that there's a + // pending update. + // TODO: This should immediately interrupt the current render, instead + // of waiting until the next time we yield. + markRootSuspendedAtTime(workInProgressRoot, renderExpirationTime); + markRootUpdatedAtTime( + workInProgressRoot, + workInProgressRootNextUnprocessedUpdateTime, + ); + } } export function renderDidError() { @@ -1426,6 +1543,14 @@ function completeUnitOfWork(unitOfWork: Fiber): Fiber | null { return null; } +function getRemainingExpirationTime(fiber: Fiber) { + const updateExpirationTime = fiber.expirationTime; + const childExpirationTime = fiber.childExpirationTime; + return updateExpirationTime > childExpirationTime + ? updateExpirationTime + : childExpirationTime; +} + function resetChildExpirationTime(completedWork: Fiber) { if ( renderExpirationTime !== Never && @@ -1528,23 +1653,21 @@ function commitRootImpl(root, renderPriorityLevel) { // So we can clear these now to allow a new callback to be scheduled. root.callbackNode = null; root.callbackExpirationTime = NoWork; + root.callbackPriority = NoPriority; + root.nextKnownPendingLevel = NoWork; startCommitTimer(); // Update the first and last pending times on this root. The new first // pending time is whatever is left on the root fiber. - const updateExpirationTimeBeforeCommit = finishedWork.expirationTime; - const childExpirationTimeBeforeCommit = finishedWork.childExpirationTime; - const firstPendingTimeBeforeCommit = - childExpirationTimeBeforeCommit > updateExpirationTimeBeforeCommit - ? childExpirationTimeBeforeCommit - : updateExpirationTimeBeforeCommit; - root.firstPendingTime = firstPendingTimeBeforeCommit; - if (firstPendingTimeBeforeCommit < root.lastPendingTime) { - // This usually means we've finished all the work, but it can also happen - // when something gets downprioritized during render, like a hidden tree. - root.lastPendingTime = firstPendingTimeBeforeCommit; - } + const remainingExpirationTimeBeforeCommit = getRemainingExpirationTime( + finishedWork, + ); + markRootFinishedAtTime( + root, + expirationTime, + remainingExpirationTimeBeforeCommit, + ); if (root === workInProgressRoot) { // We can reset these now that they are finished. @@ -1746,12 +1869,6 @@ function commitRootImpl(root, renderPriorityLevel) { // Check if there's remaining work on this root const remainingExpirationTime = root.firstPendingTime; if (remainingExpirationTime !== NoWork) { - const currentTime = requestCurrentTime(); - const priorityLevel = inferPriorityFromExpirationTime( - currentTime, - remainingExpirationTime, - ); - if (enableSchedulerTracing) { if (spawnedWorkDuringRender !== null) { const expirationTimes = spawnedWorkDuringRender; @@ -1764,9 +1881,8 @@ function commitRootImpl(root, renderPriorityLevel) { ); } } + schedulePendingInteractions(root, remainingExpirationTime); } - - scheduleCallbackForRoot(root, priorityLevel, remainingExpirationTime); } else { // If there's no remaining work, we can clear the set of already failed // error boundaries. @@ -1783,8 +1899,6 @@ function commitRootImpl(root, renderPriorityLevel) { } } - onCommitRoot(finishedWork.stateNode, expirationTime); - if (remainingExpirationTime === Sync) { // Count the number of times the root synchronously re-renders without // finishing. If there are too many, it indicates an infinite update loop. @@ -1798,6 +1912,12 @@ function commitRootImpl(root, renderPriorityLevel) { nestedUpdateCount = 0; } + onCommitRoot(finishedWork.stateNode, expirationTime); + + // Always call this before exiting `commitRoot`, to ensure that any + // additional work on this root is scheduled. + ensureRootIsScheduled(root); + if (hasUncaughtError) { hasUncaughtError = false; const error = firstUncaughtError; @@ -2062,7 +2182,8 @@ function captureCommitPhaseErrorOnRoot( enqueueUpdate(rootFiber, update); const root = markUpdateTimeFromFiberToRoot(rootFiber, Sync); if (root !== null) { - scheduleCallbackForRoot(root, ImmediatePriority, Sync); + ensureRootIsScheduled(root); + schedulePendingInteractions(root, Sync); } } @@ -2097,7 +2218,8 @@ export function captureCommitPhaseError(sourceFiber: Fiber, error: mixed) { enqueueUpdate(fiber, update); const root = markUpdateTimeFromFiberToRoot(fiber, Sync); if (root !== null) { - scheduleCallbackForRoot(root, ImmediatePriority, Sync); + ensureRootIsScheduled(root); + schedulePendingInteractions(root, Sync); } return; } @@ -2149,20 +2271,19 @@ export function pingSuspendedRoot( return; } - const lastPendingTime = root.lastPendingTime; - if (lastPendingTime < suspendedTime) { + if (!isRootSuspendedAtTime(root, suspendedTime)) { // The root is no longer suspended at this time. return; } - const pingTime = root.pingTime; - if (pingTime !== NoWork && pingTime < suspendedTime) { + const lastPingedTime = root.lastPingedTime; + if (lastPingedTime !== NoWork && lastPingedTime < suspendedTime) { // There's already a lower priority ping scheduled. return; } // Mark the time at which this ping was scheduled. - root.pingTime = suspendedTime; + root.lastPingedTime = suspendedTime; if (root.finishedExpirationTime === suspendedTime) { // If there's a pending fallback waiting to commit, throw it away. @@ -2170,12 +2291,8 @@ export function pingSuspendedRoot( root.finishedWork = null; } - const currentTime = requestCurrentTime(); - const priorityLevel = inferPriorityFromExpirationTime( - currentTime, - suspendedTime, - ); - scheduleCallbackForRoot(root, priorityLevel, suspendedTime); + ensureRootIsScheduled(root); + schedulePendingInteractions(root, suspendedTime); } function retryTimedOutBoundary( @@ -2186,9 +2303,9 @@ function retryTimedOutBoundary( // previously was rendered in its fallback state. One of the promises that // suspended it has resolved, which means at least part of the tree was // likely unblocked. Try rendering again, at a new expiration time. - const currentTime = requestCurrentTime(); if (retryTime === Never) { const suspenseConfig = null; // Retries don't carry over the already committed update. + const currentTime = requestCurrentTime(); retryTime = computeExpirationForFiber( currentTime, boundaryFiber, @@ -2196,10 +2313,10 @@ function retryTimedOutBoundary( ); } // TODO: Special case idle priority? - const priorityLevel = inferPriorityFromExpirationTime(currentTime, retryTime); const root = markUpdateTimeFromFiberToRoot(boundaryFiber, retryTime); if (root !== null) { - scheduleCallbackForRoot(root, priorityLevel, retryTime); + ensureRootIsScheduled(root); + schedulePendingInteractions(root, retryTime); } } diff --git a/packages/react-reconciler/src/ReactUpdateQueue.js b/packages/react-reconciler/src/ReactUpdateQueue.js index 4356d445c1ebc..bd3b73c5e9592 100644 --- a/packages/react-reconciler/src/ReactUpdateQueue.js +++ b/packages/react-reconciler/src/ReactUpdateQueue.js @@ -103,7 +103,10 @@ import { } from 'shared/ReactFeatureFlags'; import {StrictMode} from './ReactTypeOfMode'; -import {markRenderEventTimeAndConfig} from './ReactFiberWorkLoop'; +import { + markRenderEventTimeAndConfig, + markUnprocessedUpdateTime, +} from './ReactFiberWorkLoop'; import invariant from 'shared/invariant'; import warningWithoutStack from 'shared/warningWithoutStack'; @@ -580,6 +583,7 @@ export function processUpdateQueue( // dealt with the props. Context in components that specify // shouldComponentUpdate is tricky; but we'll have to account for // that regardless. + markUnprocessedUpdateTime(newExpirationTime); workInProgress.expirationTime = newExpirationTime; workInProgress.memoizedState = resultState; diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js index 9020e3d739cb9..094758594de45 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js @@ -243,11 +243,20 @@ describe('ReactIncrementalErrorHandling', () => { // This update is in a separate batch ReactNoop.render(, onCommit); - expect(Scheduler).toFlushAndYield([ + expect(Scheduler).toFlushAndYieldThrough([ // The first render fails. But because there's a lower priority pending // update, it doesn't throw. 'error', - // Now we retry at the lower priority. This time it succeeds. + ]); + + // React will try to recover by rendering all the pending updates in a + // single batch, synchronously. This time it succeeds. + // + // This tells Scheduler to render a single unit of work. Because the render + // to recover from the error is synchronous, this should be enough to + // finish the rest of the work. + Scheduler.unstable_flushNumberOfYields(1); + expect(Scheduler).toHaveYielded([ 'success', // Nothing commits until the second update completes. 'commit', @@ -256,54 +265,80 @@ describe('ReactIncrementalErrorHandling', () => { expect(ReactNoop.getChildren()).toEqual([span('Everything is fine.')]); }); - it('on error, retries at a lower priority using the expiration of higher priority', () => { - class Parent extends React.Component { - state = {hideChild: false}; - componentDidUpdate() { - Scheduler.unstable_yieldValue('commit: ' + this.state.hideChild); - } - render() { - if (this.state.hideChild) { - Scheduler.unstable_yieldValue('(empty)'); - return ; - } - return ; + it('does not include offscreen work when retrying after an error', () => { + function App(props) { + if (props.isBroken) { + Scheduler.unstable_yieldValue('error'); + throw new Error('Oops!'); } + Scheduler.unstable_yieldValue('success'); + return ( + <> + Everything is fine + + + ); } - function Child(props) { - if (props.isBroken) { - Scheduler.unstable_yieldValue('Error!'); - throw new Error('Error!'); - } - Scheduler.unstable_yieldValue('Child'); - return ; + function onCommit() { + Scheduler.unstable_yieldValue('commit'); + } + + function interrupt() { + ReactNoop.flushSync(() => { + ReactNoop.renderToRootWithID(null, 'other-root'); + }); } - // Initial mount - const parent = React.createRef(null); - ReactNoop.render(); - expect(Scheduler).toFlushAndYield(['Child']); - expect(ReactNoop.getChildren()).toEqual([span('Child')]); + ReactNoop.render(, onCommit); + Scheduler.unstable_advanceTime(1000); + expect(Scheduler).toFlushAndYieldThrough(['error']); + interrupt(); - // Schedule a low priority update to hide the child - parent.current.setState({hideChild: true}); + expect(ReactNoop).toMatchRenderedOutput(null); - // Before the low priority update is flushed, synchronously trigger an - // error in the child. - ReactNoop.flushSync(() => { - ReactNoop.render(); - }); + // This update is in a separate batch + ReactNoop.render(, onCommit); + + expect(Scheduler).toFlushAndYieldThrough([ + // The first render fails. But because there's a lower priority pending + // update, it doesn't throw. + 'error', + ]); + + // React will try to recover by rendering all the pending updates in a + // single batch, synchronously. This time it succeeds. + // + // This tells Scheduler to render a single unit of work. Because the render + // to recover from the error is synchronous, this should be enough to + // finish the rest of the work. + Scheduler.unstable_flushNumberOfYields(1); expect(Scheduler).toHaveYielded([ - // First the sync update triggers an error - 'Error!', - // Because there's a pending low priority update, we restart at the - // lower priority. This hides the children, suppressing the error. - '(empty)', - // Now the tree can commit. - 'commit: true', + 'success', + // Nothing commits until the second update completes. + 'commit', + 'commit', ]); - expect(ReactNoop.getChildren()).toEqual([span('(empty)')]); + // This should not include the offscreen content + expect(ReactNoop).toMatchRenderedOutput( + <> + Everything is fine +