From a7dabcb60afa1ec4de29501f0b298565af47f9e3 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Thu, 12 Sep 2019 14:21:57 -0700 Subject: [PATCH] Revert "Re-arrange slightly to prevent refactor hazard (#16743)" (#16769) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit ab4951fc03750a726e412b90126e0737fbd04014. * Track "pending" and "suspended" ranges A FiberRoot can have pending work at many distinct priorities. (Note: we refer to these levels as "expiration times" to distinguish the concept from Scheduler's notion of priority levels, which represent broad categories of work. React expiration times are more granualar. They're more like a concurrent thread ID, which also happens to correspond to a moment on a timeline. It's an overloaded concept and I'm handwaving over some of the details.) Given a root, there's no convenient way to read all the pending levels in the entire tree, i.e. there's no single queue-like structure that tracks all the levels, because that granularity of information is not needed by our algorithms. Instead we track the subset of information that we actually need — most importantly, the highest priority level that exists in the entire tree. Aside from that, the other information we track includes the range of pending levels that are known to be suspended, and therefore should not be worked on. This is a refactor of how that information is tracked, and what each field represents: - A *pending* level is work that is unfinished, or not yet committed. This includes work that is suspended from committing. `firstPendingTime` and `lastPendingTime` represent the range of pending work. (Previously, "pending" was the same as "not suspended.") - A *suspended* level is work that did not complete because data was missing. `firstSuspendedTime` and `lastSuspendedTime` represent the range of suspended work. It is a subset of the pending range. (These fields are new to this commit.) - `nextAfterSuspendedTime` represents the next known level that comes after the suspended range. This commit doesn't change much in terms of observable behavior. The one change is that, when a level is suspended, React will continue working on the next known level instead of jumping straight to the last pending level. Subsequent commits will use this new structure for a more substantial refactor for how tasks are scheduled per root. * Get next expiration time from FiberRoot Given a FiberRoot, we should be able to determine the next expiration time that needs to be worked on, taking into account the levels that are pending, suspended, pinged, and so on. This removes the `expirationTime` argument from `scheduleCallbackForRoot`, and renames it to `ensureRootIsScheduled` to reflect the new signature. The expiration time is instead read from the root using a new function, `getNextExpirationTimeToWorkOn`. The next step will be to remove the `expirationTime` argument from `renderRoot`, too. * Don't bind expiration time to render callback This is a fragile pattern because there's only meant to be a single task per root, running at a single expiration time. Instead of binding the expiration time to the render task, or closing over it, we should determine the correct expiration time to work on using fields we store on the root object itself. This removes the "return a continuation" pattern from the `renderRoot` function. Continuation handling is now handled by the wrapper function, which I've renamed from `runRootCallback` to `performWorkOnRoot`. That function is merely an entry point to `renderRoot`, so I've also removed the callback argument. So to sum up, at at the beginning of each task, `performWorkOnRoot` determines which expiration time to work on, then calls `renderRoot`. And before exiting, it checks if it needs to schedule another task. * Update error recovery test to match new semantics * Remove `lastPendingTime` field It's no longer used anywhere * Restart on update to already suspended root If the work-in-progress root already suspended with a delay, then the current render definitely won't finish. We should interrupt the render and switch to the incoming update. * Restart on suspend if return path has an update Similar to the previous commit, if we suspend with a delay, and something in the return path has a pending update, we should abort the current render and switch to the update instead. * Track the next unprocessed level globally Instead of backtracking the return path. The main advantage over the backtracking approach is that we don't have to backtrack from the source fiber. (The main disadvantages are that it requires another module-level variable, and that it could include updates from unrelated sibling paths.) * Re-arrange slightly to prevent refactor hazard It should not be possible to perform any work on a root without calling `ensureRootIsScheduled` before exiting. Otherwise, we could fail to schedule a callback for pending work and the app could freeze. To help prevent a future refactor from introducing such a bug, this change makes it so that `renderRoot` is always wrapped in try-finally, and the `finally` block calls `ensureRootIsScheduled`. * Remove recursive calls to `renderRoot`. There are a few leftover cases where `renderRoot` is called recursively. All of them are related to synchronously flushing work before its expiration time. We can remove these calls by tracking the last expired level on the root, similar to what we do for other types of pending work, like pings. * Remove argument from performSyncWorkOnRoot Read the expiration time from the root, like we do in performConcurrentWorkOnRoot. --- .../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, 282 insertions(+), 900 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index f86a7baaff4a..eb4234d4aef0 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -177,7 +177,6 @@ import { retryDehydratedSuspenseBoundary, scheduleWork, renderDidSuspendDelayIfPossible, - markUnprocessedUpdateTime, } from './ReactFiberWorkLoop'; const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner; @@ -2710,11 +2709,6 @@ 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 e61ed56079e1..652ca93aed3b 100644 --- a/packages/react-reconciler/src/ReactFiberExpirationTime.js +++ b/packages/react-reconciler/src/ReactFiberExpirationTime.js @@ -21,10 +21,7 @@ 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 3f0552ba3021..9834ac842c76 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -43,7 +43,6 @@ import { warnIfNotCurrentlyActingUpdatesInDev, warnIfNotScopedWithMatchingAct, markRenderEventTimeAndConfig, - markUnprocessedUpdateTime, } from './ReactFiberWorkLoop'; import invariant from 'shared/invariant'; @@ -532,7 +531,6 @@ 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; @@ -757,7 +755,6 @@ 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 72dba87355d7..7ef93739039e 100644 --- a/packages/react-reconciler/src/ReactFiberRoot.js +++ b/packages/react-reconciler/src/ReactFiberRoot.js @@ -14,7 +14,6 @@ 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'; @@ -24,7 +23,6 @@ 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 = { @@ -71,20 +69,12 @@ 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 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 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 following attributes are only used by interaction tracing builds. @@ -127,13 +117,10 @@ function FiberRootNode(containerInfo, tag, hydrate) { this.hydrate = hydrate; this.firstBatch = null; this.callbackNode = null; - this.callbackPriority = NoPriority; + this.callbackExpirationTime = NoWork; this.firstPendingTime = NoWork; - this.firstSuspendedTime = NoWork; - this.lastSuspendedTime = NoWork; - this.nextKnownPendingLevel = NoWork; - this.lastPingedTime = NoWork; - this.lastExpiredTime = NoWork; + this.lastPendingTime = NoWork; + this.pingTime = NoWork; if (enableSchedulerTracing) { this.interactionThreadID = unstable_getThreadID(); @@ -164,108 +151,3 @@ 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 19557350c92e..cfa2f3e0e959 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -10,7 +10,10 @@ import type {Fiber} from './ReactFiber'; import type {FiberRoot} from './ReactFiberRoot'; import type {ExpirationTime} from './ReactFiberExpirationTime'; -import type {ReactPriorityLevel} from './SchedulerWithReactIntegration'; +import type { + ReactPriorityLevel, + SchedulerCallback, +} from './SchedulerWithReactIntegration'; import type {Interaction} from 'scheduler/src/Tracing'; import type {SuspenseConfig} from './ReactFiberSuspenseConfig'; import type {SuspenseState} from './ReactFiberSuspenseComponent'; @@ -63,13 +66,6 @@ import { } from './ReactFiberHostConfig'; import {createWorkInProgress, assignFiberPropertiesInDEV} from './ReactFiber'; -import { - isRootSuspendedAtTime, - markRootSuspendedAtTime, - markRootFinishedAtTime, - markRootUpdatedAtTime, - markRootExpiredAtTime, -} from './ReactFiberRoot'; import { NoMode, StrictMode, @@ -116,7 +112,6 @@ import { inferPriorityFromExpirationTime, LOW_PRIORITY_EXPIRATION, Batched, - Idle, } from './ReactFiberExpirationTime'; import {beginWork as originalBeginWork} from './ReactFiberBeginWork'; import {completeWork} from './ReactFiberCompleteWork'; @@ -228,10 +223,6 @@ 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. @@ -387,6 +378,8 @@ export function scheduleUpdateOnFiber( return; } + root.pingTime = NoWork; + checkForInterruption(fiber, expirationTime); recordScheduleUpdate(); @@ -407,10 +400,12 @@ 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. - performSyncWorkOnRoot(root); + let callback = renderRoot(root, Sync, true); + while (callback !== null) { + callback = callback(true); + } } else { - ensureRootIsScheduled(root); - schedulePendingInteractions(root, expirationTime); + scheduleCallbackForRoot(root, ImmediatePriority, Sync); if (executionContext === NoContext) { // Flush the synchronous work now, unless we're already working or inside // a batch. This is intentionally inside scheduleUpdateOnFiber instead of @@ -421,8 +416,7 @@ export function scheduleUpdateOnFiber( } } } else { - ensureRootIsScheduled(root); - schedulePendingInteractions(root, expirationTime); + scheduleCallbackForRoot(root, priorityLevel, expirationTime); } if ( @@ -490,193 +484,97 @@ function markUpdateTimeFromFiberToRoot(fiber, expirationTime) { } if (root !== null) { - 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); - } + // 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; } - // Mark that the root has a pending update. - markRootUpdatedAtTime(root, expirationTime); } return root; } -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. +// 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; if (existingCallbackNode !== null) { - root.callbackNode = null; - root.callbackExpirationTime = NoWork; - root.callbackPriority = NoPriority; + cancelCallback(existingCallbackNode); } - return; - } + root.callbackExpirationTime = expirationTime; - // 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, - ); + 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}; + } - // 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; + root.callbackNode = scheduleCallback( + priorityLevel, + runRootCallback.bind( + null, + root, + renderRoot.bind(null, root, expirationTime), + ), + options, + ); } - // 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()}, - ); } - root.callbackNode = callbackNode; -} - -// 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; + // Associate the current interactions with this new root+priority. + schedulePendingInteractions(root, expirationTime); } -// 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; +function runRootCallback(root, callback, isSync) { + const prevCallbackNode = root.callbackNode; + let continuation = null; try { - renderRoot(root, expirationTime, true); + continuation = callback(isSync); + if (continuation !== null) { + return runRootCallback.bind(null, root, continuation); + } else { + return null; + } } finally { - // Before exiting, make sure there's a callback scheduled for the - // pending level. - ensureRootIsScheduled(root); + // 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; + } } - return null; } export function flushRoot(root: FiberRoot, expirationTime: ExpirationTime) { @@ -687,8 +585,7 @@ export function flushRoot(root: FiberRoot, expirationTime: ExpirationTime) { 'means you attempted to commit from inside a lifecycle method.', ); } - markRootExpiredAtTime(root, expirationTime); - ensureRootIsScheduled(root); + scheduleSyncCallback(renderRoot.bind(null, root, expirationTime)); flushSyncCallbackQueue(); } @@ -757,8 +654,7 @@ function flushPendingDiscreteUpdates() { const roots = rootsWithPendingDiscreteUpdates; rootsWithPendingDiscreteUpdates = null; roots.forEach((expirationTime, root) => { - markRootExpiredAtTime(root, expirationTime); - ensureRootIsScheduled(root); + scheduleSyncCallback(renderRoot.bind(null, root, expirationTime)); }); // Now flush the immediate queue. flushSyncCallbackQueue(); @@ -890,7 +786,6 @@ function prepareFreshStack(root, expirationTime) { workInProgressRootLatestProcessedExpirationTime = Sync; workInProgressRootLatestSuspenseTimeout = Sync; workInProgressRootCanSuspendUsingConfig = null; - workInProgressRootNextUnprocessedUpdateTime = NoWork; workInProgressRootHasPendingPing = false; if (enableSchedulerTracing) { @@ -903,24 +798,28 @@ function prepareFreshStack(root, expirationTime) { } } -// renderRoot should only be called from inside either -// `performConcurrentWorkOnRoot` or `performSyncWorkOnRoot`. function renderRoot( root: FiberRoot, expirationTime: ExpirationTime, isSync: boolean, -): void { +): SchedulerCallback | null { 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. - commitRoot(root); - return; + return commitRoot.bind(null, root); } flushPassiveEffects(); @@ -930,6 +829,26 @@ 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 @@ -953,6 +872,32 @@ 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) { @@ -974,7 +919,6 @@ function renderRoot( // boundary. prepareFreshStack(root, expirationTime); executionContext = prevExecutionContext; - markRootSuspendedAtTime(root, expirationTime); throw thrownValue; } @@ -993,8 +937,6 @@ 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); @@ -1007,9 +949,9 @@ function renderRoot( } if (workInProgress !== null) { - // There's still work left over. Exit without committing. + // There's still work left over. Return a continuation. stopInterruptedWorkLoopTimer(); - return; + return renderRoot.bind(null, root, expirationTime); } } @@ -1017,8 +959,7 @@ function renderRoot( // something suspended, wait to commit it after a timeout. stopFinishedWorkLoopTimer(); - const finishedWork: Fiber = ((root.finishedWork = - root.current.alternate): any); + root.finishedWork = root.current.alternate; root.finishedExpirationTime = expirationTime; const isLocked = resolveLocksOnRoot(root, expirationTime); @@ -1026,8 +967,7 @@ 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. - markRootSuspendedAtTime(root, expirationTime); - return; + return null; } // Set this to null to indicate there's no in-progress render. @@ -1041,25 +981,28 @@ function renderRoot( // but eslint doesn't know about invariant, so it complains if I do. // eslint-disable-next-line no-fallthrough case RootErrored: { - 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; + // 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); } - // Commit the root in its errored state. - commitRoot(root); - return; + 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 we're already rendering synchronously, commit the root in its + // errored state. + return commitRoot.bind(null, root); } 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 @@ -1091,31 +1034,17 @@ function renderRoot( // Don't bother with a very short suspense time. if (msUntilTimeout > 10) { if (workInProgressRootHasPendingPing) { - 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; + // 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); } - 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; + 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); } - // 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. @@ -1123,19 +1052,13 @@ function renderRoot( commitRoot.bind(null, root), msUntilTimeout, ); - return; + return null; } } // The work expired. Commit immediately. - commitRoot(root); - return; + return commitRoot.bind(null, root); } case RootSuspendedWithDelay: { - markRootSuspendedAtTime(root, expirationTime); - const lastSuspendedTime = root.lastSuspendedTime; - if (expirationTime === lastSuspendedTime) { - root.nextKnownPendingLevel = getRemainingExpirationTime(finishedWork); - } flushSuspensePriorityWarningInDEV(); if ( @@ -1150,29 +1073,16 @@ 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) { - 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; + // 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); } - 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; + 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); } let msUntilTimeout; @@ -1220,12 +1130,11 @@ function renderRoot( commitRoot.bind(null, root), msUntilTimeout, ); - return; + return null; } } // The work expired. Commit immediately. - commitRoot(root); - return; + return commitRoot.bind(null, root); } case RootCompleted: { // The work completed. Ready to commit. @@ -1249,16 +1158,14 @@ function renderRoot( workInProgressRootCanSuspendUsingConfig, ); if (msUntilTimeout > 10) { - markRootSuspendedAtTime(root, expirationTime); root.timeoutHandle = scheduleTimeout( commitRoot.bind(null, root), msUntilTimeout, ); - return; + return null; } } - commitRoot(root); - return; + return commitRoot.bind(null, root); } default: { invariant(false, 'Unknown root exit status.'); @@ -1292,14 +1199,6 @@ export function markRenderEventTimeAndConfig( } } -export function markUnprocessedUpdateTime( - expirationTime: ExpirationTime, -): void { - if (expirationTime > workInProgressRootNextUnprocessedUpdateTime) { - workInProgressRootNextUnprocessedUpdateTime = expirationTime; - } -} - export function renderDidSuspend(): void { if (workInProgressRootExitStatus === RootIncomplete) { workInProgressRootExitStatus = RootSuspended; @@ -1313,22 +1212,6 @@ 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() { @@ -1543,14 +1426,6 @@ 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 && @@ -1653,21 +1528,23 @@ 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 remainingExpirationTimeBeforeCommit = getRemainingExpirationTime( - finishedWork, - ); - markRootFinishedAtTime( - root, - expirationTime, - remainingExpirationTimeBeforeCommit, - ); + 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; + } if (root === workInProgressRoot) { // We can reset these now that they are finished. @@ -1869,6 +1746,12 @@ 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; @@ -1881,8 +1764,9 @@ 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. @@ -1899,6 +1783,8 @@ 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. @@ -1912,12 +1798,6 @@ 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; @@ -2182,8 +2062,7 @@ function captureCommitPhaseErrorOnRoot( enqueueUpdate(rootFiber, update); const root = markUpdateTimeFromFiberToRoot(rootFiber, Sync); if (root !== null) { - ensureRootIsScheduled(root); - schedulePendingInteractions(root, Sync); + scheduleCallbackForRoot(root, ImmediatePriority, Sync); } } @@ -2218,8 +2097,7 @@ export function captureCommitPhaseError(sourceFiber: Fiber, error: mixed) { enqueueUpdate(fiber, update); const root = markUpdateTimeFromFiberToRoot(fiber, Sync); if (root !== null) { - ensureRootIsScheduled(root); - schedulePendingInteractions(root, Sync); + scheduleCallbackForRoot(root, ImmediatePriority, Sync); } return; } @@ -2271,19 +2149,20 @@ export function pingSuspendedRoot( return; } - if (!isRootSuspendedAtTime(root, suspendedTime)) { + const lastPendingTime = root.lastPendingTime; + if (lastPendingTime < suspendedTime) { // The root is no longer suspended at this time. return; } - const lastPingedTime = root.lastPingedTime; - if (lastPingedTime !== NoWork && lastPingedTime < suspendedTime) { + const pingTime = root.pingTime; + if (pingTime !== NoWork && pingTime < suspendedTime) { // There's already a lower priority ping scheduled. return; } // Mark the time at which this ping was scheduled. - root.lastPingedTime = suspendedTime; + root.pingTime = suspendedTime; if (root.finishedExpirationTime === suspendedTime) { // If there's a pending fallback waiting to commit, throw it away. @@ -2291,8 +2170,12 @@ export function pingSuspendedRoot( root.finishedWork = null; } - ensureRootIsScheduled(root); - schedulePendingInteractions(root, suspendedTime); + const currentTime = requestCurrentTime(); + const priorityLevel = inferPriorityFromExpirationTime( + currentTime, + suspendedTime, + ); + scheduleCallbackForRoot(root, priorityLevel, suspendedTime); } function retryTimedOutBoundary( @@ -2303,9 +2186,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, @@ -2313,10 +2196,10 @@ function retryTimedOutBoundary( ); } // TODO: Special case idle priority? + const priorityLevel = inferPriorityFromExpirationTime(currentTime, retryTime); const root = markUpdateTimeFromFiberToRoot(boundaryFiber, retryTime); if (root !== null) { - ensureRootIsScheduled(root); - schedulePendingInteractions(root, retryTime); + scheduleCallbackForRoot(root, priorityLevel, retryTime); } } diff --git a/packages/react-reconciler/src/ReactUpdateQueue.js b/packages/react-reconciler/src/ReactUpdateQueue.js index bd3b73c5e959..4356d445c1eb 100644 --- a/packages/react-reconciler/src/ReactUpdateQueue.js +++ b/packages/react-reconciler/src/ReactUpdateQueue.js @@ -103,10 +103,7 @@ import { } from 'shared/ReactFeatureFlags'; import {StrictMode} from './ReactTypeOfMode'; -import { - markRenderEventTimeAndConfig, - markUnprocessedUpdateTime, -} from './ReactFiberWorkLoop'; +import {markRenderEventTimeAndConfig} from './ReactFiberWorkLoop'; import invariant from 'shared/invariant'; import warningWithoutStack from 'shared/warningWithoutStack'; @@ -583,7 +580,6 @@ 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 094758594de4..9020e3d739cb 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js @@ -243,20 +243,11 @@ describe('ReactIncrementalErrorHandling', () => { // This update is in a separate batch ReactNoop.render(, onCommit); - expect(Scheduler).toFlushAndYieldThrough([ + expect(Scheduler).toFlushAndYield([ // 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([ + // Now we retry at the lower priority. This time it succeeds. 'success', // Nothing commits until the second update completes. 'commit', @@ -265,80 +256,54 @@ describe('ReactIncrementalErrorHandling', () => { expect(ReactNoop.getChildren()).toEqual([span('Everything is fine.')]); }); - 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!'); + 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 ; } - Scheduler.unstable_yieldValue('success'); - return ( - <> - Everything is fine - - - ); - } - - function onCommit() { - Scheduler.unstable_yieldValue('commit'); } - function interrupt() { - ReactNoop.flushSync(() => { - ReactNoop.renderToRootWithID(null, 'other-root'); - }); + function Child(props) { + if (props.isBroken) { + Scheduler.unstable_yieldValue('Error!'); + throw new Error('Error!'); + } + Scheduler.unstable_yieldValue('Child'); + return ; } - ReactNoop.render(, onCommit); - Scheduler.unstable_advanceTime(1000); - expect(Scheduler).toFlushAndYieldThrough(['error']); - interrupt(); + // Initial mount + const parent = React.createRef(null); + ReactNoop.render(); + expect(Scheduler).toFlushAndYield(['Child']); + expect(ReactNoop.getChildren()).toEqual([span('Child')]); - expect(ReactNoop).toMatchRenderedOutput(null); - - // 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', - ]); + // Schedule a low priority update to hide the child + parent.current.setState({hideChild: true}); - // 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); + // Before the low priority update is flushed, synchronously trigger an + // error in the child. + ReactNoop.flushSync(() => { + ReactNoop.render(); + }); expect(Scheduler).toHaveYielded([ - 'success', - // Nothing commits until the second update completes. - 'commit', - 'commit', + // 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', ]); - // This should not include the offscreen content - expect(ReactNoop).toMatchRenderedOutput( - <> - Everything is fine -