diff --git a/packages/react-reconciler/src/ReactFiberThrow.new.js b/packages/react-reconciler/src/ReactFiberThrow.new.js index 4a75d3076ab6c..c17218f83be4e 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.new.js +++ b/packages/react-reconciler/src/ReactFiberThrow.new.js @@ -357,7 +357,7 @@ function throwException( sourceFiber: Fiber, value: mixed, rootRenderLanes: Lanes, -) { +): Wakeable | null { // The source fiber did not complete. sourceFiber.flags |= Incomplete; @@ -459,7 +459,7 @@ function throwException( if (suspenseBoundary.mode & ConcurrentMode) { attachPingListener(root, wakeable, rootRenderLanes); } - return; + return wakeable; } else { // No boundary was found. Unless this is a sync update, this is OK. // We can suspend and wait for more data to arrive. @@ -474,7 +474,7 @@ function throwException( // This case also applies to initial hydration. attachPingListener(root, wakeable, rootRenderLanes); renderDidSuspendDelayIfPossible(); - return; + return wakeable; } // This is a sync/discrete update. We treat this case like an error @@ -517,7 +517,7 @@ function throwException( // Even though the user may not be affected by this error, we should // still log it so it can be fixed. queueHydrationError(createCapturedValueAtFiber(value, sourceFiber)); - return; + return null; } } else { // Otherwise, fall through to the error path. @@ -540,7 +540,7 @@ function throwException( workInProgress.lanes = mergeLanes(workInProgress.lanes, lane); const update = createRootErrorUpdate(workInProgress, errorInfo, lane); enqueueCapturedUpdate(workInProgress, update); - return; + return null; } case ClassComponent: // Capture and retry @@ -564,7 +564,7 @@ function throwException( lane, ); enqueueCapturedUpdate(workInProgress, update); - return; + return null; } break; default: @@ -572,6 +572,7 @@ function throwException( } workInProgress = workInProgress.return; } while (workInProgress !== null); + return null; } export {throwException, createRootErrorUpdate, createClassErrorUpdate}; diff --git a/packages/react-reconciler/src/ReactFiberThrow.old.js b/packages/react-reconciler/src/ReactFiberThrow.old.js index d34c32770e881..d6c5255807f32 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.old.js +++ b/packages/react-reconciler/src/ReactFiberThrow.old.js @@ -357,7 +357,7 @@ function throwException( sourceFiber: Fiber, value: mixed, rootRenderLanes: Lanes, -) { +): Wakeable | null { // The source fiber did not complete. sourceFiber.flags |= Incomplete; @@ -459,7 +459,7 @@ function throwException( if (suspenseBoundary.mode & ConcurrentMode) { attachPingListener(root, wakeable, rootRenderLanes); } - return; + return wakeable; } else { // No boundary was found. Unless this is a sync update, this is OK. // We can suspend and wait for more data to arrive. @@ -474,7 +474,7 @@ function throwException( // This case also applies to initial hydration. attachPingListener(root, wakeable, rootRenderLanes); renderDidSuspendDelayIfPossible(); - return; + return wakeable; } // This is a sync/discrete update. We treat this case like an error @@ -517,7 +517,7 @@ function throwException( // Even though the user may not be affected by this error, we should // still log it so it can be fixed. queueHydrationError(createCapturedValueAtFiber(value, sourceFiber)); - return; + return null; } } else { // Otherwise, fall through to the error path. @@ -540,7 +540,7 @@ function throwException( workInProgress.lanes = mergeLanes(workInProgress.lanes, lane); const update = createRootErrorUpdate(workInProgress, errorInfo, lane); enqueueCapturedUpdate(workInProgress, update); - return; + return null; } case ClassComponent: // Capture and retry @@ -564,7 +564,7 @@ function throwException( lane, ); enqueueCapturedUpdate(workInProgress, update); - return; + return null; } break; default: @@ -572,6 +572,7 @@ function throwException( } workInProgress = workInProgress.return; } while (workInProgress !== null); + return null; } export {throwException, createRootErrorUpdate, createClassErrorUpdate}; diff --git a/packages/react-reconciler/src/ReactFiberWakeable.new.js b/packages/react-reconciler/src/ReactFiberWakeable.new.js new file mode 100644 index 0000000000000..589d61eae814a --- /dev/null +++ b/packages/react-reconciler/src/ReactFiberWakeable.new.js @@ -0,0 +1,50 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +import type {Wakeable} from 'shared/ReactTypes'; + +let suspendedWakeable: Wakeable | null = null; +let wasPinged = false; +let adHocSuspendCount: number = 0; + +const MAX_AD_HOC_SUSPEND_COUNT = 50; + +export function suspendedWakeableWasPinged() { + return wasPinged; +} + +export function trackSuspendedWakeable(wakeable: Wakeable) { + adHocSuspendCount++; + suspendedWakeable = wakeable; +} + +export function attemptToPingSuspendedWakeable(wakeable: Wakeable) { + if (wakeable === suspendedWakeable) { + // This ping is from the wakeable that just suspended. Mark it as pinged. + // When the work loop resumes, we'll immediately try rendering the fiber + // again instead of unwinding the stack. + wasPinged = true; + return true; + } + return false; +} + +export function resetWakeableState() { + suspendedWakeable = null; + wasPinged = false; + adHocSuspendCount = 0; +} + +export function throwIfInfinitePingLoopDetected() { + if (adHocSuspendCount > MAX_AD_HOC_SUSPEND_COUNT) { + // TODO: Guard against an infinite loop by throwing an error if the same + // component suspends too many times in a row. This should be thrown from + // the render phase so that it gets the component stack. + } +} diff --git a/packages/react-reconciler/src/ReactFiberWakeable.old.js b/packages/react-reconciler/src/ReactFiberWakeable.old.js new file mode 100644 index 0000000000000..589d61eae814a --- /dev/null +++ b/packages/react-reconciler/src/ReactFiberWakeable.old.js @@ -0,0 +1,50 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +import type {Wakeable} from 'shared/ReactTypes'; + +let suspendedWakeable: Wakeable | null = null; +let wasPinged = false; +let adHocSuspendCount: number = 0; + +const MAX_AD_HOC_SUSPEND_COUNT = 50; + +export function suspendedWakeableWasPinged() { + return wasPinged; +} + +export function trackSuspendedWakeable(wakeable: Wakeable) { + adHocSuspendCount++; + suspendedWakeable = wakeable; +} + +export function attemptToPingSuspendedWakeable(wakeable: Wakeable) { + if (wakeable === suspendedWakeable) { + // This ping is from the wakeable that just suspended. Mark it as pinged. + // When the work loop resumes, we'll immediately try rendering the fiber + // again instead of unwinding the stack. + wasPinged = true; + return true; + } + return false; +} + +export function resetWakeableState() { + suspendedWakeable = null; + wasPinged = false; + adHocSuspendCount = 0; +} + +export function throwIfInfinitePingLoopDetected() { + if (adHocSuspendCount > MAX_AD_HOC_SUSPEND_COUNT) { + // TODO: Guard against an infinite loop by throwing an error if the same + // component suspends too many times in a row. This should be thrown from + // the render phase so that it gets the component stack. + } +} diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 4e965bb3f42d4..a8844d2c1a411 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -86,6 +86,7 @@ import { import { createWorkInProgress, assignFiberPropertiesInDEV, + resetWorkInProgress, } from './ReactFiber.new'; import {isRootDehydrated} from './ReactFiberShellHydration'; import {didSuspendOrErrorWhileHydratingDEV} from './ReactFiberHydrationContext.new'; @@ -245,6 +246,12 @@ import { isConcurrentActEnvironment, } from './ReactFiberAct.new'; import {processTransitionCallbacks} from './ReactFiberTracingMarkerComponent.new'; +import { + resetWakeableState, + trackSuspendedWakeable, + suspendedWakeableWasPinged, + attemptToPingSuspendedWakeable, +} from './ReactFiberWakeable.new'; const ceil = Math.ceil; @@ -280,6 +287,12 @@ let workInProgress: Fiber | null = null; // The lanes we're rendering let workInProgressRootRenderLanes: Lanes = NoLanes; +// When this is true, the work-in-progress fiber just suspended (or errored) and +// we've yet to unwind the stack. In some cases, we may yield to the main thread +// after this happens. If the fiber is pinged before we resume, we can retry +// immediately instead of unwinding the stack. +let workInProgressIsSuspended: boolean = false; + // A contextual version of workInProgressRootRenderLanes. It is a superset of // the lanes that we started working on at the root. When we enter a subtree // that is currently hidden, we add the lanes that would have committed if @@ -1543,11 +1556,13 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber { ); interruptedWork = interruptedWork.return; } + resetWakeableState(); } workInProgressRoot = root; const rootWorkInProgress = createWorkInProgress(root.current, null); workInProgress = rootWorkInProgress; workInProgressRootRenderLanes = renderLanes = lanes; + workInProgressIsSuspended = false; workInProgressRootExitStatus = RootInProgress; workInProgressRootFatalError = null; workInProgressRootSkippedLanes = NoLanes; @@ -1566,7 +1581,7 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber { return rootWorkInProgress; } -function handleError(root, thrownValue): void { +function handleError(root, thrownValue): Wakeable | null { do { let erroredWork = workInProgress; try { @@ -1592,7 +1607,7 @@ function handleError(root, thrownValue): void { // intentionally not calling those, we need set it here. // TODO: Consider calling `unwindWork` to pop the contexts. workInProgress = null; - return; + return null; } if (enableProfilerTimer && erroredWork.mode & ProfileMode) { @@ -1625,14 +1640,21 @@ function handleError(root, thrownValue): void { } } - throwException( + const maybeWakeable = throwException( root, erroredWork.return, erroredWork, thrownValue, workInProgressRootRenderLanes, ); - completeUnitOfWork(erroredWork); + // Setting this to `true` tells the work loop to unwind the stack instead + // of entering the begin phase. It's called "suspended" because it usually + // happens because of Suspense, but it also applies to errors. Think of it + // as suspending the execution of the work loop. + workInProgressIsSuspended = true; + + // Return to the normal work loop. + return maybeWakeable; } catch (yetAnotherThrownValue) { // Something in the return path also threw. thrownValue = yetAnotherThrownValue; @@ -1646,8 +1668,6 @@ function handleError(root, thrownValue): void { } continue; } - // Return to the normal work loop. - return; } while (true); } @@ -1810,7 +1830,14 @@ function renderRootSync(root: FiberRoot, lanes: Lanes) { // The work loop is an extremely hot path. Tell Closure not to inline it. /** @noinline */ function workLoopSync() { - // Already timed out, so perform work without checking if we need to yield. + // Perform work without checking if we need to yield between fiber. + + if (workInProgressIsSuspended && workInProgress !== null) { + // The current work-in-progress was already attempted. We need to unwind + // it before we continue the normal work loop. + resumeSuspendedUnitOfWork(workInProgress); + } + while (workInProgress !== null) { performUnitOfWork(workInProgress); } @@ -1860,7 +1887,14 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { workLoopConcurrent(); break; } catch (thrownValue) { - handleError(root, thrownValue); + const maybeWakeable = handleError(root, thrownValue); + if (maybeWakeable !== null) { + // If this fiber just suspended, it's possible the data is already + // cached. Yield to the the main thread to give it a chance to ping. If + // it does, we can retry immediately without unwinding the stack. + trackSuspendedWakeable(maybeWakeable); + break; + } } } while (true); resetContextDependencies(); @@ -1899,6 +1933,13 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { /** @noinline */ function workLoopConcurrent() { // Perform work until Scheduler asks us to yield + + if (workInProgressIsSuspended && workInProgress !== null) { + // The current work-in-progress was already attempted. We need to unwind + // it before we continue the normal work loop. + resumeSuspendedUnitOfWork(workInProgress); + } + while (workInProgress !== null && !shouldYield()) { performUnitOfWork(workInProgress); } @@ -1932,6 +1973,56 @@ function performUnitOfWork(unitOfWork: Fiber): void { ReactCurrentOwner.current = null; } +function resumeSuspendedUnitOfWork(unitOfWork: Fiber): void { + // This is a fork of performUnitOfWork specifcally for resuming a fiber that + // just suspended. In some cases, we may choose to retry the fiber immediately + // instead of unwinding the stack. It's a separate function to keep the + // additional logic out of the work loop's hot path. + + if (!suspendedWakeableWasPinged()) { + // The wakeable wasn't pinged. Return to the normal work loop. This will + // unwind the stack, and potentially result in showing a fallback. + workInProgressIsSuspended = false; + resetWakeableState(); + completeUnitOfWork(unitOfWork); + return; + } + + // The work-in-progress was immediately pinged. Instead of unwinding the + // stack and potentially showing a fallback, reset the fiber and try rendering + // it again. + unitOfWork = workInProgress = resetWorkInProgress(unitOfWork, renderLanes); + + const current = unitOfWork.alternate; + setCurrentDebugFiberInDEV(unitOfWork); + + let next; + if (enableProfilerTimer && (unitOfWork.mode & ProfileMode) !== NoMode) { + startProfilerTimer(unitOfWork); + next = beginWork(current, unitOfWork, renderLanes); + stopProfilerTimerIfRunningAndRecordDelta(unitOfWork, true); + } else { + next = beginWork(current, unitOfWork, renderLanes); + } + + // The begin phase finished successfully without suspending. Reset the state + // used to track the fiber while it was suspended. Then return to the normal + // work loop. + workInProgressIsSuspended = false; + resetWakeableState(); + + resetCurrentDebugFiberInDEV(); + unitOfWork.memoizedProps = unitOfWork.pendingProps; + if (next === null) { + // If this doesn't spawn new work, complete the current work. + completeUnitOfWork(unitOfWork); + } else { + workInProgress = next; + } + + ReactCurrentOwner.current = null; +} + function completeUnitOfWork(unitOfWork: Fiber): void { // Attempt to complete the current unit of work, then move to the next // sibling. If there are no more siblings, return to the parent fiber. @@ -2743,27 +2834,31 @@ export function pingSuspendedRoot( // Received a ping at the same priority level at which we're currently // rendering. We might want to restart this render. This should mirror // the logic of whether or not a root suspends once it completes. - - // TODO: If we're rendering sync either due to Sync, Batched or expired, - // we should probably never restart. - - // If we're suspended with delay, or if it's a retry, we'll always suspend - // so we can always restart. - if ( - workInProgressRootExitStatus === RootSuspendedWithDelay || - (workInProgressRootExitStatus === RootSuspended && - includesOnlyRetries(workInProgressRootRenderLanes) && - now() - globalMostRecentFallbackTime < FALLBACK_THROTTLE_MS) - ) { - // Restart from the root. - prepareFreshStack(root, NoLanes); + const didPingSuspendedWakeable = attemptToPingSuspendedWakeable(wakeable); + if (didPingSuspendedWakeable) { + // Successfully pinged the in-progress fiber. Don't unwind the stack. } else { - // Even though we can't restart right now, we might get an - // opportunity later. So we mark this render as having a ping. - workInProgressRootPingedLanes = mergeLanes( - workInProgressRootPingedLanes, - pingedLanes, - ); + // TODO: If we're rendering sync either due to Sync, Batched or expired, + // we should probably never restart. + + // If we're suspended with delay, or if it's a retry, we'll always suspend + // so we can always restart. + if ( + workInProgressRootExitStatus === RootSuspendedWithDelay || + (workInProgressRootExitStatus === RootSuspended && + includesOnlyRetries(workInProgressRootRenderLanes) && + now() - globalMostRecentFallbackTime < FALLBACK_THROTTLE_MS) + ) { + // Restart from the root. + prepareFreshStack(root, NoLanes); + } else { + // Even though we can't restart right now, we might get an + // opportunity later. So we mark this render as having a ping. + workInProgressRootPingedLanes = mergeLanes( + workInProgressRootPingedLanes, + pingedLanes, + ); + } } } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index 4b2e523d40a21..14e378794bfbc 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -86,6 +86,7 @@ import { import { createWorkInProgress, assignFiberPropertiesInDEV, + resetWorkInProgress, } from './ReactFiber.old'; import {isRootDehydrated} from './ReactFiberShellHydration'; import {didSuspendOrErrorWhileHydratingDEV} from './ReactFiberHydrationContext.old'; @@ -245,6 +246,12 @@ import { isConcurrentActEnvironment, } from './ReactFiberAct.old'; import {processTransitionCallbacks} from './ReactFiberTracingMarkerComponent.old'; +import { + resetWakeableState, + trackSuspendedWakeable, + suspendedWakeableWasPinged, + attemptToPingSuspendedWakeable, +} from './ReactFiberWakeable.old'; const ceil = Math.ceil; @@ -280,6 +287,12 @@ let workInProgress: Fiber | null = null; // The lanes we're rendering let workInProgressRootRenderLanes: Lanes = NoLanes; +// When this is true, the work-in-progress fiber just suspended (or errored) and +// we've yet to unwind the stack. In some cases, we may yield to the main thread +// after this happens. If the fiber is pinged before we resume, we can retry +// immediately instead of unwinding the stack. +let workInProgressIsSuspended: boolean = false; + // A contextual version of workInProgressRootRenderLanes. It is a superset of // the lanes that we started working on at the root. When we enter a subtree // that is currently hidden, we add the lanes that would have committed if @@ -1543,11 +1556,13 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber { ); interruptedWork = interruptedWork.return; } + resetWakeableState(); } workInProgressRoot = root; const rootWorkInProgress = createWorkInProgress(root.current, null); workInProgress = rootWorkInProgress; workInProgressRootRenderLanes = renderLanes = lanes; + workInProgressIsSuspended = false; workInProgressRootExitStatus = RootInProgress; workInProgressRootFatalError = null; workInProgressRootSkippedLanes = NoLanes; @@ -1566,7 +1581,7 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber { return rootWorkInProgress; } -function handleError(root, thrownValue): void { +function handleError(root, thrownValue): Wakeable | null { do { let erroredWork = workInProgress; try { @@ -1592,7 +1607,7 @@ function handleError(root, thrownValue): void { // intentionally not calling those, we need set it here. // TODO: Consider calling `unwindWork` to pop the contexts. workInProgress = null; - return; + return null; } if (enableProfilerTimer && erroredWork.mode & ProfileMode) { @@ -1625,14 +1640,21 @@ function handleError(root, thrownValue): void { } } - throwException( + const maybeWakeable = throwException( root, erroredWork.return, erroredWork, thrownValue, workInProgressRootRenderLanes, ); - completeUnitOfWork(erroredWork); + // Setting this to `true` tells the work loop to unwind the stack instead + // of entering the begin phase. It's called "suspended" because it usually + // happens because of Suspense, but it also applies to errors. Think of it + // as suspending the execution of the work loop. + workInProgressIsSuspended = true; + + // Return to the normal work loop. + return maybeWakeable; } catch (yetAnotherThrownValue) { // Something in the return path also threw. thrownValue = yetAnotherThrownValue; @@ -1646,8 +1668,6 @@ function handleError(root, thrownValue): void { } continue; } - // Return to the normal work loop. - return; } while (true); } @@ -1810,7 +1830,14 @@ function renderRootSync(root: FiberRoot, lanes: Lanes) { // The work loop is an extremely hot path. Tell Closure not to inline it. /** @noinline */ function workLoopSync() { - // Already timed out, so perform work without checking if we need to yield. + // Perform work without checking if we need to yield between fiber. + + if (workInProgressIsSuspended && workInProgress !== null) { + // The current work-in-progress was already attempted. We need to unwind + // it before we continue the normal work loop. + resumeSuspendedUnitOfWork(workInProgress); + } + while (workInProgress !== null) { performUnitOfWork(workInProgress); } @@ -1860,7 +1887,14 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { workLoopConcurrent(); break; } catch (thrownValue) { - handleError(root, thrownValue); + const maybeWakeable = handleError(root, thrownValue); + if (maybeWakeable !== null) { + // If this fiber just suspended, it's possible the data is already + // cached. Yield to the the main thread to give it a chance to ping. If + // it does, we can retry immediately without unwinding the stack. + trackSuspendedWakeable(maybeWakeable); + break; + } } } while (true); resetContextDependencies(); @@ -1899,6 +1933,13 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { /** @noinline */ function workLoopConcurrent() { // Perform work until Scheduler asks us to yield + + if (workInProgressIsSuspended && workInProgress !== null) { + // The current work-in-progress was already attempted. We need to unwind + // it before we continue the normal work loop. + resumeSuspendedUnitOfWork(workInProgress); + } + while (workInProgress !== null && !shouldYield()) { performUnitOfWork(workInProgress); } @@ -1932,6 +1973,56 @@ function performUnitOfWork(unitOfWork: Fiber): void { ReactCurrentOwner.current = null; } +function resumeSuspendedUnitOfWork(unitOfWork: Fiber): void { + // This is a fork of performUnitOfWork specifcally for resuming a fiber that + // just suspended. In some cases, we may choose to retry the fiber immediately + // instead of unwinding the stack. It's a separate function to keep the + // additional logic out of the work loop's hot path. + + if (!suspendedWakeableWasPinged()) { + // The wakeable wasn't pinged. Return to the normal work loop. This will + // unwind the stack, and potentially result in showing a fallback. + workInProgressIsSuspended = false; + resetWakeableState(); + completeUnitOfWork(unitOfWork); + return; + } + + // The work-in-progress was immediately pinged. Instead of unwinding the + // stack and potentially showing a fallback, reset the fiber and try rendering + // it again. + unitOfWork = workInProgress = resetWorkInProgress(unitOfWork, renderLanes); + + const current = unitOfWork.alternate; + setCurrentDebugFiberInDEV(unitOfWork); + + let next; + if (enableProfilerTimer && (unitOfWork.mode & ProfileMode) !== NoMode) { + startProfilerTimer(unitOfWork); + next = beginWork(current, unitOfWork, renderLanes); + stopProfilerTimerIfRunningAndRecordDelta(unitOfWork, true); + } else { + next = beginWork(current, unitOfWork, renderLanes); + } + + // The begin phase finished successfully without suspending. Reset the state + // used to track the fiber while it was suspended. Then return to the normal + // work loop. + workInProgressIsSuspended = false; + resetWakeableState(); + + resetCurrentDebugFiberInDEV(); + unitOfWork.memoizedProps = unitOfWork.pendingProps; + if (next === null) { + // If this doesn't spawn new work, complete the current work. + completeUnitOfWork(unitOfWork); + } else { + workInProgress = next; + } + + ReactCurrentOwner.current = null; +} + function completeUnitOfWork(unitOfWork: Fiber): void { // Attempt to complete the current unit of work, then move to the next // sibling. If there are no more siblings, return to the parent fiber. @@ -2743,27 +2834,31 @@ export function pingSuspendedRoot( // Received a ping at the same priority level at which we're currently // rendering. We might want to restart this render. This should mirror // the logic of whether or not a root suspends once it completes. - - // TODO: If we're rendering sync either due to Sync, Batched or expired, - // we should probably never restart. - - // If we're suspended with delay, or if it's a retry, we'll always suspend - // so we can always restart. - if ( - workInProgressRootExitStatus === RootSuspendedWithDelay || - (workInProgressRootExitStatus === RootSuspended && - includesOnlyRetries(workInProgressRootRenderLanes) && - now() - globalMostRecentFallbackTime < FALLBACK_THROTTLE_MS) - ) { - // Restart from the root. - prepareFreshStack(root, NoLanes); + const didPingSuspendedWakeable = attemptToPingSuspendedWakeable(wakeable); + if (didPingSuspendedWakeable) { + // Successfully pinged the in-progress fiber. Don't unwind the stack. } else { - // Even though we can't restart right now, we might get an - // opportunity later. So we mark this render as having a ping. - workInProgressRootPingedLanes = mergeLanes( - workInProgressRootPingedLanes, - pingedLanes, - ); + // TODO: If we're rendering sync either due to Sync, Batched or expired, + // we should probably never restart. + + // If we're suspended with delay, or if it's a retry, we'll always suspend + // so we can always restart. + if ( + workInProgressRootExitStatus === RootSuspendedWithDelay || + (workInProgressRootExitStatus === RootSuspended && + includesOnlyRetries(workInProgressRootRenderLanes) && + now() - globalMostRecentFallbackTime < FALLBACK_THROTTLE_MS) + ) { + // Restart from the root. + prepareFreshStack(root, NoLanes); + } else { + // Even though we can't restart right now, we might get an + // opportunity later. So we mark this render as having a ping. + workInProgressRootPingedLanes = mergeLanes( + workInProgressRootPingedLanes, + pingedLanes, + ); + } } } diff --git a/packages/react-reconciler/src/__tests__/ReactOffscreenSuspense-test.js b/packages/react-reconciler/src/__tests__/ReactOffscreenSuspense-test.js index 290dd33b89a09..3c3a79a8efd74 100644 --- a/packages/react-reconciler/src/__tests__/ReactOffscreenSuspense-test.js +++ b/packages/react-reconciler/src/__tests__/ReactOffscreenSuspense-test.js @@ -485,22 +485,33 @@ describe('ReactOffscreen', () => { // In the same render, also hide the offscreen tree. root.render(); - expect(Scheduler).toFlushUntilNextPaint([ - // The outer update will commit, but the inner update is deferred until - // a later render. - 'Outer: 1', - - // Something suspended. This means we won't commit immediately; there - // will be an async gap between render and commit. In this test, we will - // use this property to schedule a concurrent update. The fact that - // we're using Suspense to schedule a concurrent update is not directly - // relevant to the test — we could also use time slicing, but I've - // chosen to use Suspense the because implementation details of time - // slicing are more volatile. - 'Suspend! [Async: 1]', + if (gate(flags => flags.enableSyncDefaultUpdates)) { + expect(Scheduler).toFlushUntilNextPaint([ + // The outer update will commit, but the inner update is deferred until + // a later render. + 'Outer: 1', + + // Something suspended. This means we won't commit immediately; there + // will be an async gap between render and commit. In this test, we will + // use this property to schedule a concurrent update. The fact that + // we're using Suspense to schedule a concurrent update is not directly + // relevant to the test — we could also use time slicing, but I've + // chosen to use Suspense the because implementation details of time + // slicing are more volatile. + 'Suspend! [Async: 1]', + + 'Loading...', + ]); + } else { + // When default updates are time sliced, React yields before preparing + // the fallback. + expect(Scheduler).toFlushUntilNextPaint([ + 'Outer: 1', + 'Suspend! [Async: 1]', + ]); + expect(Scheduler).toFlushUntilNextPaint(['Loading...']); + } - 'Loading...', - ]); // Assert that we haven't committed quite yet expect(root).toMatchRenderedOutput( <> diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js index 3f2f4cc38aa09..07838b223e687 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js @@ -963,33 +963,36 @@ describe('ReactSuspenseWithNoopRenderer', () => { // @gate enableCache it('resolves successfully even if fallback render is pending', async () => { - ReactNoop.render( + const root = ReactNoop.createRoot(); + root.render( <> } /> , ); expect(Scheduler).toFlushAndYield([]); - expect(ReactNoop.getChildren()).toEqual([]); + expect(root).toMatchRenderedOutput(null); if (gate(flags => flags.enableSyncDefaultUpdates)) { React.startTransition(() => { - ReactNoop.render( + root.render( <> }> + , ); }); } else { - ReactNoop.render( + root.render( <> }> + , ); } - expect(ReactNoop.flushNextYield()).toEqual(['Suspend! [Async]']); + expect(Scheduler).toFlushAndYieldThrough(['Suspend! [Async]', 'Sibling']); await resolveText('Async'); expect(Scheduler).toFlushAndYield([ @@ -998,8 +1001,14 @@ describe('ReactSuspenseWithNoopRenderer', () => { 'Loading...', // Once we've completed the boundary we restarted. 'Async', + 'Sibling', ]); - expect(ReactNoop.getChildren()).toEqual([span('Async')]); + expect(root).toMatchRenderedOutput( + <> + + + , + ); }); // @gate enableCache @@ -3859,7 +3868,6 @@ describe('ReactSuspenseWithNoopRenderer', () => { 'Suspend! [A2]', 'Loading...', 'Suspend! [B2]', - 'Loading...', ]); expect(root).toMatchRenderedOutput( <> diff --git a/packages/react-reconciler/src/__tests__/ReactWakeable-test.js b/packages/react-reconciler/src/__tests__/ReactWakeable-test.js new file mode 100644 index 0000000000000..848962c696c0b --- /dev/null +++ b/packages/react-reconciler/src/__tests__/ReactWakeable-test.js @@ -0,0 +1,67 @@ +'use strict'; + +let React; +let ReactNoop; +let Scheduler; +let act; +let Suspense; +let startTransition; + +describe('ReactWakeable', () => { + beforeEach(() => { + jest.resetModules(); + + React = require('react'); + ReactNoop = require('react-noop-renderer'); + Scheduler = require('scheduler'); + act = require('jest-react').act; + Suspense = React.Suspense; + startTransition = React.startTransition; + }); + + function Text(props) { + Scheduler.unstable_yieldValue(props.text); + return props.text; + } + + test('if suspended fiber is pinged in a microtask, retry immediately without unwinding the stack', async () => { + let resolved = false; + function Async() { + if (resolved) { + return ; + } + Scheduler.unstable_yieldValue('Suspend!'); + throw Promise.resolve().then(() => { + Scheduler.unstable_yieldValue('Resolve in microtask'); + resolved = true; + }); + } + + function App() { + return ( + }> + + + ); + } + + await act(async () => { + startTransition(() => { + ReactNoop.render(); + }); + + // React will yield when the async component suspends. + expect(Scheduler).toFlushUntilNextPaint(['Suspend!']); + + // Wait for microtasks to resolve + // TODO: The async form of `act` should automatically yield to microtasks + // when a continuation is returned, the way Scheduler does. + await null; + + expect(Scheduler).toHaveYielded(['Resolve in microtask']); + }); + + // Finished rendering without unwinding the stack. + expect(Scheduler).toHaveYielded(['Async']); + }); +}); diff --git a/packages/scheduler/npm/umd/scheduler.development.js b/packages/scheduler/npm/umd/scheduler.development.js index 21316812d1454..b960dc91132e7 100644 --- a/packages/scheduler/npm/umd/scheduler.development.js +++ b/packages/scheduler/npm/umd/scheduler.development.js @@ -54,13 +54,6 @@ ); } - function unstable_requestYield() { - return global.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Scheduler.unstable_requestYield.apply( - this, - arguments - ); - } - function unstable_runWithPriority() { return global.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Scheduler.unstable_runWithPriority.apply( this, @@ -123,7 +116,6 @@ unstable_cancelCallback: unstable_cancelCallback, unstable_shouldYield: unstable_shouldYield, unstable_requestPaint: unstable_requestPaint, - unstable_requestYield: unstable_requestYield, unstable_runWithPriority: unstable_runWithPriority, unstable_next: unstable_next, unstable_wrapCallback: unstable_wrapCallback, diff --git a/packages/scheduler/npm/umd/scheduler.production.min.js b/packages/scheduler/npm/umd/scheduler.production.min.js index 41c76570e1ab5..0c2584331b847 100644 --- a/packages/scheduler/npm/umd/scheduler.production.min.js +++ b/packages/scheduler/npm/umd/scheduler.production.min.js @@ -54,13 +54,6 @@ ); } - function unstable_requestYield() { - return global.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Scheduler.unstable_requestYield.apply( - this, - arguments - ); - } - function unstable_runWithPriority() { return global.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Scheduler.unstable_runWithPriority.apply( this, @@ -117,7 +110,6 @@ unstable_cancelCallback: unstable_cancelCallback, unstable_shouldYield: unstable_shouldYield, unstable_requestPaint: unstable_requestPaint, - unstable_requestYield: unstable_requestYield, unstable_runWithPriority: unstable_runWithPriority, unstable_next: unstable_next, unstable_wrapCallback: unstable_wrapCallback, diff --git a/packages/scheduler/npm/umd/scheduler.profiling.min.js b/packages/scheduler/npm/umd/scheduler.profiling.min.js index 41c76570e1ab5..0c2584331b847 100644 --- a/packages/scheduler/npm/umd/scheduler.profiling.min.js +++ b/packages/scheduler/npm/umd/scheduler.profiling.min.js @@ -54,13 +54,6 @@ ); } - function unstable_requestYield() { - return global.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Scheduler.unstable_requestYield.apply( - this, - arguments - ); - } - function unstable_runWithPriority() { return global.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Scheduler.unstable_runWithPriority.apply( this, @@ -117,7 +110,6 @@ unstable_cancelCallback: unstable_cancelCallback, unstable_shouldYield: unstable_shouldYield, unstable_requestPaint: unstable_requestPaint, - unstable_requestYield: unstable_requestYield, unstable_runWithPriority: unstable_runWithPriority, unstable_next: unstable_next, unstable_wrapCallback: unstable_wrapCallback, diff --git a/packages/scheduler/src/__tests__/Scheduler-test.js b/packages/scheduler/src/__tests__/Scheduler-test.js index 82cd773629a25..925c64202bb4a 100644 --- a/packages/scheduler/src/__tests__/Scheduler-test.js +++ b/packages/scheduler/src/__tests__/Scheduler-test.js @@ -18,7 +18,6 @@ let performance; let cancelCallback; let scheduleCallback; let requestPaint; -let requestYield; let shouldYield; let NormalPriority; @@ -44,7 +43,6 @@ describe('SchedulerBrowser', () => { scheduleCallback = Scheduler.unstable_scheduleCallback; NormalPriority = Scheduler.unstable_NormalPriority; requestPaint = Scheduler.unstable_requestPaint; - requestYield = Scheduler.unstable_requestYield; shouldYield = Scheduler.unstable_shouldYield; }); @@ -480,19 +478,13 @@ describe('SchedulerBrowser', () => { ]); }); - it('requestYield forces a yield immediately', () => { + it('yielding continues in a new task regardless of how much time is remaining', () => { scheduleCallback(NormalPriority, () => { runtime.log('Original Task'); runtime.log('shouldYield: ' + shouldYield()); - runtime.log('requestYield'); - requestYield(); - runtime.log('shouldYield: ' + shouldYield()); + runtime.log('Return a continuation'); return () => { runtime.log('Continuation Task'); - runtime.log('shouldYield: ' + shouldYield()); - runtime.log('Advance time past frame deadline'); - runtime.advanceTime(10000); - runtime.log('shouldYield: ' + shouldYield()); }; }); runtime.assertLog(['Post Message']); @@ -501,27 +493,20 @@ describe('SchedulerBrowser', () => { runtime.assertLog([ 'Message Event', 'Original Task', + // Immediately before returning a continuation, `shouldYield` returns + // false, which means there must be time remaining in the frame. 'shouldYield: false', - 'requestYield', - // Immediately after calling requestYield, shouldYield starts - // returning true, even though no time has elapsed in the frame - 'shouldYield: true', + 'Return a continuation', - // The continuation should be scheduled in a separate macrotask. + // The continuation should be scheduled in a separate macrotask even + // though there's time remaining. 'Post Message', ]); // No time has elapsed expect(performance.now()).toBe(0); - // Subsequent tasks work as normal runtime.fireMessageEvent(); - runtime.assertLog([ - 'Message Event', - 'Continuation Task', - 'shouldYield: false', - 'Advance time past frame deadline', - 'shouldYield: true', - ]); + runtime.assertLog(['Message Event', 'Continuation Task']); }); }); diff --git a/packages/scheduler/src/__tests__/SchedulerMock-test.js b/packages/scheduler/src/__tests__/SchedulerMock-test.js index 8b01ec3c0e319..aa17e88ececda 100644 --- a/packages/scheduler/src/__tests__/SchedulerMock-test.js +++ b/packages/scheduler/src/__tests__/SchedulerMock-test.js @@ -726,37 +726,53 @@ describe('Scheduler', () => { expect(Scheduler).toFlushWithoutYielding(); }); - it('requestYield forces a yield immediately', () => { + it('toFlushUntilNextPaint stops if a continuation is returned', () => { scheduleCallback(NormalPriority, () => { Scheduler.unstable_yieldValue('Original Task'); - Scheduler.unstable_yieldValue( - 'shouldYield: ' + Scheduler.unstable_shouldYield(), - ); - Scheduler.unstable_yieldValue('requestYield'); - Scheduler.unstable_requestYield(); - Scheduler.unstable_yieldValue( - 'shouldYield: ' + Scheduler.unstable_shouldYield(), - ); + Scheduler.unstable_yieldValue('shouldYield: ' + shouldYield()); + Scheduler.unstable_yieldValue('Return a continuation'); return () => { Scheduler.unstable_yieldValue('Continuation Task'); - Scheduler.unstable_yieldValue( - 'shouldYield: ' + Scheduler.unstable_shouldYield(), - ); - Scheduler.unstable_yieldValue('Advance time past frame deadline'); - Scheduler.unstable_yieldValue( - 'shouldYield: ' + Scheduler.unstable_shouldYield(), - ); }; }); - // The continuation should be scheduled in a separate macrotask. expect(Scheduler).toFlushUntilNextPaint([ 'Original Task', + // Immediately before returning a continuation, `shouldYield` returns + // false, which means there must be time remaining in the frame. 'shouldYield: false', - 'requestYield', - // Immediately after calling requestYield, shouldYield starts - // returning true - 'shouldYield: true', + 'Return a continuation', + + // The continuation should not flush yet. + ]); + + // No time has elapsed + expect(Scheduler.unstable_now()).toBe(0); + + // Continue the task + expect(Scheduler).toFlushAndYield(['Continuation Task']); + }); + + it("toFlushAndYield keeps flushing even if there's a continuation", () => { + scheduleCallback(NormalPriority, () => { + Scheduler.unstable_yieldValue('Original Task'); + Scheduler.unstable_yieldValue('shouldYield: ' + shouldYield()); + Scheduler.unstable_yieldValue('Return a continuation'); + return () => { + Scheduler.unstable_yieldValue('Continuation Task'); + }; + }); + + expect(Scheduler).toFlushAndYield([ + 'Original Task', + // Immediately before returning a continuation, `shouldYield` returns + // false, which means there must be time remaining in the frame. + 'shouldYield: false', + 'Return a continuation', + + // The continuation should flush immediately, even though the task + // yielded a continuation. + 'Continuation Task', ]); }); }); diff --git a/packages/scheduler/src/__tests__/SchedulerPostTask-test.js b/packages/scheduler/src/__tests__/SchedulerPostTask-test.js index 2dbb96dd8993b..cc1fc8b63f674 100644 --- a/packages/scheduler/src/__tests__/SchedulerPostTask-test.js +++ b/packages/scheduler/src/__tests__/SchedulerPostTask-test.js @@ -23,7 +23,6 @@ let UserBlockingPriority; let LowPriority; let IdlePriority; let shouldYield; -let requestYield; // The Scheduler postTask implementation uses a new postTask browser API to // schedule work on the main thread. This test suite mocks all browser methods @@ -47,7 +46,6 @@ describe('SchedulerPostTask', () => { LowPriority = Scheduler.unstable_LowPriority; IdlePriority = Scheduler.unstable_IdlePriority; shouldYield = Scheduler.unstable_shouldYield; - requestYield = Scheduler.unstable_requestYield; }); afterEach(() => { @@ -301,19 +299,13 @@ describe('SchedulerPostTask', () => { ]); }); - it('requestYield forces a yield immediately', () => { + it('yielding continues in a new task regardless of how much time is remaining', () => { scheduleCallback(NormalPriority, () => { runtime.log('Original Task'); runtime.log('shouldYield: ' + shouldYield()); - runtime.log('requestYield'); - requestYield(); - runtime.log('shouldYield: ' + shouldYield()); + runtime.log('Return a continuation'); return () => { runtime.log('Continuation Task'); - runtime.log('shouldYield: ' + shouldYield()); - runtime.log('Advance time past frame deadline'); - runtime.advanceTime(10000); - runtime.log('shouldYield: ' + shouldYield()); }; }); runtime.assertLog(['Post Task 0 [user-visible]']); @@ -322,27 +314,20 @@ describe('SchedulerPostTask', () => { runtime.assertLog([ 'Task 0 Fired', 'Original Task', + // Immediately before returning a continuation, `shouldYield` returns + // false, which means there must be time remaining in the frame. 'shouldYield: false', - 'requestYield', - // Immediately after calling requestYield, shouldYield starts - // returning true, even though no time has elapsed in the frame - 'shouldYield: true', + 'Return a continuation', - // The continuation should be scheduled in a separate macrotask. + // The continuation should be scheduled in a separate macrotask even + // though there's time remaining. 'Post Task 1 [user-visible]', ]); // No time has elapsed expect(performance.now()).toBe(0); - // Subsequent tasks work as normal runtime.flushTasks(); - runtime.assertLog([ - 'Task 1 Fired', - 'Continuation Task', - 'shouldYield: false', - 'Advance time past frame deadline', - 'shouldYield: true', - ]); + runtime.assertLog(['Task 1 Fired', 'Continuation Task']); }); }); diff --git a/packages/scheduler/src/forks/Scheduler.js b/packages/scheduler/src/forks/Scheduler.js index ea440375a35a6..3350c798fabe1 100644 --- a/packages/scheduler/src/forks/Scheduler.js +++ b/packages/scheduler/src/forks/Scheduler.js @@ -212,10 +212,14 @@ function workLoop(hasTimeRemaining, initialTime) { const continuationCallback = callback(didUserCallbackTimeout); currentTime = getCurrentTime(); if (typeof continuationCallback === 'function') { + // If a continuation is returned, immediately yield to the main thread + // regardless of how much time is left in the current time slice. currentTask.callback = continuationCallback; if (enableProfiling) { markTaskYield(currentTask, currentTime); } + advanceTimers(currentTime); + return true; } else { if (enableProfiling) { markTaskCompleted(currentTask, currentTime); @@ -224,8 +228,8 @@ function workLoop(hasTimeRemaining, initialTime) { if (currentTask === peek(taskQueue)) { pop(taskQueue); } + advanceTimers(currentTime); } - advanceTimers(currentTime); } else { pop(taskQueue); } @@ -495,11 +499,6 @@ function requestPaint() { // Since we yield every frame regardless, `requestPaint` has no effect. } -function requestYield() { - // Force a yield at the next opportunity. - startTime = -99999; -} - function forceFrameRate(fps) { if (fps < 0 || fps > 125) { // Using console['error'] to evade Babel and ESLint @@ -617,7 +616,6 @@ export { unstable_getCurrentPriorityLevel, shouldYieldToHost as unstable_shouldYield, requestPaint as unstable_requestPaint, - requestYield as unstable_requestYield, unstable_continueExecution, unstable_pauseExecution, unstable_getFirstCallbackNode, diff --git a/packages/scheduler/src/forks/SchedulerMock.js b/packages/scheduler/src/forks/SchedulerMock.js index 6898be823904b..a1374005ed05c 100644 --- a/packages/scheduler/src/forks/SchedulerMock.js +++ b/packages/scheduler/src/forks/SchedulerMock.js @@ -195,10 +195,22 @@ function workLoop(hasTimeRemaining, initialTime) { const continuationCallback = callback(didUserCallbackTimeout); currentTime = getCurrentTime(); if (typeof continuationCallback === 'function') { + // If a continuation is returned, immediately yield to the main thread + // regardless of how much time is left in the current time slice. currentTask.callback = continuationCallback; if (enableProfiling) { markTaskYield(currentTask, currentTime); } + advanceTimers(currentTime); + + if (shouldYieldForPaint) { + needsPaint = true; + return true; + } else { + // If `shouldYieldForPaint` is false, we keep flushing synchronously + // without yielding to the main thread. This is the behavior of the + // `toFlushAndYield` and `toFlushAndYieldThrough` testing helpers . + } } else { if (enableProfiling) { markTaskCompleted(currentTask, currentTime); @@ -207,8 +219,8 @@ function workLoop(hasTimeRemaining, initialTime) { if (currentTask === peek(taskQueue)) { pop(taskQueue); } + advanceTimers(currentTime); } - advanceTimers(currentTime); } else { pop(taskQueue); } @@ -608,11 +620,6 @@ function requestPaint() { needsPaint = true; } -function requestYield() { - // Force a yield at the next opportunity. - shouldYieldForPaint = needsPaint = true; -} - export { ImmediatePriority as unstable_ImmediatePriority, UserBlockingPriority as unstable_UserBlockingPriority, @@ -627,7 +634,6 @@ export { unstable_getCurrentPriorityLevel, shouldYieldToHost as unstable_shouldYield, requestPaint as unstable_requestPaint, - requestYield as unstable_requestYield, unstable_continueExecution, unstable_pauseExecution, unstable_getFirstCallbackNode, diff --git a/packages/scheduler/src/forks/SchedulerPostTask.js b/packages/scheduler/src/forks/SchedulerPostTask.js index 4e51a5873430e..c07f7f03819c3 100644 --- a/packages/scheduler/src/forks/SchedulerPostTask.js +++ b/packages/scheduler/src/forks/SchedulerPostTask.js @@ -67,11 +67,6 @@ export function unstable_requestPaint() { // Since we yield every frame regardless, `requestPaint` has no effect. } -export function unstable_requestYield() { - // Force a yield at the next opportunity. - deadline = -99999; -} - type SchedulerCallback = ( didTimeout_DEPRECATED: boolean, ) =>