From 1eaafc9ade46ba708b2361b324dd907d019e3939 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Thu, 27 Aug 2020 11:47:58 -0500 Subject: [PATCH] Clean up timeoutMs-related implementation details (#19704) * Disable busyDelayMs and busyMinDurationMs Refer to explanation in previous commit. * Remove unnecessary work loop variables Since we no longer support SuspenseConfig options, we don't need to track these values. * Remove unnecessary Update fields --- .../src/ReactFiberClassComponent.new.js | 6 +- .../src/ReactFiberClassComponent.old.js | 6 +- .../src/ReactFiberHooks.new.js | 21 ----- .../src/ReactFiberHooks.old.js | 21 ----- .../react-reconciler/src/ReactFiberLane.js | 3 + .../src/ReactFiberNewContext.new.js | 1 - .../src/ReactFiberNewContext.old.js | 1 - .../src/ReactFiberReconciler.new.js | 2 +- .../src/ReactFiberReconciler.old.js | 2 +- .../src/ReactFiberThrow.new.js | 6 +- .../src/ReactFiberThrow.old.js | 6 +- .../src/ReactFiberWorkLoop.new.js | 82 +------------------ .../src/ReactFiberWorkLoop.old.js | 82 +------------------ .../src/ReactUpdateQueue.new.js | 25 +----- .../src/ReactUpdateQueue.old.js | 25 +----- .../ReactHooksWithNoopRenderer-test.js | 79 ------------------ .../ReactSuspenseWithNoopRenderer-test.js | 68 --------------- 17 files changed, 25 insertions(+), 411 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.new.js b/packages/react-reconciler/src/ReactFiberClassComponent.new.js index 0c94bd9441c90..59914edf43398 100644 --- a/packages/react-reconciler/src/ReactFiberClassComponent.new.js +++ b/packages/react-reconciler/src/ReactFiberClassComponent.new.js @@ -199,7 +199,7 @@ const classComponentUpdater = { const suspenseConfig = requestCurrentSuspenseConfig(); const lane = requestUpdateLane(fiber, suspenseConfig); - const update = createUpdate(eventTime, lane, suspenseConfig); + const update = createUpdate(eventTime, lane); update.payload = payload; if (callback !== undefined && callback !== null) { if (__DEV__) { @@ -230,7 +230,7 @@ const classComponentUpdater = { const suspenseConfig = requestCurrentSuspenseConfig(); const lane = requestUpdateLane(fiber, suspenseConfig); - const update = createUpdate(eventTime, lane, suspenseConfig); + const update = createUpdate(eventTime, lane); update.tag = ReplaceState; update.payload = payload; @@ -263,7 +263,7 @@ const classComponentUpdater = { const suspenseConfig = requestCurrentSuspenseConfig(); const lane = requestUpdateLane(fiber, suspenseConfig); - const update = createUpdate(eventTime, lane, suspenseConfig); + const update = createUpdate(eventTime, lane); update.tag = ForceUpdate; if (callback !== undefined && callback !== null) { diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.old.js b/packages/react-reconciler/src/ReactFiberClassComponent.old.js index 68834e49a1d50..d05c494f1ba6d 100644 --- a/packages/react-reconciler/src/ReactFiberClassComponent.old.js +++ b/packages/react-reconciler/src/ReactFiberClassComponent.old.js @@ -199,7 +199,7 @@ const classComponentUpdater = { const suspenseConfig = requestCurrentSuspenseConfig(); const lane = requestUpdateLane(fiber, suspenseConfig); - const update = createUpdate(eventTime, lane, suspenseConfig); + const update = createUpdate(eventTime, lane); update.payload = payload; if (callback !== undefined && callback !== null) { if (__DEV__) { @@ -230,7 +230,7 @@ const classComponentUpdater = { const suspenseConfig = requestCurrentSuspenseConfig(); const lane = requestUpdateLane(fiber, suspenseConfig); - const update = createUpdate(eventTime, lane, suspenseConfig); + const update = createUpdate(eventTime, lane); update.tag = ReplaceState; update.payload = payload; @@ -263,7 +263,7 @@ const classComponentUpdater = { const suspenseConfig = requestCurrentSuspenseConfig(); const lane = requestUpdateLane(fiber, suspenseConfig); - const update = createUpdate(eventTime, lane, suspenseConfig); + const update = createUpdate(eventTime, lane); update.tag = ForceUpdate; if (callback !== undefined && callback !== null) { diff --git a/packages/react-reconciler/src/ReactFiberHooks.new.js b/packages/react-reconciler/src/ReactFiberHooks.new.js index 7a399fac8a9d1..2fc397f470164 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.new.js +++ b/packages/react-reconciler/src/ReactFiberHooks.new.js @@ -63,7 +63,6 @@ import { warnIfNotCurrentlyActingEffectsInDEV, warnIfNotCurrentlyActingUpdatesInDev, warnIfNotScopedWithMatchingAct, - markRenderEventTimeAndConfig, markSkippedUpdateLanes, } from './ReactFiberWorkLoop.new'; @@ -97,11 +96,7 @@ import {markStateUpdateScheduled} from './SchedulingProfiler'; const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals; type Update = {| - // TODO: Temporary field. Will remove this by storing a map of - // transition -> start time on the root. - eventTime: number, lane: Lane, - suspenseConfig: null | SuspenseConfig, action: A, eagerReducer: ((S, A) => S) | null, eagerState: S | null, @@ -715,17 +710,13 @@ function updateReducer( let newBaseQueueLast = null; let update = first; do { - const suspenseConfig = update.suspenseConfig; const updateLane = update.lane; - const updateEventTime = update.eventTime; if (!isSubsetOfLanes(renderLanes, updateLane)) { // Priority is insufficient. Skip this update. If this is the first // skipped update, the previous update/state is the new base // update/state. const clone: Update = { - eventTime: updateEventTime, lane: updateLane, - suspenseConfig: suspenseConfig, action: update.action, eagerReducer: update.eagerReducer, eagerState: update.eagerState, @@ -750,12 +741,10 @@ function updateReducer( if (newBaseQueueLast !== null) { const clone: Update = { - eventTime: updateEventTime, // This update is going to be committed so we never want uncommit // it. Using NoLane works because 0 is a subset of all bitmasks, so // this will never be skipped by the check above. lane: NoLane, - suspenseConfig: update.suspenseConfig, action: update.action, eagerReducer: update.eagerReducer, eagerState: update.eagerState, @@ -764,14 +753,6 @@ function updateReducer( newBaseQueueLast = newBaseQueueLast.next = clone; } - // Mark the event time of this update as relevant to this render pass. - // TODO: This should ideally use the true event time of this update rather than - // its priority which is a derived and not reverseable value. - // TODO: We should skip this update if it was already committed but currently - // we have no way of detecting the difference between a committed and suspended - // update here. - markRenderEventTimeAndConfig(updateEventTime, suspenseConfig); - // Process this update. if (update.eagerReducer === reducer) { // If this update was processed eagerly, and its reducer matches the @@ -1708,9 +1689,7 @@ function dispatchAction( const lane = requestUpdateLane(fiber, suspenseConfig); const update: Update = { - eventTime, lane, - suspenseConfig, action, eagerReducer: null, eagerState: null, diff --git a/packages/react-reconciler/src/ReactFiberHooks.old.js b/packages/react-reconciler/src/ReactFiberHooks.old.js index 3928337b73393..32320c2c35a94 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.old.js +++ b/packages/react-reconciler/src/ReactFiberHooks.old.js @@ -62,7 +62,6 @@ import { warnIfNotCurrentlyActingEffectsInDEV, warnIfNotCurrentlyActingUpdatesInDev, warnIfNotScopedWithMatchingAct, - markRenderEventTimeAndConfig, markSkippedUpdateLanes, } from './ReactFiberWorkLoop.old'; @@ -96,11 +95,7 @@ import {markStateUpdateScheduled} from './SchedulingProfiler'; const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals; type Update = {| - // TODO: Temporary field. Will remove this by storing a map of - // transition -> start time on the root. - eventTime: number, lane: Lane, - suspenseConfig: null | SuspenseConfig, action: A, eagerReducer: ((S, A) => S) | null, eagerState: S | null, @@ -714,17 +709,13 @@ function updateReducer( let newBaseQueueLast = null; let update = first; do { - const suspenseConfig = update.suspenseConfig; const updateLane = update.lane; - const updateEventTime = update.eventTime; if (!isSubsetOfLanes(renderLanes, updateLane)) { // Priority is insufficient. Skip this update. If this is the first // skipped update, the previous update/state is the new base // update/state. const clone: Update = { - eventTime: updateEventTime, lane: updateLane, - suspenseConfig: suspenseConfig, action: update.action, eagerReducer: update.eagerReducer, eagerState: update.eagerState, @@ -749,12 +740,10 @@ function updateReducer( if (newBaseQueueLast !== null) { const clone: Update = { - eventTime: updateEventTime, // This update is going to be committed so we never want uncommit // it. Using NoLane works because 0 is a subset of all bitmasks, so // this will never be skipped by the check above. lane: NoLane, - suspenseConfig: update.suspenseConfig, action: update.action, eagerReducer: update.eagerReducer, eagerState: update.eagerState, @@ -763,14 +752,6 @@ function updateReducer( newBaseQueueLast = newBaseQueueLast.next = clone; } - // Mark the event time of this update as relevant to this render pass. - // TODO: This should ideally use the true event time of this update rather than - // its priority which is a derived and not reversible value. - // TODO: We should skip this update if it was already committed but currently - // we have no way of detecting the difference between a committed and suspended - // update here. - markRenderEventTimeAndConfig(updateEventTime, suspenseConfig); - // Process this update. if (update.eagerReducer === reducer) { // If this update was processed eagerly, and its reducer matches the @@ -1706,9 +1687,7 @@ function dispatchAction( const lane = requestUpdateLane(fiber, suspenseConfig); const update: Update = { - eventTime, lane, - suspenseConfig, action, eagerReducer: null, eagerState: null, diff --git a/packages/react-reconciler/src/ReactFiberLane.js b/packages/react-reconciler/src/ReactFiberLane.js index d9ee9f43eb16c..8e6a66f9210a8 100644 --- a/packages/react-reconciler/src/ReactFiberLane.js +++ b/packages/react-reconciler/src/ReactFiberLane.js @@ -461,6 +461,9 @@ export function includesNonIdleWork(lanes: Lanes) { export function includesOnlyRetries(lanes: Lanes) { return (lanes & RetryLanes) === lanes; } +export function includesOnlyTransitions(lanes: Lanes) { + return (lanes & TransitionLanes) === lanes; +} // To ensure consistency across multiple updates in the same event, this should // be a pure function, so that it always returns the same lane for given inputs. diff --git a/packages/react-reconciler/src/ReactFiberNewContext.new.js b/packages/react-reconciler/src/ReactFiberNewContext.new.js index 43f0ac07b773b..fb464e62a2592 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.new.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.new.js @@ -212,7 +212,6 @@ export function propagateContextChange( const update = createUpdate( NoTimestamp, pickArbitraryLane(renderLanes), - null, ); update.tag = ForceUpdate; // TODO: Because we don't have a work-in-progress, this will add the diff --git a/packages/react-reconciler/src/ReactFiberNewContext.old.js b/packages/react-reconciler/src/ReactFiberNewContext.old.js index 92b78587737c7..af28997272441 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.old.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.old.js @@ -212,7 +212,6 @@ export function propagateContextChange( const update = createUpdate( NoTimestamp, pickArbitraryLane(renderLanes), - null, ); update.tag = ForceUpdate; // TODO: Because we don't have a work-in-progress, this will add the diff --git a/packages/react-reconciler/src/ReactFiberReconciler.new.js b/packages/react-reconciler/src/ReactFiberReconciler.new.js index befd70866dc53..fd90dda8e961c 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.new.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.new.js @@ -297,7 +297,7 @@ export function updateContainer( } } - const update = createUpdate(eventTime, lane, suspenseConfig); + const update = createUpdate(eventTime, lane); // Caution: React DevTools currently depends on this property // being called "element". update.payload = {element}; diff --git a/packages/react-reconciler/src/ReactFiberReconciler.old.js b/packages/react-reconciler/src/ReactFiberReconciler.old.js index d2cecfed4976a..549d69724385d 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.old.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.old.js @@ -297,7 +297,7 @@ export function updateContainer( } } - const update = createUpdate(eventTime, lane, suspenseConfig); + const update = createUpdate(eventTime, lane); // Caution: React DevTools currently depends on this property // being called "element". update.payload = {element}; diff --git a/packages/react-reconciler/src/ReactFiberThrow.new.js b/packages/react-reconciler/src/ReactFiberThrow.new.js index 12d3eb663c88b..f5c6d2aba4cb5 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.new.js +++ b/packages/react-reconciler/src/ReactFiberThrow.new.js @@ -76,7 +76,7 @@ function createRootErrorUpdate( errorInfo: CapturedValue, lane: Lane, ): Update { - const update = createUpdate(NoTimestamp, lane, null); + const update = createUpdate(NoTimestamp, lane); // Unmount the root by rendering null. update.tag = CaptureUpdate; // Caution: React DevTools currently depends on this property @@ -95,7 +95,7 @@ function createClassErrorUpdate( errorInfo: CapturedValue, lane: Lane, ): Update { - const update = createUpdate(NoTimestamp, lane, null); + const update = createUpdate(NoTimestamp, lane); update.tag = CaptureUpdate; const getDerivedStateFromError = fiber.type.getDerivedStateFromError; if (typeof getDerivedStateFromError === 'function') { @@ -274,7 +274,7 @@ function throwException( // When we try rendering again, we should not reuse the current fiber, // since it's known to be in an inconsistent state. Use a force update to // prevent a bail out. - const update = createUpdate(NoTimestamp, SyncLane, null); + const update = createUpdate(NoTimestamp, SyncLane); update.tag = ForceUpdate; enqueueUpdate(sourceFiber, update); } diff --git a/packages/react-reconciler/src/ReactFiberThrow.old.js b/packages/react-reconciler/src/ReactFiberThrow.old.js index a215d363e2fa0..e6c27de5e9853 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.old.js +++ b/packages/react-reconciler/src/ReactFiberThrow.old.js @@ -76,7 +76,7 @@ function createRootErrorUpdate( errorInfo: CapturedValue, lane: Lane, ): Update { - const update = createUpdate(NoTimestamp, lane, null); + const update = createUpdate(NoTimestamp, lane); // Unmount the root by rendering null. update.tag = CaptureUpdate; // Caution: React DevTools currently depends on this property @@ -95,7 +95,7 @@ function createClassErrorUpdate( errorInfo: CapturedValue, lane: Lane, ): Update { - const update = createUpdate(NoTimestamp, lane, null); + const update = createUpdate(NoTimestamp, lane); update.tag = CaptureUpdate; const getDerivedStateFromError = fiber.type.getDerivedStateFromError; if (typeof getDerivedStateFromError === 'function') { @@ -276,7 +276,7 @@ function throwException( // When we try rendering again, we should not reuse the current fiber, // since it's known to be in an inconsistent state. Use a force update to // prevent a bail out. - const update = createUpdate(NoTimestamp, SyncLane, null); + const update = createUpdate(NoTimestamp, SyncLane); update.tag = ForceUpdate; enqueueUpdate(sourceFiber, update); } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index ca8f6e45b0e1c..4205a65ba4f10 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -171,6 +171,7 @@ import { hasDiscreteLanes, includesNonIdleWork, includesOnlyRetries, + includesOnlyTransitions, getNextLanes, returnNextLanesPriority, setCurrentUpdateLanePriority, @@ -302,8 +303,6 @@ const subtreeRenderLanesCursor: StackCursor = createCursor(NoLanes); let workInProgressRootExitStatus: RootExitStatus = RootIncomplete; // A fatal error, if one is thrown let workInProgressRootFatalError: mixed = null; -let workInProgressRootLatestSuspenseTimeout: number = NoTimestamp; -let workInProgressRootCanSuspendUsingConfig: null | SuspenseConfig = null; // "Included" lanes refer to lanes that were worked on during this render. It's // slightly different than `renderLanes` because `renderLanes` can change as you // enter and exit an Offscreen tree. This value is the combination of all render @@ -323,7 +322,6 @@ let mostRecentlyUpdatedRoot: FiberRoot | null = null; // model where we don't commit new loading states in too quick succession. let globalMostRecentFallbackTime: number = 0; const FALLBACK_THROTTLE_MS: number = 500; -const DEFAULT_TIMEOUT_MS: number = 5000; // The absolute time for when we should start giving up on rendering // more and prefer CPU suspense heuristics instead. @@ -923,12 +921,10 @@ function finishConcurrentRender(root, exitStatus, lanes) { case RootSuspendedWithDelay: { markRootSuspended(root, lanes); - if (workInProgressRootLatestSuspenseTimeout !== NoTimestamp) { + if (includesOnlyTransitions(lanes)) { // This is a transition, so we should exit without committing a // placeholder and without scheduling a timeout. Delay indefinitely // until we receive more data. - // TODO: Check the lanes to see if it's a transition, instead of - // tracking the latest timeout. break; } @@ -963,30 +959,6 @@ function finishConcurrentRender(root, exitStatus, lanes) { } case RootCompleted: { // The work completed. Ready to commit. - const mostRecentEventTime = getMostRecentEventTime(root, lanes); - if ( - // do not delay if we're inside an act() scope - !shouldForceFlushFallbacksInDEV() && - mostRecentEventTime !== NoTimestamp && - workInProgressRootCanSuspendUsingConfig !== null - ) { - // If we have exceeded the minimum loading delay, which probably - // means we have shown a spinner already, we might have to suspend - // a bit longer to ensure that the spinner is shown for - // enough time. - const msUntilTimeout = computeMsUntilSuspenseLoadingDelay( - mostRecentEventTime, - workInProgressRootCanSuspendUsingConfig, - ); - if (msUntilTimeout > 10) { - markRootSuspended(root, lanes); - root.timeoutHandle = scheduleTimeout( - commitRoot.bind(null, root), - msUntilTimeout, - ); - break; - } - } commitRoot(root); break; } @@ -1369,8 +1341,6 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes) { workInProgressRootRenderLanes = subtreeRenderLanes = workInProgressRootIncludedLanes = lanes; workInProgressRootExitStatus = RootIncomplete; workInProgressRootFatalError = null; - workInProgressRootLatestSuspenseTimeout = NoTimestamp; - workInProgressRootCanSuspendUsingConfig = null; workInProgressRootSkippedLanes = NoLanes; workInProgressRootUpdatedLanes = NoLanes; workInProgressRootPingedLanes = NoLanes; @@ -1482,30 +1452,6 @@ export function markCommitTimeOfFallback() { globalMostRecentFallbackTime = now(); } -export function markRenderEventTimeAndConfig( - eventTime: number, - suspenseConfig: null | SuspenseConfig, -): void { - // Track the largest/latest timeout deadline in this batch. - // TODO: If there are two transitions in the same batch, shouldn't we - // choose the smaller one? Maybe this is because when an intermediate - // transition is superseded, we should ignore its suspense config, but - // we don't currently. - if (suspenseConfig !== null) { - // If `timeoutMs` is not specified, we default to 5 seconds. We have to - // resolve this default here because `suspenseConfig` is owned - // by userspace. - // TODO: Store this on the root instead (transition -> timeoutMs) - // TODO: Should this default to a JND instead? - const timeoutMs = suspenseConfig.timeoutMs | 0 || DEFAULT_TIMEOUT_MS; - const timeoutTime = eventTime + timeoutMs; - if (timeoutTime > workInProgressRootLatestSuspenseTimeout) { - workInProgressRootLatestSuspenseTimeout = timeoutTime; - workInProgressRootCanSuspendUsingConfig = suspenseConfig; - } - } -} - export function markSkippedUpdateLanes(lane: Lane | Lanes): void { workInProgressRootSkippedLanes = mergeLanes( lane, @@ -3079,30 +3025,6 @@ function jnd(timeElapsed: number) { : ceil(timeElapsed / 1960) * 1960; } -function computeMsUntilSuspenseLoadingDelay( - mostRecentEventTime: number, - suspenseConfig: SuspenseConfig, -) { - const busyMinDurationMs = (suspenseConfig.busyMinDurationMs: any) | 0; - if (busyMinDurationMs <= 0) { - return 0; - } - const busyDelayMs = (suspenseConfig.busyDelayMs: any) | 0; - - // Compute the time until this render pass would expire. - const currentTimeMs: number = now(); - const eventTimeMs: number = mostRecentEventTime; - const timeElapsed = currentTimeMs - eventTimeMs; - if (timeElapsed <= busyDelayMs) { - // If we haven't yet waited longer than the initial delay, we don't - // have to wait any additional time. - return 0; - } - const msUntilTimeout = busyDelayMs + busyMinDurationMs - timeElapsed; - // This is the value that is passed to `setTimeout`. - return msUntilTimeout; -} - function checkForNestedUpdates() { if (nestedUpdateCount > NESTED_UPDATE_LIMIT) { nestedUpdateCount = 0; diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index 37e781a18bbb3..47dae385793b8 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -157,6 +157,7 @@ import { hasDiscreteLanes, includesNonIdleWork, includesOnlyRetries, + includesOnlyTransitions, getNextLanes, returnNextLanesPriority, setCurrentUpdateLanePriority, @@ -287,8 +288,6 @@ const subtreeRenderLanesCursor: StackCursor = createCursor(NoLanes); let workInProgressRootExitStatus: RootExitStatus = RootIncomplete; // A fatal error, if one is thrown let workInProgressRootFatalError: mixed = null; -let workInProgressRootLatestSuspenseTimeout: number = NoTimestamp; -let workInProgressRootCanSuspendUsingConfig: null | SuspenseConfig = null; // "Included" lanes refer to lanes that were worked on during this render. It's // slightly different than `renderLanes` because `renderLanes` can change as you // enter and exit an Offscreen tree. This value is the combination of all render @@ -308,7 +307,6 @@ let mostRecentlyUpdatedRoot: FiberRoot | null = null; // model where we don't commit new loading states in too quick succession. let globalMostRecentFallbackTime: number = 0; const FALLBACK_THROTTLE_MS: number = 500; -const DEFAULT_TIMEOUT_MS: number = 5000; // The absolute time for when we should start giving up on rendering // more and prefer CPU suspense heuristics instead. @@ -911,12 +909,10 @@ function finishConcurrentRender(root, exitStatus, lanes) { case RootSuspendedWithDelay: { markRootSuspended(root, lanes); - if (workInProgressRootLatestSuspenseTimeout !== NoTimestamp) { + if (includesOnlyTransitions(lanes)) { // This is a transition, so we should exit without committing a // placeholder and without scheduling a timeout. Delay indefinitely // until we receive more data. - // TODO: Check the lanes to see if it's a transition, instead of - // tracking the latest timeout. break; } @@ -951,30 +947,6 @@ function finishConcurrentRender(root, exitStatus, lanes) { } case RootCompleted: { // The work completed. Ready to commit. - const mostRecentEventTime = getMostRecentEventTime(root, lanes); - if ( - // do not delay if we're inside an act() scope - !shouldForceFlushFallbacksInDEV() && - mostRecentEventTime !== NoTimestamp && - workInProgressRootCanSuspendUsingConfig !== null - ) { - // If we have exceeded the minimum loading delay, which probably - // means we have shown a spinner already, we might have to suspend - // a bit longer to ensure that the spinner is shown for - // enough time. - const msUntilTimeout = computeMsUntilSuspenseLoadingDelay( - mostRecentEventTime, - workInProgressRootCanSuspendUsingConfig, - ); - if (msUntilTimeout > 10) { - markRootSuspended(root, lanes); - root.timeoutHandle = scheduleTimeout( - commitRoot.bind(null, root), - msUntilTimeout, - ); - break; - } - } commitRoot(root); break; } @@ -1357,8 +1329,6 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes) { workInProgressRootRenderLanes = subtreeRenderLanes = workInProgressRootIncludedLanes = lanes; workInProgressRootExitStatus = RootIncomplete; workInProgressRootFatalError = null; - workInProgressRootLatestSuspenseTimeout = NoTimestamp; - workInProgressRootCanSuspendUsingConfig = null; workInProgressRootSkippedLanes = NoLanes; workInProgressRootUpdatedLanes = NoLanes; workInProgressRootPingedLanes = NoLanes; @@ -1470,30 +1440,6 @@ export function markCommitTimeOfFallback() { globalMostRecentFallbackTime = now(); } -export function markRenderEventTimeAndConfig( - eventTime: number, - suspenseConfig: null | SuspenseConfig, -): void { - // Track the largest/latest timeout deadline in this batch. - // TODO: If there are two transitions in the same batch, shouldn't we - // choose the smaller one? Maybe this is because when an intermediate - // transition is superseded, we should ignore its suspense config, but - // we don't currently. - if (suspenseConfig !== null) { - // If `timeoutMs` is not specified, we default to 5 seconds. We have to - // resolve this default here because `suspenseConfig` is owned - // by userspace. - // TODO: Store this on the root instead (transition -> timeoutMs) - // TODO: Should this default to a JND instead? - const timeoutMs = suspenseConfig.timeoutMs | 0 || DEFAULT_TIMEOUT_MS; - const timeoutTime = eventTime + timeoutMs; - if (timeoutTime > workInProgressRootLatestSuspenseTimeout) { - workInProgressRootLatestSuspenseTimeout = timeoutTime; - workInProgressRootCanSuspendUsingConfig = suspenseConfig; - } - } -} - export function markSkippedUpdateLanes(lane: Lane | Lanes): void { workInProgressRootSkippedLanes = mergeLanes( lane, @@ -3025,30 +2971,6 @@ function jnd(timeElapsed: number) { : ceil(timeElapsed / 1960) * 1960; } -function computeMsUntilSuspenseLoadingDelay( - mostRecentEventTime: number, - suspenseConfig: SuspenseConfig, -) { - const busyMinDurationMs = (suspenseConfig.busyMinDurationMs: any) | 0; - if (busyMinDurationMs <= 0) { - return 0; - } - const busyDelayMs = (suspenseConfig.busyDelayMs: any) | 0; - - // Compute the time until this render pass would expire. - const currentTimeMs: number = now(); - const eventTimeMs: number = mostRecentEventTime; - const timeElapsed = currentTimeMs - eventTimeMs; - if (timeElapsed <= busyDelayMs) { - // If we haven't yet waited longer than the initial delay, we don't - // have to wait any additional time. - return 0; - } - const msUntilTimeout = busyDelayMs + busyMinDurationMs - timeElapsed; - // This is the value that is passed to `setTimeout`. - return msUntilTimeout; -} - function checkForNestedUpdates() { if (nestedUpdateCount > NESTED_UPDATE_LIMIT) { nestedUpdateCount = 0; diff --git a/packages/react-reconciler/src/ReactUpdateQueue.new.js b/packages/react-reconciler/src/ReactUpdateQueue.new.js index ae5f441a8e921..303e02f07d609 100644 --- a/packages/react-reconciler/src/ReactUpdateQueue.new.js +++ b/packages/react-reconciler/src/ReactUpdateQueue.new.js @@ -86,7 +86,6 @@ import type {Fiber} from './ReactInternalTypes'; import type {Lanes, Lane} from './ReactFiberLane'; -import type {SuspenseConfig} from './ReactFiberSuspenseConfig'; import {NoLane, NoLanes, isSubsetOfLanes, mergeLanes} from './ReactFiberLane'; import { @@ -98,10 +97,7 @@ import {Callback, ShouldCapture, DidCapture} from './ReactSideEffectTags'; import {debugRenderPhaseSideEffectsForStrictMode} from 'shared/ReactFeatureFlags'; import {StrictMode} from './ReactTypeOfMode'; -import { - markRenderEventTimeAndConfig, - markSkippedUpdateLanes, -} from './ReactFiberWorkLoop.new'; +import {markSkippedUpdateLanes} from './ReactFiberWorkLoop.new'; import invariant from 'shared/invariant'; @@ -112,7 +108,6 @@ export type Update = {| // transition -> event time on the root. eventTime: number, lane: Lane, - suspenseConfig: null | SuspenseConfig, tag: 0 | 1 | 2 | 3, payload: any, @@ -186,15 +181,10 @@ export function cloneUpdateQueue( } } -export function createUpdate( - eventTime: number, - lane: Lane, - suspenseConfig: null | SuspenseConfig, -): Update<*> { +export function createUpdate(eventTime: number, lane: Lane): Update<*> { const update: Update<*> = { eventTime, lane, - suspenseConfig, tag: UpdateState, payload: null, @@ -269,7 +259,6 @@ export function enqueueCapturedUpdate( const clone: Update = { eventTime: update.eventTime, lane: update.lane, - suspenseConfig: update.suspenseConfig, tag: update.tag, payload: update.payload, @@ -482,7 +471,6 @@ export function processUpdateQueue( const clone: Update = { eventTime: updateEventTime, lane: updateLane, - suspenseConfig: update.suspenseConfig, tag: update.tag, payload: update.payload, @@ -508,7 +496,6 @@ export function processUpdateQueue( // it. Using NoLane works because 0 is a subset of all bitmasks, so // this will never be skipped by the check above. lane: NoLane, - suspenseConfig: update.suspenseConfig, tag: update.tag, payload: update.payload, @@ -519,14 +506,6 @@ export function processUpdateQueue( newLastBaseUpdate = newLastBaseUpdate.next = clone; } - // Mark the event time of this update as relevant to this render pass. - // TODO: This should ideally use the true event time of this update rather than - // its priority which is a derived and not reversible value. - // TODO: We should skip this update if it was already committed but currently - // we have no way of detecting the difference between a committed and suspended - // update here. - markRenderEventTimeAndConfig(updateEventTime, update.suspenseConfig); - // Process this update. newState = getStateFromUpdate( workInProgress, diff --git a/packages/react-reconciler/src/ReactUpdateQueue.old.js b/packages/react-reconciler/src/ReactUpdateQueue.old.js index 48d4cb63bb39f..787ec3d18c3a3 100644 --- a/packages/react-reconciler/src/ReactUpdateQueue.old.js +++ b/packages/react-reconciler/src/ReactUpdateQueue.old.js @@ -86,7 +86,6 @@ import type {Fiber} from './ReactInternalTypes'; import type {Lanes, Lane} from './ReactFiberLane'; -import type {SuspenseConfig} from './ReactFiberSuspenseConfig'; import {NoLane, NoLanes, isSubsetOfLanes, mergeLanes} from './ReactFiberLane'; import { @@ -98,10 +97,7 @@ import {Callback, ShouldCapture, DidCapture} from './ReactSideEffectTags'; import {debugRenderPhaseSideEffectsForStrictMode} from 'shared/ReactFeatureFlags'; import {StrictMode} from './ReactTypeOfMode'; -import { - markRenderEventTimeAndConfig, - markSkippedUpdateLanes, -} from './ReactFiberWorkLoop.old'; +import {markSkippedUpdateLanes} from './ReactFiberWorkLoop.old'; import invariant from 'shared/invariant'; @@ -112,7 +108,6 @@ export type Update = {| // transition -> event time on the root. eventTime: number, lane: Lane, - suspenseConfig: null | SuspenseConfig, tag: 0 | 1 | 2 | 3, payload: any, @@ -186,15 +181,10 @@ export function cloneUpdateQueue( } } -export function createUpdate( - eventTime: number, - lane: Lane, - suspenseConfig: null | SuspenseConfig, -): Update<*> { +export function createUpdate(eventTime: number, lane: Lane): Update<*> { const update: Update<*> = { eventTime, lane, - suspenseConfig, tag: UpdateState, payload: null, @@ -269,7 +259,6 @@ export function enqueueCapturedUpdate( const clone: Update = { eventTime: update.eventTime, lane: update.lane, - suspenseConfig: update.suspenseConfig, tag: update.tag, payload: update.payload, @@ -482,7 +471,6 @@ export function processUpdateQueue( const clone: Update = { eventTime: updateEventTime, lane: updateLane, - suspenseConfig: update.suspenseConfig, tag: update.tag, payload: update.payload, @@ -508,7 +496,6 @@ export function processUpdateQueue( // it. Using NoLane works because 0 is a subset of all bitmasks, so // this will never be skipped by the check above. lane: NoLane, - suspenseConfig: update.suspenseConfig, tag: update.tag, payload: update.payload, @@ -519,14 +506,6 @@ export function processUpdateQueue( newLastBaseUpdate = newLastBaseUpdate.next = clone; } - // Mark the event time of this update as relevant to this render pass. - // TODO: This should ideally use the true event time of this update rather than - // its priority which is a derived and not reversible value. - // TODO: We should skip this update if it was already committed but currently - // we have no way of detecting the difference between a committed and suspended - // update here. - markRenderEventTimeAndConfig(updateEventTime, update.suspenseConfig); - // Process this update. newState = getStateFromUpdate( workInProgress, diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js index 8b673ed80752d..3296a8ec6dcc0 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js @@ -3318,85 +3318,6 @@ describe('ReactHooksWithNoopRenderer', () => { ]); }); }); - - // @gate experimental - it('delays showing loading state until after busyDelayMs + busyMinDurationMs', async () => { - let transition; - function App() { - const [show, setShow] = useState(false); - const [startTransition, isPending] = useTransition({ - busyDelayMs: 1000, - busyMinDurationMs: 2000, - }); - transition = () => { - startTransition(() => { - setShow(true); - }); - }; - return ( - }> - {show ? ( - - ) : ( - - )} - - ); - } - ReactNoop.render(); - expect(Scheduler).toFlushAndYield(['Before... Pending: false']); - expect(ReactNoop.getChildren()).toEqual([ - span('Before... Pending: false'), - ]); - - await act(async () => { - Scheduler.unstable_runWithPriority( - Scheduler.unstable_UserBlockingPriority, - transition, - ); - - expect(Scheduler).toFlushAndYield([ - 'Before... Pending: true', - 'Suspend! [After... Pending: false]', - 'Loading... Pending: false', - ]); - expect(ReactNoop.getChildren()).toEqual([ - span('Before... Pending: true'), - ]); - - Scheduler.unstable_advanceTime(1000); - await advanceTimers(1000); - - // Resolve the promise. The whole tree has now completed. However, - // because we exceeded the busy threshold, we won't commit the - // result yet. - Scheduler.unstable_advanceTime(1000); - await advanceTimers(1000); - await resolveText('After... Pending: false'); - expect(Scheduler).toHaveYielded([ - 'Promise resolved [After... Pending: false]', - ]); - expect(Scheduler).toFlushAndYield(['After... Pending: false']); - expect(ReactNoop.getChildren()).toEqual([ - span('Before... Pending: true'), - ]); - - // Advance time until just before the `busyMinDuration` threshold. - Scheduler.unstable_advanceTime(999); - await advanceTimers(999); - expect(ReactNoop.getChildren()).toEqual([ - span('Before... Pending: true'), - ]); - - // Advance time just a bit more. Now we complete the transition. - Scheduler.unstable_advanceTime(300); - await advanceTimers(300); - expect(ReactNoop.getChildren()).toEqual([ - span('After... Pending: false'), - ]); - }); - }); }); describe('useDeferredValue', () => { diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js index 693f2712f1e80..035bd8bc57537 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js @@ -2860,74 +2860,6 @@ describe('ReactSuspenseWithNoopRenderer', () => { ); }); - // @gate experimental - it('supports delaying a busy spinner from disappearing', async () => { - const SUSPENSE_CONFIG = { - timeoutMs: 10000, - busyDelayMs: 500, - busyMinDurationMs: 400, - }; - - let transitionToPage; - - function App() { - const [page, setPage] = React.useState('A'); - const [startLoading, isLoading] = React.unstable_useTransition( - SUSPENSE_CONFIG, - ); - transitionToPage = nextPage => startLoading(() => setPage(nextPage)); - return ( - - - {isLoading ? : null} - - ); - } - - // Initial render. - ReactNoop.render(); - expect(Scheduler).toFlushAndYield(['A']); - expect(ReactNoop.getChildren()).toEqual([span('A')]); - - await ReactNoop.act(async () => { - transitionToPage('B'); - // Rendering B is quick and we didn't have enough - // time to show the loading indicator. - Scheduler.unstable_advanceTime(200); - await advanceTimers(200); - expect(Scheduler).toFlushAndYield(['A', 'L', 'B']); - expect(ReactNoop.getChildren()).toEqual([span('B')]); - }); - - await ReactNoop.act(async () => { - transitionToPage('C'); - // Rendering C is a bit slower so we've already showed - // the loading indicator. - Scheduler.unstable_advanceTime(600); - await advanceTimers(600); - expect(Scheduler).toFlushAndYield(['B', 'L', 'C']); - // We're technically done now but we haven't shown the - // loading indicator for long enough yet so we'll suspend - // while we keep it on the screen a bit longer. - expect(ReactNoop.getChildren()).toEqual([span('B'), span('L')]); - Scheduler.unstable_advanceTime(400); - await advanceTimers(400); - expect(ReactNoop.getChildren()).toEqual([span('C')]); - }); - - await ReactNoop.act(async () => { - transitionToPage('D'); - // Rendering D is very slow so we've already showed - // the loading indicator. - Scheduler.unstable_advanceTime(1000); - await advanceTimers(1000); - expect(Scheduler).toFlushAndYield(['C', 'L', 'D']); - // However, since we exceeded the minimum time to show - // the loading indicator, we commit immediately. - expect(ReactNoop.getChildren()).toEqual([span('D')]); - }); - }); - it("suspended commit remains suspended even if there's another update at same expiration", async () => { // Regression test function App({text}) {