diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 6c34f2bbf4c69..9db5886a6c2b4 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -137,7 +137,7 @@ import { calculateChangedBits, scheduleWorkOnParentPath, } from './ReactFiberNewContext'; -import {resetHooks, renderWithHooks, bailoutHooks} from './ReactFiberHooks'; +import {renderWithHooks, bailoutHooks} from './ReactFiberHooks'; import {stopProfilerTimerIfRunning} from './ReactProfilerTimer'; import { getMaskedContext, @@ -1407,7 +1407,8 @@ function mountIndeterminateComponent( workInProgress.tag = ClassComponent; // Throw out any hooks that were used. - resetHooks(); + workInProgress.memoizedState = null; + workInProgress.updateQueue = null; // Push context providers early to prevent context stack mismatches. // During mounting we don't know the child context yet as the instance doesn't exist. diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.js b/packages/react-reconciler/src/ReactFiberClassComponent.js index 6b28bf900a89c..54622014870a5 100644 --- a/packages/react-reconciler/src/ReactFiberClassComponent.js +++ b/packages/react-reconciler/src/ReactFiberClassComponent.js @@ -190,7 +190,7 @@ const classComponentUpdater = { suspenseConfig, ); - const update = createUpdate(expirationTime, suspenseConfig); + const update = createUpdate(currentTime, expirationTime, suspenseConfig); update.payload = payload; if (callback !== undefined && callback !== null) { if (__DEV__) { @@ -212,7 +212,7 @@ const classComponentUpdater = { suspenseConfig, ); - const update = createUpdate(expirationTime, suspenseConfig); + const update = createUpdate(currentTime, expirationTime, suspenseConfig); update.tag = ReplaceState; update.payload = payload; @@ -236,7 +236,7 @@ const classComponentUpdater = { suspenseConfig, ); - const update = createUpdate(expirationTime, suspenseConfig); + const update = createUpdate(currentTime, expirationTime, suspenseConfig); update.tag = ForceUpdate; if (callback !== undefined && callback !== null) { diff --git a/packages/react-reconciler/src/ReactFiberExpirationTime.js b/packages/react-reconciler/src/ReactFiberExpirationTime.js index 5f4660dd5175f..25a576117b59c 100644 --- a/packages/react-reconciler/src/ReactFiberExpirationTime.js +++ b/packages/react-reconciler/src/ReactFiberExpirationTime.js @@ -85,16 +85,13 @@ export function computeAsyncExpiration( ); } -export function computeSuspenseExpiration( +export function computeSuspenseTimeout( currentTime: ExpirationTime, timeoutMs: number, ): ExpirationTime { - // TODO: Should we warn if timeoutMs is lower than the normal pri expiration time? - return computeExpirationBucket( - currentTime, - timeoutMs, - LOW_PRIORITY_BATCH_SIZE, - ); + const currentTimeMs = expirationTimeToMs(currentTime); + const deadlineMs = currentTimeMs + timeoutMs; + return msToExpirationTime(deadlineMs); } // We intentionally set a higher expiration time for interactive updates in diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 393b2c5653660..aabb6b7ed890f 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -16,8 +16,10 @@ import type {Fiber} from './ReactFiber'; import type {ExpirationTime} from './ReactFiberExpirationTime'; import type {HookEffectTag} from './ReactHookEffectTags'; import type {SuspenseConfig} from './ReactFiberSuspenseConfig'; +import type {TransitionInstance} from './ReactFiberTransition'; import type {ReactPriorityLevel} from './SchedulerWithReactIntegration'; +import {preventIntermediateStates} from 'shared/ReactFeatureFlags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import {NoWork, Sync} from './ReactFiberExpirationTime'; @@ -49,13 +51,14 @@ import invariant from 'shared/invariant'; import getComponentName from 'shared/getComponentName'; import is from 'shared/objectIs'; import {markWorkInProgressReceivedUpdate} from './ReactFiberBeginWork'; +import {SuspendOnTask} from './ReactFiberThrow'; import {requestCurrentSuspenseConfig} from './ReactFiberSuspenseConfig'; import { - UserBlockingPriority, - NormalPriority, - runWithPriority, - getCurrentPriorityLevel, -} from './SchedulerWithReactIntegration'; + startTransition, + requestCurrentTransition, + cancelPendingTransition, +} from './ReactFiberTransition'; +import {getCurrentPriorityLevel} from './SchedulerWithReactIntegration'; const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals; @@ -102,6 +105,9 @@ export type Dispatcher = {| |}; type Update = {| + // TODO: Temporary field. Will remove this by storing a map of + // transition -> start time on the root. + eventTime: ExpirationTime, expirationTime: ExpirationTime, suspenseConfig: null | SuspenseConfig, action: A, @@ -114,6 +120,7 @@ type Update = {| type UpdateQueue = {| pending: Update | null, dispatch: (A => mixed) | null, + pendingTransition: TransitionInstance | null, lastRenderedReducer: ((S, A) => S) | null, lastRenderedState: S | null, |}; @@ -177,23 +184,12 @@ let currentlyRenderingFiber: Fiber = (null: any); let currentHook: Hook | null = null; let workInProgressHook: Hook | null = null; -// Updates scheduled during render will trigger an immediate re-render at the -// end of the current pass. We can't store these updates on the normal queue, -// because if the work is aborted, they should be discarded. Because this is -// a relatively rare case, we also don't want to add an additional field to -// either the hook or queue object types. So we store them in a lazily create -// map of queue -> render-phase updates, which are discarded once the component -// completes without re-rendering. - -// Whether an update was scheduled during the currently executing render pass. +// Whether an update was scheduled at any point during the render phase. This +// does not get reset if we do another render pass; only when we're completely +// finished evaluating this component. This is an optimization so we know +// whether we need to clear render phase updates after a throw. let didScheduleRenderPhaseUpdate: boolean = false; -// Lazily created map of render-phase updates -let renderPhaseUpdates: Map< - UpdateQueue, - Update, -> | null = null; -// Counter to prevent infinite loops. -let numberOfReRenders: number = 0; + const RE_RENDER_LIMIT = 25; // In DEV, this is the name of the currently executing primitive hook @@ -387,8 +383,6 @@ export function renderWithHooks( // workInProgressHook = null; // didScheduleRenderPhaseUpdate = false; - // renderPhaseUpdates = null; - // numberOfReRenders = 0; // TODO Warn if no hooks are used at all during mount, then some are used during update. // Currently we will identify the update render as a mount because memoizedState === null. @@ -419,9 +413,20 @@ export function renderWithHooks( let children = Component(props, secondArg); - if (didScheduleRenderPhaseUpdate) { + // Check if there was a render phase update + if (workInProgress.expirationTime === renderExpirationTime) { + // Keep rendering in a loop for as long as render phase updates continue to + // be scheduled. Use a counter to prevent infinite loops. + let numberOfReRenders: number = 0; do { - didScheduleRenderPhaseUpdate = false; + workInProgress.expirationTime = NoWork; + + invariant( + numberOfReRenders < RE_RENDER_LIMIT, + 'Too many re-renders. React limits the number of renders to prevent ' + + 'an infinite loop.', + ); + numberOfReRenders += 1; if (__DEV__) { // Even when hot reloading, allow dependencies to stabilize @@ -441,14 +446,11 @@ export function renderWithHooks( } ReactCurrentDispatcher.current = __DEV__ - ? HooksDispatcherOnUpdateInDEV - : HooksDispatcherOnUpdate; + ? HooksDispatcherOnRerenderInDEV + : HooksDispatcherOnRerender; children = Component(props, secondArg); - } while (didScheduleRenderPhaseUpdate); - - renderPhaseUpdates = null; - numberOfReRenders = 0; + } while (workInProgress.expirationTime === renderExpirationTime); } // We can assume the previous dispatcher is always this one, since we set it @@ -476,10 +478,7 @@ export function renderWithHooks( hookTypesUpdateIndexDev = -1; } - // These were reset above - // didScheduleRenderPhaseUpdate = false; - // renderPhaseUpdates = null; - // numberOfReRenders = 0; + didScheduleRenderPhaseUpdate = false; invariant( !didRenderTooFewHooks, @@ -502,14 +501,29 @@ export function bailoutHooks( } } -export function resetHooks(): void { +export function resetHooksAfterThrow(): void { // We can assume the previous dispatcher is always this one, since we set it // at the beginning of the render phase and there's no re-entrancy. ReactCurrentDispatcher.current = ContextOnlyDispatcher; - // 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. + if (didScheduleRenderPhaseUpdate) { + // There were render phase updates. These are only valid for this render + // phase, which we are now aborting. Remove the updates from the queues so + // they do not persist to the next render. Do not remove updates from hooks + // that weren't processed. + // + // Only reset the updates from the queue if it has a clone. If it does + // not have a clone, that means it wasn't processed, and the updates were + // scheduled before we entered the render phase. + let hook: Hook | null = currentlyRenderingFiber.memoizedState; + while (hook !== null) { + const queue = hook.queue; + if (queue !== null) { + queue.pending = null; + } + hook = hook.next; + } + } renderExpirationTime = NoWork; currentlyRenderingFiber = (null: any); @@ -525,8 +539,6 @@ export function resetHooks(): void { } didScheduleRenderPhaseUpdate = false; - renderPhaseUpdates = null; - numberOfReRenders = 0; } function mountWorkInProgressHook(): Hook { @@ -637,6 +649,7 @@ function mountReducer( const queue = (hook.queue = { pending: null, dispatch: null, + pendingTransition: null, lastRenderedReducer: reducer, lastRenderedState: (initialState: any), }); @@ -662,49 +675,6 @@ function updateReducer( queue.lastRenderedReducer = reducer; - if (numberOfReRenders > 0) { - // This is a re-render. Apply the new render phase updates to the previous - // work-in-progress hook. - const dispatch: Dispatch = (queue.dispatch: any); - if (renderPhaseUpdates !== null) { - // Render phase updates are stored in a map of queue -> linked list - const firstRenderPhaseUpdate = renderPhaseUpdates.get(queue); - if (firstRenderPhaseUpdate !== undefined) { - renderPhaseUpdates.delete(queue); - let newState = hook.memoizedState; - let update = firstRenderPhaseUpdate; - do { - // Process this render phase update. We don't have to check the - // priority because it will always be the same as the current - // render's. - const action = update.action; - newState = reducer(newState, action); - update = update.next; - } while (update !== null); - - // Mark that the fiber performed work, but only if the new state is - // different from the current state. - if (!is(newState, hook.memoizedState)) { - markWorkInProgressReceivedUpdate(); - } - - hook.memoizedState = newState; - // Don't persist the state accumulated from the render phase updates to - // the base state unless the queue is empty. - // TODO: Not sure if this is the desired semantics, but it's what we - // do for gDSFP. I can't remember why. - if (hook.baseQueue === null) { - hook.baseState = newState; - } - - queue.lastRenderedState = newState; - - return [newState, dispatch]; - } - } - return [hook.memoizedState, dispatch]; - } - const current: Hook = (currentHook: any); // The last rebase update that is NOT part of the base state. @@ -735,13 +705,17 @@ function updateReducer( let newBaseQueueFirst = null; let newBaseQueueLast = null; let update = first; + let lastProcessedTransitionTime = NoWork; + let lastSkippedTransitionTime = NoWork; do { + const suspenseConfig = update.suspenseConfig; const updateExpirationTime = update.expirationTime; if (updateExpirationTime < renderExpirationTime) { // 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: update.eventTime, expirationTime: update.expirationTime, suspenseConfig: update.suspenseConfig, action: update.action, @@ -760,11 +734,23 @@ function updateReducer( currentlyRenderingFiber.expirationTime = updateExpirationTime; markUnprocessedUpdateTime(updateExpirationTime); } + + if (suspenseConfig !== null) { + // This update is part of a transition + if ( + lastSkippedTransitionTime === NoWork || + lastSkippedTransitionTime > updateExpirationTime + ) { + lastSkippedTransitionTime = updateExpirationTime; + } + } } else { // This update does have sufficient priority. + const eventTime = update.eventTime; if (newBaseQueueLast !== null) { const clone: Update = { + eventTime, expirationTime: Sync, // This update is going to be committed so we never want uncommit it. suspenseConfig: update.suspenseConfig, action: update.action, @@ -781,10 +767,7 @@ function updateReducer( // 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( - updateExpirationTime, - update.suspenseConfig, - ); + markRenderEventTimeAndConfig(eventTime, suspenseConfig); // Process this update. if (update.eagerReducer === reducer) { @@ -795,10 +778,31 @@ function updateReducer( const action = update.action; newState = reducer(newState, action); } + + if (suspenseConfig !== null) { + // This update is part of a transition + if ( + lastProcessedTransitionTime === NoWork || + lastProcessedTransitionTime > updateExpirationTime + ) { + lastProcessedTransitionTime = updateExpirationTime; + } + } } update = update.next; } while (update !== null && update !== first); + if ( + preventIntermediateStates && + lastProcessedTransitionTime !== NoWork && + lastSkippedTransitionTime !== NoWork + ) { + // There are multiple updates scheduled on this queue, but only some of + // them were processed. To avoid showing an intermediate state, abort + // the current render and restart at a level that includes them all. + throw new SuspendOnTask(lastSkippedTransitionTime); + } + if (newBaseQueueLast === null) { newBaseState = newState; } else { @@ -822,6 +826,60 @@ function updateReducer( return [hook.memoizedState, dispatch]; } +function rerenderReducer( + reducer: (S, A) => S, + initialArg: I, + init?: I => S, +): [S, Dispatch] { + const hook = updateWorkInProgressHook(); + const queue = hook.queue; + invariant( + queue !== null, + 'Should have a queue. This is likely a bug in React. Please file an issue.', + ); + + queue.lastRenderedReducer = reducer; + + // This is a re-render. Apply the new render phase updates to the previous + // work-in-progress hook. + const dispatch: Dispatch = (queue.dispatch: any); + const lastRenderPhaseUpdate = queue.pending; + let newState = hook.memoizedState; + if (lastRenderPhaseUpdate !== null) { + // The queue doesn't persist past this render pass. + queue.pending = null; + + const firstRenderPhaseUpdate = lastRenderPhaseUpdate.next; + let update = firstRenderPhaseUpdate; + do { + // Process this render phase update. We don't have to check the + // priority because it will always be the same as the current + // render's. + const action = update.action; + newState = reducer(newState, action); + update = update.next; + } while (update !== firstRenderPhaseUpdate); + + // Mark that the fiber performed work, but only if the new state is + // different from the current state. + if (!is(newState, hook.memoizedState)) { + markWorkInProgressReceivedUpdate(); + } + + hook.memoizedState = newState; + // Don't persist the state accumulated from the render phase updates to + // the base state unless the queue is empty. + // TODO: Not sure if this is the desired semantics, but it's what we + // do for gDSFP. I can't remember why. + if (hook.baseQueue === null) { + hook.baseState = newState; + } + + queue.lastRenderedState = newState; + } + return [newState, dispatch]; +} + function mountState( initialState: (() => S) | S, ): [S, Dispatch>] { @@ -833,6 +891,7 @@ function mountState( const queue = (hook.queue = { pending: null, dispatch: null, + pendingTransition: null, lastRenderedReducer: basicStateReducer, lastRenderedState: (initialState: any), }); @@ -852,6 +911,12 @@ function updateState( return updateReducer(basicStateReducer, (initialState: any)); } +function rerenderState( + initialState: (() => S) | S, +): [S, Dispatch>] { + return rerenderReducer(basicStateReducer, (initialState: any)); +} + function pushEffect(tag, create, destroy, deps) { const effect: Effect = { tag, @@ -1165,49 +1230,145 @@ function updateDeferredValue( return prevValue; } -function startTransition(setPending, config, callback) { - const priorityLevel = getCurrentPriorityLevel(); - runWithPriority( - priorityLevel < UserBlockingPriority ? UserBlockingPriority : priorityLevel, - () => { - setPending(true); - }, - ); - runWithPriority( - priorityLevel > NormalPriority ? NormalPriority : priorityLevel, - () => { - const previousConfig = ReactCurrentBatchConfig.suspense; - ReactCurrentBatchConfig.suspense = config === undefined ? null : config; - try { - setPending(false); - callback(); - } finally { - ReactCurrentBatchConfig.suspense = previousConfig; - } - }, - ); +function rerenderDeferredValue( + value: T, + config: TimeoutConfig | void | null, +): T { + const [prevValue, setValue] = rerenderState(value); + updateEffect(() => { + const previousConfig = ReactCurrentBatchConfig.suspense; + ReactCurrentBatchConfig.suspense = config === undefined ? null : config; + try { + setValue(value); + } finally { + ReactCurrentBatchConfig.suspense = previousConfig; + } + }, [value, config]); + return prevValue; } function mountTransition( config: SuspenseConfig | void | null, ): [(() => void) => void, boolean] { - const [isPending, setPending] = mountState(false); - const start = mountCallback(startTransition.bind(null, setPending, config), [ - setPending, + const hook = mountWorkInProgressHook(); + const fiber = ((currentlyRenderingFiber: any): Fiber); + const instance: TransitionInstance = { + pendingExpirationTime: NoWork, + fiber, + }; + // TODO: Intentionally storing this on the queue field to avoid adding a new/ + // one; `queue` should be a union. + hook.queue = (instance: any); + + const isPending = false; + + // TODO: Consider passing `config` to `startTransition` instead of the hook. + // Then we don't have to recompute the callback whenever it changes. However, + // if we don't end up changing the API, we should at least optimize this + // to use the same hook instead of a separate hook just for the callback. + const start = mountCallback(startTransition.bind(null, instance, config), [ config, ]); + + const resolvedExpirationTime = NoWork; + hook.memoizedState = { + isPending, + + // Represents the last processed expiration time. + resolvedExpirationTime, + }; + return [start, isPending]; } function updateTransition( config: SuspenseConfig | void | null, ): [(() => void) => void, boolean] { - const [isPending, setPending] = updateState(false); - const start = updateCallback(startTransition.bind(null, setPending, config), [ - setPending, + const hook = updateWorkInProgressHook(); + + const instance: TransitionInstance = (hook.queue: any); + + const pendingExpirationTime = instance.pendingExpirationTime; + const oldState = hook.memoizedState; + const oldIsPending = oldState.isPending; + const oldResolvedExpirationTime = oldState.resolvedExpirationTime; + + // Check if the most recent transition is pending. The following logic is + // a little confusing, but it conceptually maps to same logic used to process + // state update queues (see: updateReducer). We're cheating a bit because + // we know that there is only ever a single pending transition, and the last + // one always wins. So we don't need to maintain an actual queue of updates; + // we only need to track 1) which is the most recent pending level 2) did + // we already resolve + // + // Note: This could be even simpler if we used a commit effect to mark when a + // pending transition is resolved. The cleverness that follows is meant to + // avoid the overhead of an extra effect; however, if this ends up being *too* + // clever, an effect probably isn't that bad, since it would only fire once + // per transition. + let newIsPending; + let newResolvedExpirationTime; + + if (pendingExpirationTime === NoWork) { + // There are no pending transitions. Reset all fields. + newIsPending = false; + newResolvedExpirationTime = NoWork; + } else { + // There is a pending transition. It may or may not have resolved. Compare + // the time at which we last resolved to the pending time. If the pending + // time is in the future, then we're still pending. + if ( + oldResolvedExpirationTime === NoWork || + oldResolvedExpirationTime > pendingExpirationTime + ) { + // We have not already resolved at the pending time. Check if this render + // includes the pending level. + if (renderExpirationTime <= pendingExpirationTime) { + // This render does include the pending level. Mark it as resolved. + newIsPending = false; + newResolvedExpirationTime = renderExpirationTime; + } else { + // This render does not include the pending level. Still pending. + newIsPending = true; + newResolvedExpirationTime = oldResolvedExpirationTime; + + // Mark that there's still pending work on this queue + if (pendingExpirationTime > currentlyRenderingFiber.expirationTime) { + currentlyRenderingFiber.expirationTime = pendingExpirationTime; + markUnprocessedUpdateTime(pendingExpirationTime); + } + } + } else { + // Already resolved at this expiration time. + newIsPending = false; + newResolvedExpirationTime = oldResolvedExpirationTime; + } + } + + if (newIsPending !== oldIsPending) { + markWorkInProgressReceivedUpdate(); + } else if (oldIsPending === false) { + // This is a trick to mutate the instance without a commit effect. If + // neither the current nor work-in-progress hook are pending, and there's no + // pending transition at a lower priority (which we know because there can + // only be one pending level per useTransition hook), then we can be certain + // there are no pending transitions even if this render does not finish. + // It's similar to the trick we use for eager setState bailouts. Like that + // optimization, this should have no semantic effect. + instance.pendingExpirationTime = NoWork; + newResolvedExpirationTime = NoWork; + } + + hook.memoizedState = { + isPending: newIsPending, + resolvedExpirationTime: newResolvedExpirationTime, + }; + + const start = updateCallback(startTransition.bind(null, instance, config), [ config, ]); - return [start, isPending]; + + return [start, newIsPending]; } function dispatchAction( @@ -1215,12 +1376,6 @@ function dispatchAction( queue: UpdateQueue, action: A, ) { - invariant( - numberOfReRenders < RE_RENDER_LIMIT, - 'Too many re-renders. React limits the number of renders to prevent ' + - 'an infinite loop.', - ); - if (__DEV__) { if (typeof arguments[3] === 'function') { console.error( @@ -1231,6 +1386,53 @@ function dispatchAction( } } + const currentTime = requestCurrentTimeForUpdate(); + const suspenseConfig = requestCurrentSuspenseConfig(); + const transition = requestCurrentTransition(); + const expirationTime = computeExpirationForFiber( + currentTime, + fiber, + suspenseConfig, + ); + + const update: Update = { + eventTime: currentTime, + expirationTime, + suspenseConfig, + action, + eagerReducer: null, + eagerState: null, + next: (null: any), + }; + + if (__DEV__) { + update.priority = getCurrentPriorityLevel(); + } + + // Append the update to the end of the list. + const pending = queue.pending; + if (pending === null) { + // This is the first update. Create a circular list. + update.next = update; + } else { + update.next = pending.next; + pending.next = update; + } + queue.pending = update; + + if (transition !== null) { + const prevPendingTransition = queue.pendingTransition; + if (transition !== prevPendingTransition) { + queue.pendingTransition = transition; + if (prevPendingTransition !== null) { + // There's already a pending transition on this queue. The new + // transition supersedes the old one. Turn of the `isPending` state + // of the previous transition. + cancelPendingTransition(prevPendingTransition); + } + } + } + const alternate = fiber.alternate; if ( fiber === currentlyRenderingFiber || @@ -1240,64 +1442,9 @@ function dispatchAction( // queue -> linked list of updates. After this render pass, we'll restart // and apply the stashed updates on top of the work-in-progress hook. didScheduleRenderPhaseUpdate = true; - const update: Update = { - expirationTime: renderExpirationTime, - suspenseConfig: null, - action, - eagerReducer: null, - eagerState: null, - next: (null: any), - }; - if (__DEV__) { - update.priority = getCurrentPriorityLevel(); - } - if (renderPhaseUpdates === null) { - renderPhaseUpdates = new Map(); - } - const firstRenderPhaseUpdate = renderPhaseUpdates.get(queue); - if (firstRenderPhaseUpdate === undefined) { - renderPhaseUpdates.set(queue, update); - } else { - // Append the update to the end of the list. - let lastRenderPhaseUpdate = firstRenderPhaseUpdate; - while (lastRenderPhaseUpdate.next !== null) { - lastRenderPhaseUpdate = lastRenderPhaseUpdate.next; - } - lastRenderPhaseUpdate.next = update; - } + update.expirationTime = renderExpirationTime; + currentlyRenderingFiber.expirationTime = renderExpirationTime; } else { - const currentTime = requestCurrentTimeForUpdate(); - const suspenseConfig = requestCurrentSuspenseConfig(); - const expirationTime = computeExpirationForFiber( - currentTime, - fiber, - suspenseConfig, - ); - - const update: Update = { - expirationTime, - suspenseConfig, - action, - eagerReducer: null, - eagerState: null, - next: (null: any), - }; - - if (__DEV__) { - update.priority = getCurrentPriorityLevel(); - } - - // Append the update to the end of the list. - const pending = queue.pending; - if (pending === null) { - // This is the first update. Create a circular list. - update.next = update; - } else { - update.next = pending.next; - pending.next = update; - } - queue.pending = update; - if ( fiber.expirationTime === NoWork && (alternate === null || alternate.expirationTime === NoWork) @@ -1344,6 +1491,7 @@ function dispatchAction( warnIfNotCurrentlyActingUpdatesInDev(fiber); } } + scheduleWork(fiber, expirationTime); } } @@ -1402,11 +1550,31 @@ const HooksDispatcherOnUpdate: Dispatcher = { useTransition: updateTransition, }; +const HooksDispatcherOnRerender: Dispatcher = { + readContext, + + useCallback: updateCallback, + useContext: readContext, + useEffect: updateEffect, + useImperativeHandle: updateImperativeHandle, + useLayoutEffect: updateLayoutEffect, + useMemo: updateMemo, + useReducer: rerenderReducer, + useRef: updateRef, + useState: rerenderState, + useDebugValue: updateDebugValue, + useResponder: createDeprecatedResponderListener, + useDeferredValue: rerenderDeferredValue, + useTransition: updateTransition, +}; + let HooksDispatcherOnMountInDEV: Dispatcher | null = null; let HooksDispatcherOnMountWithHookTypesInDEV: Dispatcher | null = null; let HooksDispatcherOnUpdateInDEV: Dispatcher | null = null; +let HooksDispatcherOnRerenderInDEV: Dispatcher | null = null; let InvalidNestedHooksDispatcherOnMountInDEV: Dispatcher | null = null; let InvalidNestedHooksDispatcherOnUpdateInDEV: Dispatcher | null = null; +let InvalidNestedHooksDispatcherOnRerenderInDEV: Dispatcher | null = null; if (__DEV__) { const warnInvalidContextAccess = () => { @@ -1783,6 +1951,123 @@ if (__DEV__) { }, }; + HooksDispatcherOnRerenderInDEV = { + readContext( + context: ReactContext, + observedBits: void | number | boolean, + ): T { + return readContext(context, observedBits); + }, + + useCallback(callback: T, deps: Array | void | null): T { + currentHookNameInDev = 'useCallback'; + updateHookTypesDev(); + return updateCallback(callback, deps); + }, + useContext( + context: ReactContext, + observedBits: void | number | boolean, + ): T { + currentHookNameInDev = 'useContext'; + updateHookTypesDev(); + return readContext(context, observedBits); + }, + useEffect( + create: () => (() => void) | void, + deps: Array | void | null, + ): void { + currentHookNameInDev = 'useEffect'; + updateHookTypesDev(); + return updateEffect(create, deps); + }, + useImperativeHandle( + ref: {|current: T | null|} | ((inst: T | null) => mixed) | null | void, + create: () => T, + deps: Array | void | null, + ): void { + currentHookNameInDev = 'useImperativeHandle'; + updateHookTypesDev(); + return updateImperativeHandle(ref, create, deps); + }, + useLayoutEffect( + create: () => (() => void) | void, + deps: Array | void | null, + ): void { + currentHookNameInDev = 'useLayoutEffect'; + updateHookTypesDev(); + return updateLayoutEffect(create, deps); + }, + useMemo(create: () => T, deps: Array | void | null): T { + currentHookNameInDev = 'useMemo'; + updateHookTypesDev(); + const prevDispatcher = ReactCurrentDispatcher.current; + ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnRerenderInDEV; + try { + return updateMemo(create, deps); + } finally { + ReactCurrentDispatcher.current = prevDispatcher; + } + }, + useReducer( + reducer: (S, A) => S, + initialArg: I, + init?: I => S, + ): [S, Dispatch] { + currentHookNameInDev = 'useReducer'; + updateHookTypesDev(); + const prevDispatcher = ReactCurrentDispatcher.current; + ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnRerenderInDEV; + try { + return rerenderReducer(reducer, initialArg, init); + } finally { + ReactCurrentDispatcher.current = prevDispatcher; + } + }, + useRef(initialValue: T): {|current: T|} { + currentHookNameInDev = 'useRef'; + updateHookTypesDev(); + return updateRef(initialValue); + }, + useState( + initialState: (() => S) | S, + ): [S, Dispatch>] { + currentHookNameInDev = 'useState'; + updateHookTypesDev(); + const prevDispatcher = ReactCurrentDispatcher.current; + ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnRerenderInDEV; + try { + return rerenderState(initialState); + } finally { + ReactCurrentDispatcher.current = prevDispatcher; + } + }, + useDebugValue(value: T, formatterFn: ?(value: T) => mixed): void { + currentHookNameInDev = 'useDebugValue'; + updateHookTypesDev(); + return updateDebugValue(value, formatterFn); + }, + useResponder( + responder: ReactEventResponder, + props, + ): ReactEventResponderListener { + currentHookNameInDev = 'useResponder'; + updateHookTypesDev(); + return createDeprecatedResponderListener(responder, props); + }, + useDeferredValue(value: T, config: TimeoutConfig | void | null): T { + currentHookNameInDev = 'useDeferredValue'; + updateHookTypesDev(); + return rerenderDeferredValue(value, config); + }, + useTransition( + config: SuspenseConfig | void | null, + ): [(() => void) => void, boolean] { + currentHookNameInDev = 'useTransition'; + updateHookTypesDev(); + return updateTransition(config); + }, + }; + InvalidNestedHooksDispatcherOnMountInDEV = { readContext( context: ReactContext, @@ -2044,4 +2329,135 @@ if (__DEV__) { return updateTransition(config); }, }; + + InvalidNestedHooksDispatcherOnRerenderInDEV = { + readContext( + context: ReactContext, + observedBits: void | number | boolean, + ): T { + warnInvalidContextAccess(); + return readContext(context, observedBits); + }, + + useCallback(callback: T, deps: Array | void | null): T { + currentHookNameInDev = 'useCallback'; + warnInvalidHookAccess(); + updateHookTypesDev(); + return updateCallback(callback, deps); + }, + useContext( + context: ReactContext, + observedBits: void | number | boolean, + ): T { + currentHookNameInDev = 'useContext'; + warnInvalidHookAccess(); + updateHookTypesDev(); + return readContext(context, observedBits); + }, + useEffect( + create: () => (() => void) | void, + deps: Array | void | null, + ): void { + currentHookNameInDev = 'useEffect'; + warnInvalidHookAccess(); + updateHookTypesDev(); + return updateEffect(create, deps); + }, + useImperativeHandle( + ref: {|current: T | null|} | ((inst: T | null) => mixed) | null | void, + create: () => T, + deps: Array | void | null, + ): void { + currentHookNameInDev = 'useImperativeHandle'; + warnInvalidHookAccess(); + updateHookTypesDev(); + return updateImperativeHandle(ref, create, deps); + }, + useLayoutEffect( + create: () => (() => void) | void, + deps: Array | void | null, + ): void { + currentHookNameInDev = 'useLayoutEffect'; + warnInvalidHookAccess(); + updateHookTypesDev(); + return updateLayoutEffect(create, deps); + }, + useMemo(create: () => T, deps: Array | void | null): T { + currentHookNameInDev = 'useMemo'; + warnInvalidHookAccess(); + updateHookTypesDev(); + const prevDispatcher = ReactCurrentDispatcher.current; + ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnUpdateInDEV; + try { + return updateMemo(create, deps); + } finally { + ReactCurrentDispatcher.current = prevDispatcher; + } + }, + useReducer( + reducer: (S, A) => S, + initialArg: I, + init?: I => S, + ): [S, Dispatch] { + currentHookNameInDev = 'useReducer'; + warnInvalidHookAccess(); + updateHookTypesDev(); + const prevDispatcher = ReactCurrentDispatcher.current; + ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnUpdateInDEV; + try { + return rerenderReducer(reducer, initialArg, init); + } finally { + ReactCurrentDispatcher.current = prevDispatcher; + } + }, + useRef(initialValue: T): {|current: T|} { + currentHookNameInDev = 'useRef'; + warnInvalidHookAccess(); + updateHookTypesDev(); + return updateRef(initialValue); + }, + useState( + initialState: (() => S) | S, + ): [S, Dispatch>] { + currentHookNameInDev = 'useState'; + warnInvalidHookAccess(); + updateHookTypesDev(); + const prevDispatcher = ReactCurrentDispatcher.current; + ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnUpdateInDEV; + try { + return rerenderState(initialState); + } finally { + ReactCurrentDispatcher.current = prevDispatcher; + } + }, + useDebugValue(value: T, formatterFn: ?(value: T) => mixed): void { + currentHookNameInDev = 'useDebugValue'; + warnInvalidHookAccess(); + updateHookTypesDev(); + return updateDebugValue(value, formatterFn); + }, + useResponder( + responder: ReactEventResponder, + props, + ): ReactEventResponderListener { + currentHookNameInDev = 'useResponder'; + warnInvalidHookAccess(); + updateHookTypesDev(); + return createDeprecatedResponderListener(responder, props); + }, + useDeferredValue(value: T, config: TimeoutConfig | void | null): T { + currentHookNameInDev = 'useDeferredValue'; + warnInvalidHookAccess(); + updateHookTypesDev(); + return rerenderDeferredValue(value, config); + }, + useTransition( + config: SuspenseConfig | void | null, + ): [(() => void) => void, boolean] { + currentHookNameInDev = 'useTransition'; + warnInvalidHookAccess(); + updateHookTypesDev(); + return updateTransition(config); + }, + }; } diff --git a/packages/react-reconciler/src/ReactFiberNewContext.js b/packages/react-reconciler/src/ReactFiberNewContext.js index 510c1eb674ad1..37b683cedaf11 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.js @@ -217,7 +217,7 @@ export function propagateContextChange( if (fiber.tag === ClassComponent) { // Schedule a force update on the work-in-progress. - const update = createUpdate(renderExpirationTime, null); + const update = createUpdate(NoWork, renderExpirationTime, null); update.tag = ForceUpdate; // TODO: Because we don't have a work-in-progress, this will add the // update to the current fiber, too, which means it will persist even if diff --git a/packages/react-reconciler/src/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js index fb46524645aee..fb521cca5ae41 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.js @@ -269,7 +269,7 @@ export function updateContainer( } } - const update = createUpdate(expirationTime, suspenseConfig); + const update = createUpdate(currentTime, expirationTime, suspenseConfig); // Caution: React DevTools currently depends on this property // being called "element". update.payload = {element}; diff --git a/packages/react-reconciler/src/ReactFiberSuspenseConfig.js b/packages/react-reconciler/src/ReactFiberSuspenseConfig.js index 4dabb29c93a10..2fc5f8c0f5eeb 100644 --- a/packages/react-reconciler/src/ReactFiberSuspenseConfig.js +++ b/packages/react-reconciler/src/ReactFiberSuspenseConfig.js @@ -9,6 +9,7 @@ import ReactSharedInternals from 'shared/ReactSharedInternals'; +// TODO: Remove React.unstable_withSuspenseConfig and move this to the renderer const {ReactCurrentBatchConfig} = ReactSharedInternals; export type SuspenseConfig = {| diff --git a/packages/react-reconciler/src/ReactFiberThrow.js b/packages/react-reconciler/src/ReactFiberThrow.js index f56b1a04b33a3..6f7b0766751c3 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.js +++ b/packages/react-reconciler/src/ReactFiberThrow.js @@ -57,16 +57,22 @@ import { checkForWrongSuspensePriorityInDEV, } from './ReactFiberWorkLoop'; -import {Sync} from './ReactFiberExpirationTime'; +import {Sync, NoWork} from './ReactFiberExpirationTime'; const PossiblyWeakMap = typeof WeakMap === 'function' ? WeakMap : Map; +// Throw an object with this type to abort the current render and restart at +// a different level. +export function SuspendOnTask(expirationTime: ExpirationTime) { + this.retryTime = expirationTime; +} + function createRootErrorUpdate( fiber: Fiber, errorInfo: CapturedValue, expirationTime: ExpirationTime, ): Update { - const update = createUpdate(expirationTime, null); + const update = createUpdate(NoWork, expirationTime, null); // Unmount the root by rendering null. update.tag = CaptureUpdate; // Caution: React DevTools currently depends on this property @@ -85,7 +91,7 @@ function createClassErrorUpdate( errorInfo: CapturedValue, expirationTime: ExpirationTime, ): Update { - const update = createUpdate(expirationTime, null); + const update = createUpdate(NoWork, expirationTime, null); update.tag = CaptureUpdate; const getDerivedStateFromError = fiber.type.getDerivedStateFromError; if (typeof getDerivedStateFromError === 'function') { @@ -261,7 +267,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(Sync, null); + const update = createUpdate(NoWork, Sync, null); update.tag = ForceUpdate; enqueueUpdate(sourceFiber, update); } diff --git a/packages/react-reconciler/src/ReactFiberTransition.js b/packages/react-reconciler/src/ReactFiberTransition.js new file mode 100644 index 0000000000000..2a9a07b392083 --- /dev/null +++ b/packages/react-reconciler/src/ReactFiberTransition.js @@ -0,0 +1,115 @@ +/** + * 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 {Fiber} from './ReactFiber'; +import type {ExpirationTime} from './ReactFiberExpirationTime'; +import type {SuspenseConfig} from './ReactFiberSuspenseConfig'; + +import ReactSharedInternals from 'shared/ReactSharedInternals'; + +import { + UserBlockingPriority, + NormalPriority, + runWithPriority, + getCurrentPriorityLevel, +} from './SchedulerWithReactIntegration'; +import { + scheduleUpdateOnFiber, + computeExpirationForFiber, + requestCurrentTimeForUpdate, +} from './ReactFiberWorkLoop'; +import {NoWork} from './ReactFiberExpirationTime'; + +const {ReactCurrentBatchConfig} = ReactSharedInternals; + +export type TransitionInstance = {| + pendingExpirationTime: ExpirationTime, + fiber: Fiber, +|}; + +// Inside `startTransition`, this is the transition instance that corresponds to +// the `useTransition` hook. +let currentTransition: TransitionInstance | null = null; + +// Inside `startTransition`, this is the expiration time of the update that +// turns on `isPending`. We also use it to turn off the `isPending` of previous +// transitions, if they exists. +let userBlockingExpirationTime = NoWork; + +export function requestCurrentTransition(): TransitionInstance | null { + return currentTransition; +} + +export function startTransition( + transitionInstance: TransitionInstance, + config: SuspenseConfig | null | void, + callback: () => void, +) { + const fiber = transitionInstance.fiber; + + const resolvedConfig: SuspenseConfig | null = + config === undefined ? null : config; + + const currentTime = requestCurrentTimeForUpdate(); + + // TODO: runWithPriority shouldn't be necessary here. React should manage its + // own concept of priority, and only consult Scheduler for updates that are + // scheduled from outside a React context. + const priorityLevel = getCurrentPriorityLevel(); + runWithPriority( + priorityLevel < UserBlockingPriority ? UserBlockingPriority : priorityLevel, + () => { + userBlockingExpirationTime = computeExpirationForFiber( + currentTime, + fiber, + null, + ); + scheduleUpdateOnFiber(fiber, userBlockingExpirationTime); + }, + ); + runWithPriority( + priorityLevel > NormalPriority ? NormalPriority : priorityLevel, + () => { + let expirationTime = computeExpirationForFiber( + currentTime, + fiber, + resolvedConfig, + ); + // Set the expiration time at which the pending transition will finish. + // Because there's only a single transition per useTransition hook, we + // don't need a queue here; we can cheat by only tracking the most + // recently scheduled transition. + const oldPendingExpirationTime = transitionInstance.pendingExpirationTime; + if (oldPendingExpirationTime === expirationTime) { + expirationTime -= 1; + } + transitionInstance.pendingExpirationTime = expirationTime; + + scheduleUpdateOnFiber(fiber, expirationTime); + const previousConfig = ReactCurrentBatchConfig.suspense; + const previousTransition = currentTransition; + ReactCurrentBatchConfig.suspense = resolvedConfig; + currentTransition = transitionInstance; + try { + callback(); + } finally { + ReactCurrentBatchConfig.suspense = previousConfig; + currentTransition = previousTransition; + userBlockingExpirationTime = NoWork; + } + }, + ); +} + +export function cancelPendingTransition(prevTransition: TransitionInstance) { + // Turn off the `isPending` state of the previous transition, at the same + // priority we use to turn on the `isPending` state of the current transition. + prevTransition.pendingExpirationTime = NoWork; + scheduleUpdateOnFiber(prevTransition.fiber, userBlockingExpirationTime); +} diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index cb9de0ba4a884..ecc6ff94d134f 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -114,9 +114,9 @@ import { expirationTimeToMs, computeInteractiveExpiration, computeAsyncExpiration, - computeSuspenseExpiration, - inferPriorityFromExpirationTime, + computeSuspenseTimeout, LOW_PRIORITY_EXPIRATION, + inferPriorityFromExpirationTime, Batched, Idle, } from './ReactFiberExpirationTime'; @@ -127,6 +127,7 @@ import { throwException, createRootErrorUpdate, createClassErrorUpdate, + SuspendOnTask, } from './ReactFiberThrow'; import { commitBeforeMutationLifeCycles as commitBeforeMutationEffectOnFiber, @@ -141,7 +142,7 @@ import { } from './ReactFiberCommitWork'; import {enqueueUpdate} from './ReactUpdateQueue'; import {resetContextDependencies} from './ReactFiberNewContext'; -import {resetHooks, ContextOnlyDispatcher} from './ReactFiberHooks'; +import {resetHooksAfterThrow, ContextOnlyDispatcher} from './ReactFiberHooks'; import {createCapturedValue} from './ReactCapturedValue'; import { @@ -182,6 +183,7 @@ import { clearCaughtError, } from 'shared/ReactErrorUtils'; import {onCommitRoot} from './ReactFiberDevToolsHook'; +import {requestCurrentTransition} from './ReactFiberTransition'; const ceil = Math.ceil; @@ -201,13 +203,14 @@ const LegacyUnbatchedContext = /* */ 0b001000; const RenderContext = /* */ 0b010000; const CommitContext = /* */ 0b100000; -type RootExitStatus = 0 | 1 | 2 | 3 | 4 | 5; +type RootExitStatus = 0 | 1 | 2 | 3 | 4 | 5 | 6; const RootIncomplete = 0; const RootFatalErrored = 1; -const RootErrored = 2; -const RootSuspended = 3; -const RootSuspendedWithDelay = 4; -const RootCompleted = 5; +const RootSuspendedOnTask = 2; +const RootErrored = 3; +const RootSuspended = 4; +const RootSuspendedWithDelay = 5; +const RootCompleted = 6; export type Thenable = { then(resolve: () => mixed, reject?: () => mixed): Thenable | void, @@ -232,13 +235,13 @@ let workInProgressRootFatalError: mixed = null; // This is conceptually a time stamp but expressed in terms of an ExpirationTime // because we deal mostly with expiration times in the hot path, so this avoids // the conversion happening in the hot path. -let workInProgressRootLatestProcessedExpirationTime: ExpirationTime = Sync; +let workInProgressRootLatestProcessedEventTime: 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; - +let workInProgressRootRestartTime: 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. @@ -331,11 +334,13 @@ export function computeExpirationForFiber( let expirationTime; if (suspenseConfig !== null) { - // Compute an expiration time based on the Suspense timeout. - expirationTime = computeSuspenseExpiration( - currentTime, - suspenseConfig.timeoutMs | 0 || LOW_PRIORITY_EXPIRATION, - ); + // This is a transition + const transitionInstance = requestCurrentTransition(); + if (transitionInstance !== null) { + expirationTime = transitionInstance.pendingExpirationTime; + } else { + expirationTime = computeAsyncExpiration(currentTime); + } } else { // Compute an expiration time based on the Scheduler priority. switch (priorityLevel) { @@ -708,7 +713,12 @@ function performConcurrentWorkOnRoot(root, didTimeout) { throw fatalError; } - if (workInProgress !== null) { + if (workInProgressRootExitStatus === RootSuspendedOnTask) { + // Can't finish rendering at this level. Exit early and restart at the + // specified time. + markRootSuspendedAtTime(root, expirationTime); + root.nextKnownPendingLevel = workInProgressRootRestartTime; + } else if (workInProgress !== null) { // There's still work left over. Exit without committing. stopInterruptedWorkLoopTimer(); } else { @@ -749,7 +759,8 @@ function finishConcurrentRender( switch (exitStatus) { case RootIncomplete: - case RootFatalErrored: { + case RootFatalErrored: + case RootSuspendedOnTask: { invariant(false, 'Root did not complete. This is a bug in React.'); } // Flow knows about invariant, so it complains if I add a break @@ -786,7 +797,7 @@ function finishConcurrentRender( // have a new loading state ready. We want to ensure that we commit // that as soon as possible. const hasNotProcessedNewUpdates = - workInProgressRootLatestProcessedExpirationTime === Sync; + workInProgressRootLatestProcessedEventTime === Sync; if ( hasNotProcessedNewUpdates && // do not delay if we're inside an act() scope @@ -898,34 +909,19 @@ function finishConcurrentRender( // can use as the timeout. msUntilTimeout = expirationTimeToMs(workInProgressRootLatestSuspenseTimeout) - now(); - } else if (workInProgressRootLatestProcessedExpirationTime === Sync) { + } else if (workInProgressRootLatestProcessedEventTime === Sync) { // This should never normally happen because only new updates // cause delayed states, so we should have processed something. // However, this could also happen in an offscreen tree. msUntilTimeout = 0; } else { - // If we don't have a suspense config, we're going to use a - // heuristic to determine how long we can suspend. - const eventTimeMs: number = inferTimeFromExpirationTime( - workInProgressRootLatestProcessedExpirationTime, + // If we didn't process a suspense config, compute a JND based on + // the amount of time elapsed since the most recent event time. + const eventTimeMs = expirationTimeToMs( + workInProgressRootLatestProcessedEventTime, ); - const currentTimeMs = now(); - const timeUntilExpirationMs = - expirationTimeToMs(expirationTime) - currentTimeMs; - let timeElapsed = currentTimeMs - eventTimeMs; - if (timeElapsed < 0) { - // We get this wrong some time since we estimate the time. - timeElapsed = 0; - } - - msUntilTimeout = jnd(timeElapsed) - timeElapsed; - - // Clamp the timeout to the expiration time. TODO: Once the - // event time is exact instead of inferred from expiration time - // we don't need this. - if (timeUntilExpirationMs < msUntilTimeout) { - msUntilTimeout = timeUntilExpirationMs; - } + const timeElapsedMs = now() - eventTimeMs; + msUntilTimeout = jnd(timeElapsedMs) - timeElapsedMs; } // Don't bother with a very short suspense time. @@ -953,7 +949,7 @@ function finishConcurrentRender( flushSuspenseFallbacksInTests && IsThisRendererActing.current ) && - workInProgressRootLatestProcessedExpirationTime !== Sync && + workInProgressRootLatestProcessedEventTime !== Sync && workInProgressRootCanSuspendUsingConfig !== null ) { // If we have exceeded the minimum loading delay, which probably @@ -961,7 +957,7 @@ function finishConcurrentRender( // a bit longer to ensure that the spinner is shown for // enough time. const msUntilTimeout = computeMsUntilSuspenseLoadingDelay( - workInProgressRootLatestProcessedExpirationTime, + workInProgressRootLatestProcessedEventTime, expirationTime, workInProgressRootCanSuspendUsingConfig, ); @@ -1036,7 +1032,12 @@ function performSyncWorkOnRoot(root) { throw fatalError; } - if (workInProgress !== null) { + if (workInProgressRootExitStatus === RootSuspendedOnTask) { + // Can't finish rendering at this level. Exit early and restart at the + // specified time. + markRootSuspendedAtTime(root, expirationTime); + root.nextKnownPendingLevel = workInProgressRootRestartTime; + } else if (workInProgress !== null) { // This is a sync render, so we should have finished the whole tree. invariant( false, @@ -1260,10 +1261,11 @@ function prepareFreshStack(root, expirationTime) { renderExpirationTime = expirationTime; workInProgressRootExitStatus = RootIncomplete; workInProgressRootFatalError = null; - workInProgressRootLatestProcessedExpirationTime = Sync; + workInProgressRootLatestProcessedEventTime = Sync; workInProgressRootLatestSuspenseTimeout = Sync; workInProgressRootCanSuspendUsingConfig = null; workInProgressRootNextUnprocessedUpdateTime = NoWork; + workInProgressRootRestartTime = NoWork; workInProgressRootHasPendingPing = false; if (enableSchedulerTracing) { @@ -1281,9 +1283,23 @@ function handleError(root, thrownValue) { try { // Reset module-level state that was set during the render phase. resetContextDependencies(); - resetHooks(); + resetHooksAfterThrow(); resetCurrentDebugFiberInDEV(); + // Check if this is a SuspendOnTask exception. This is the one type of + // exception that is allowed to happen at the root. + // TODO: I think instanceof is OK here? A brand check seems unnecessary + // since this is always thrown by the renderer and not across realms + // or packages. + if (thrownValue instanceof SuspendOnTask) { + // Can't finish rendering at this level. Exit early and restart at + // the specified time. + workInProgressRootExitStatus = RootSuspendedOnTask; + workInProgressRootRestartTime = thrownValue.retryTime; + workInProgress = null; + return; + } + if (workInProgress === null || workInProgress.return === null) { // Expected to be working on a non-root fiber. This is a fatal error // because there's no ancestor that can handle it; the root is @@ -1356,23 +1372,30 @@ export function markCommitTimeOfFallback() { } export function markRenderEventTimeAndConfig( - expirationTime: ExpirationTime, + eventTime: ExpirationTime, suspenseConfig: null | SuspenseConfig, ): void { - if ( - expirationTime < workInProgressRootLatestProcessedExpirationTime && - expirationTime > Idle - ) { - workInProgressRootLatestProcessedExpirationTime = expirationTime; - } - if (suspenseConfig !== null) { - if ( - expirationTime < workInProgressRootLatestSuspenseTimeout && - expirationTime > Idle - ) { - workInProgressRootLatestSuspenseTimeout = expirationTime; - // Most of the time we only have one config and getting wrong is not bad. - workInProgressRootCanSuspendUsingConfig = suspenseConfig; + // Anything lower pri than Idle is not an update, so we should skip it. + if (eventTime > Idle) { + // Track the most recent event time of all updates processed in this batch. + if (workInProgressRootLatestProcessedEventTime > eventTime) { + workInProgressRootLatestProcessedEventTime = eventTime; + } + + // 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. + // TODO: Should this default to a JND instead? + const timeoutMs = suspenseConfig.timeoutMs | 0 || LOW_PRIORITY_EXPIRATION; + const timeoutTime = computeSuspenseTimeout(eventTime, timeoutMs); + if (timeoutTime < workInProgressRootLatestSuspenseTimeout) { + workInProgressRootLatestSuspenseTimeout = timeoutTime; + workInProgressRootCanSuspendUsingConfig = suspenseConfig; + } } } } @@ -1430,27 +1453,6 @@ export function renderHasNotSuspendedYet(): boolean { return workInProgressRootExitStatus === RootIncomplete; } -function inferTimeFromExpirationTime(expirationTime: ExpirationTime): number { - // We don't know exactly when the update was scheduled, but we can infer an - // approximate start time from the expiration time. - const earliestExpirationTimeMs = expirationTimeToMs(expirationTime); - return earliestExpirationTimeMs - LOW_PRIORITY_EXPIRATION; -} - -function inferTimeFromExpirationTimeWithSuspenseConfig( - expirationTime: ExpirationTime, - suspenseConfig: SuspenseConfig, -): number { - // We don't know exactly when the update was scheduled, but we can infer an - // approximate start time from the expiration time by subtracting the timeout - // that was added to the event time. - const earliestExpirationTimeMs = expirationTimeToMs(expirationTime); - return ( - earliestExpirationTimeMs - - (suspenseConfig.timeoutMs | 0 || LOW_PRIORITY_EXPIRATION) - ); -} - // The work loop is an extremely hot path. Tell Closure not to inline it. /** @noinline */ function workLoopSync() { @@ -2346,7 +2348,7 @@ export function pingSuspendedRoot( if ( workInProgressRootExitStatus === RootSuspendedWithDelay || (workInProgressRootExitStatus === RootSuspended && - workInProgressRootLatestProcessedExpirationTime === Sync && + workInProgressRootLatestProcessedEventTime === Sync && now() - globalMostRecentFallbackTime < FALLBACK_THROTTLE_MS) ) { // Restart from the root. Don't need to schedule a ping because @@ -2491,10 +2493,7 @@ function computeMsUntilSuspenseLoadingDelay( // Compute the time until this render pass would expire. const currentTimeMs: number = now(); - const eventTimeMs: number = inferTimeFromExpirationTimeWithSuspenseConfig( - mostRecentEventTime, - suspenseConfig, - ); + const eventTimeMs: number = expirationTimeToMs(mostRecentEventTime); const timeElapsed = currentTimeMs - eventTimeMs; if (timeElapsed <= busyDelayMs) { // If we haven't yet waited longer than the initial delay, we don't @@ -2624,19 +2623,21 @@ if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) { try { return originalBeginWork(current, unitOfWork, expirationTime); } catch (originalError) { + // Filter out special exception types if ( originalError !== null && typeof originalError === 'object' && - typeof originalError.then === 'function' + // Promise + (typeof originalError.then === 'function' || + // SuspendOnTask exception + originalError instanceof SuspendOnTask) ) { - // Don't replay promises. Treat everything else like an error. throw originalError; } - // Keep this code in sync with handleError; any changes here must have // corresponding changes there. resetContextDependencies(); - resetHooks(); + resetHooksAfterThrow(); // Don't reset current debug fiber, since we're about to work on the // same fiber again. diff --git a/packages/react-reconciler/src/ReactUpdateQueue.js b/packages/react-reconciler/src/ReactUpdateQueue.js index dba13e3cf2a38..70908de85325a 100644 --- a/packages/react-reconciler/src/ReactUpdateQueue.js +++ b/packages/react-reconciler/src/ReactUpdateQueue.js @@ -87,6 +87,7 @@ import type {Fiber} from './ReactFiber'; import type {ExpirationTime} from './ReactFiberExpirationTime'; import type {SuspenseConfig} from './ReactFiberSuspenseConfig'; +import type {TransitionInstance} from './ReactFiberTransition'; import type {ReactPriorityLevel} from './SchedulerWithReactIntegration'; import {NoWork, Sync} from './ReactFiberExpirationTime'; @@ -96,18 +97,30 @@ import { } from './ReactFiberNewContext'; import {Callback, ShouldCapture, DidCapture} from 'shared/ReactSideEffectTags'; -import {debugRenderPhaseSideEffectsForStrictMode} from 'shared/ReactFeatureFlags'; +import { + debugRenderPhaseSideEffectsForStrictMode, + preventIntermediateStates, +} from 'shared/ReactFeatureFlags'; import {StrictMode} from './ReactTypeOfMode'; import { markRenderEventTimeAndConfig, markUnprocessedUpdateTime, } from './ReactFiberWorkLoop'; +import {SuspendOnTask} from './ReactFiberThrow'; import invariant from 'shared/invariant'; import {getCurrentPriorityLevel} from './SchedulerWithReactIntegration'; +import { + requestCurrentTransition, + cancelPendingTransition, +} from './ReactFiberTransition'; + export type Update = {| + // TODO: Temporary field. Will remove this by storing a map of + // transition -> event time on the root. + eventTime: ExpirationTime, expirationTime: ExpirationTime, suspenseConfig: null | SuspenseConfig, @@ -121,7 +134,10 @@ export type Update = {| priority?: ReactPriorityLevel, |}; -type SharedQueue = {|pending: Update | null|}; +type SharedQueue = {| + pending: Update | null, + pendingTransition: TransitionInstance | null, +|}; export type UpdateQueue = {| baseState: State, @@ -157,6 +173,7 @@ export function initializeUpdateQueue(fiber: Fiber): void { baseQueue: null, shared: { pending: null, + pendingTransition: null, }, effects: null, }; @@ -182,10 +199,12 @@ export function cloneUpdateQueue( } export function createUpdate( + eventTime: ExpirationTime, expirationTime: ExpirationTime, suspenseConfig: null | SuspenseConfig, ): Update<*> { let update: Update<*> = { + eventTime, expirationTime, suspenseConfig, @@ -220,6 +239,20 @@ export function enqueueUpdate(fiber: Fiber, update: Update) { } sharedQueue.pending = update; + const transition = requestCurrentTransition(); + if (transition !== null) { + const prevPendingTransition = sharedQueue.pendingTransition; + if (transition !== prevPendingTransition) { + sharedQueue.pendingTransition = transition; + if (prevPendingTransition !== null) { + // There's already a pending transition on this queue. The new + // transition supersedes the old one. Turn of the `isPending` state + // of the previous transition. + cancelPendingTransition(prevPendingTransition); + } + } + } + if (__DEV__) { if ( currentlyProcessingQueue === sharedQueue && @@ -389,15 +422,19 @@ export function processUpdateQueue( if (first !== null) { let update = first; + let lastProcessedTransitionTime = NoWork; + let lastSkippedTransitionTime = NoWork; do { const updateExpirationTime = update.expirationTime; + const suspenseConfig = update.suspenseConfig; if (updateExpirationTime < renderExpirationTime) { // 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 = { - expirationTime: update.expirationTime, - suspenseConfig: update.suspenseConfig, + eventTime: update.eventTime, + expirationTime: updateExpirationTime, + suspenseConfig, tag: update.tag, payload: update.payload, @@ -415,11 +452,22 @@ export function processUpdateQueue( if (updateExpirationTime > newExpirationTime) { newExpirationTime = updateExpirationTime; } + + if (suspenseConfig !== null) { + // This update is part of a transition + if ( + lastSkippedTransitionTime === NoWork || + lastSkippedTransitionTime > updateExpirationTime + ) { + lastSkippedTransitionTime = updateExpirationTime; + } + } } else { // This update does have sufficient priority. - + const eventTime = update.eventTime; if (newBaseQueueLast !== null) { const clone: Update = { + eventTime, expirationTime: Sync, // This update is going to be committed so we never want uncommit it. suspenseConfig: update.suspenseConfig, @@ -438,10 +486,7 @@ export function processUpdateQueue( // 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( - updateExpirationTime, - update.suspenseConfig, - ); + markRenderEventTimeAndConfig(eventTime, update.suspenseConfig); // Process this update. newState = getStateFromUpdate( @@ -463,6 +508,17 @@ export function processUpdateQueue( } } } + + if (suspenseConfig !== null) { + // This update is part of a transition + if ( + lastProcessedTransitionTime === NoWork || + lastProcessedTransitionTime > updateExpirationTime + ) { + lastProcessedTransitionTime = updateExpirationTime; + } + } + update = update.next; if (update === null || update === first) { pendingQueue = queue.shared.pending; @@ -478,6 +534,17 @@ export function processUpdateQueue( } } } while (true); + + if ( + preventIntermediateStates && + lastProcessedTransitionTime !== NoWork && + lastSkippedTransitionTime !== NoWork + ) { + // There are multiple updates scheduled on this queue, but only some of + // them were processed. To avoid showing an intermediate state, abort + // the current render and restart at a level that includes them all. + throw new SuspendOnTask(lastSkippedTransitionTime); + } } if (newBaseQueueLast === null) { diff --git a/packages/react-reconciler/src/__tests__/ReactExpiration-test.internal.js b/packages/react-reconciler/src/__tests__/ReactExpiration-test.internal.js index 3c8cbbb8809b2..35a76c0c1afb4 100644 --- a/packages/react-reconciler/src/__tests__/ReactExpiration-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactExpiration-test.internal.js @@ -14,6 +14,7 @@ let ReactFeatureFlags; let ReactNoop; let Scheduler; +// TODO: Rename to something like ReactBatching describe('ReactExpiration', () => { beforeEach(() => { jest.resetModules(); @@ -28,159 +29,6 @@ describe('ReactExpiration', () => { return {type: 'span', children: [], prop, hidden: false}; } - it('increases priority of updates as time progresses', () => { - ReactNoop.render(); - - expect(ReactNoop.getChildren()).toEqual([]); - - // Nothing has expired yet because time hasn't advanced. - ReactNoop.flushExpired(); - expect(ReactNoop.getChildren()).toEqual([]); - - // Advance time a bit, but not enough to expire the low pri update. - ReactNoop.expire(4500); - ReactNoop.flushExpired(); - expect(ReactNoop.getChildren()).toEqual([]); - - // Advance by another second. Now the update should expire and flush. - ReactNoop.expire(1000); - ReactNoop.flushExpired(); - expect(ReactNoop.getChildren()).toEqual([span('done')]); - }); - - it('two updates of like priority in the same event always flush within the same batch', () => { - class Text extends React.Component { - componentDidMount() { - Scheduler.unstable_yieldValue(`${this.props.text} [commit]`); - } - componentDidUpdate() { - Scheduler.unstable_yieldValue(`${this.props.text} [commit]`); - } - render() { - Scheduler.unstable_yieldValue(`${this.props.text} [render]`); - return ; - } - } - - function interrupt() { - ReactNoop.flushSync(() => { - ReactNoop.renderToRootWithID(null, 'other-root'); - }); - } - - // First, show what happens for updates in two separate events. - // Schedule an update. - ReactNoop.render(); - // Advance the timer. - Scheduler.unstable_advanceTime(2000); - // Partially flush the the first update, then interrupt it. - expect(Scheduler).toFlushAndYieldThrough(['A [render]']); - interrupt(); - - // Don't advance time by enough to expire the first update. - expect(Scheduler).toHaveYielded([]); - expect(ReactNoop.getChildren()).toEqual([]); - - // Schedule another update. - ReactNoop.render(); - // The updates should flush in separate batches, since sufficient time - // passed in between them *and* they occurred in separate events. - // Note: This isn't necessarily the ideal behavior. It might be better to - // batch these two updates together. The fact that they aren't batched - // is an implementation detail. The important part of this unit test is that - // they are batched if it's possible that they happened in the same event. - expect(Scheduler).toFlushAndYield([ - 'A [render]', - 'A [commit]', - 'B [render]', - 'B [commit]', - ]); - expect(ReactNoop.getChildren()).toEqual([span('B')]); - - // Now do the same thing again, except this time don't flush any work in - // between the two updates. - ReactNoop.render(); - Scheduler.unstable_advanceTime(2000); - expect(Scheduler).toHaveYielded([]); - expect(ReactNoop.getChildren()).toEqual([span('B')]); - // Schedule another update. - ReactNoop.render(); - // The updates should flush in the same batch, since as far as the scheduler - // knows, they may have occurred inside the same event. - expect(Scheduler).toFlushAndYield(['B [render]', 'B [commit]']); - }); - - it( - 'two updates of like priority in the same event always flush within the ' + - "same batch, even if there's a sync update in between", - () => { - class Text extends React.Component { - componentDidMount() { - Scheduler.unstable_yieldValue(`${this.props.text} [commit]`); - } - componentDidUpdate() { - Scheduler.unstable_yieldValue(`${this.props.text} [commit]`); - } - render() { - Scheduler.unstable_yieldValue(`${this.props.text} [render]`); - return ; - } - } - - function interrupt() { - ReactNoop.flushSync(() => { - ReactNoop.renderToRootWithID(null, 'other-root'); - }); - } - - // First, show what happens for updates in two separate events. - // Schedule an update. - ReactNoop.render(); - // Advance the timer. - Scheduler.unstable_advanceTime(2000); - // Partially flush the the first update, then interrupt it. - expect(Scheduler).toFlushAndYieldThrough(['A [render]']); - interrupt(); - - // Don't advance time by enough to expire the first update. - expect(Scheduler).toHaveYielded([]); - expect(ReactNoop.getChildren()).toEqual([]); - - // Schedule another update. - ReactNoop.render(); - // The updates should flush in separate batches, since sufficient time - // passed in between them *and* they occurred in separate events. - // Note: This isn't necessarily the ideal behavior. It might be better to - // batch these two updates together. The fact that they aren't batched - // is an implementation detail. The important part of this unit test is that - // they are batched if it's possible that they happened in the same event. - expect(Scheduler).toFlushAndYield([ - 'A [render]', - 'A [commit]', - 'B [render]', - 'B [commit]', - ]); - expect(ReactNoop.getChildren()).toEqual([span('B')]); - - // Now do the same thing again, except this time don't flush any work in - // between the two updates. - ReactNoop.render(); - Scheduler.unstable_advanceTime(2000); - expect(Scheduler).toHaveYielded([]); - expect(ReactNoop.getChildren()).toEqual([span('B')]); - - // Perform some synchronous work. The scheduler must assume we're inside - // the same event. - interrupt(); - - // Schedule another update. - ReactNoop.render(); - // The updates should flush in the same batch, since as far as the scheduler - // knows, they may have occurred inside the same event. - expect(Scheduler).toFlushAndYield(['B [render]', 'B [commit]']); - }, - ); - it('cannot update at the same expiration time that is already rendering', () => { let store = {text: 'initial'}; let subscribers = []; @@ -273,24 +121,174 @@ describe('ReactExpiration', () => { expect(ReactNoop).toMatchRenderedOutput('Hi'); }); - it('should measure callback timeout relative to current time, not start-up time', () => { - // Corresponds to a bugfix: https://github.com/facebook/react/pull/15479 - // The bug wasn't caught by other tests because we use virtual times that - // default to 0, and most tests don't advance time. + describe.skip('old expiration times train model', () => { + it('increases priority of updates as time progresses', () => { + ReactNoop.render(); - // Before scheduling an update, advance the current time. - Scheduler.unstable_advanceTime(10000); + expect(ReactNoop.getChildren()).toEqual([]); - ReactNoop.render('Hi'); - expect(Scheduler).toFlushExpired([]); - expect(ReactNoop).toMatchRenderedOutput(null); + // Nothing has expired yet because time hasn't advanced. + ReactNoop.flushExpired(); + expect(ReactNoop.getChildren()).toEqual([]); - // Advancing by ~5 seconds should be sufficient to expire the update. (I - // used a slightly larger number to allow for possible rounding.) - Scheduler.unstable_advanceTime(6000); + // Advance time a bit, but not enough to expire the low pri update. + ReactNoop.expire(4500); + ReactNoop.flushExpired(); + expect(ReactNoop.getChildren()).toEqual([]); - ReactNoop.render('Hi'); - expect(Scheduler).toFlushExpired([]); - expect(ReactNoop).toMatchRenderedOutput('Hi'); + // Advance by another second. Now the update should expire and flush. + ReactNoop.expire(1000); + ReactNoop.flushExpired(); + expect(ReactNoop.getChildren()).toEqual([span('done')]); + }); + + it('two updates of like priority in the same event always flush within the same batch', () => { + class Text extends React.Component { + componentDidMount() { + Scheduler.unstable_yieldValue(`${this.props.text} [commit]`); + } + componentDidUpdate() { + Scheduler.unstable_yieldValue(`${this.props.text} [commit]`); + } + render() { + Scheduler.unstable_yieldValue(`${this.props.text} [render]`); + return ; + } + } + + function interrupt() { + ReactNoop.flushSync(() => { + ReactNoop.renderToRootWithID(null, 'other-root'); + }); + } + + // First, show what happens for updates in two separate events. + // Schedule an update. + ReactNoop.render(); + // Advance the timer. + Scheduler.unstable_advanceTime(2000); + // Partially flush the the first update, then interrupt it. + expect(Scheduler).toFlushAndYieldThrough(['A [render]']); + interrupt(); + + // Don't advance time by enough to expire the first update. + expect(Scheduler).toHaveYielded([]); + expect(ReactNoop.getChildren()).toEqual([]); + + // Schedule another update. + ReactNoop.render(); + // The updates should flush in separate batches, since sufficient time + // passed in between them *and* they occurred in separate events. + // Note: This isn't necessarily the ideal behavior. It might be better to + // batch these two updates together. The fact that they aren't batched + // is an implementation detail. The important part of this unit test is that + // they are batched if it's possible that they happened in the same event. + expect(Scheduler).toFlushAndYield(['B [render]', 'B [commit]']); + expect(ReactNoop.getChildren()).toEqual([span('B')]); + + // Now do the same thing again, except this time don't flush any work in + // between the two updates. + ReactNoop.render(); + Scheduler.unstable_advanceTime(2000); + expect(Scheduler).toHaveYielded([]); + expect(ReactNoop.getChildren()).toEqual([span('B')]); + // Schedule another update. + ReactNoop.render(); + // The updates should flush in the same batch, since as far as the scheduler + // knows, they may have occurred inside the same event. + expect(Scheduler).toFlushAndYield(['B [render]', 'B [commit]']); + }); + + it( + 'two updates of like priority in the same event always flush within the ' + + "same batch, even if there's a sync update in between", + () => { + class Text extends React.Component { + componentDidMount() { + Scheduler.unstable_yieldValue(`${this.props.text} [commit]`); + } + componentDidUpdate() { + Scheduler.unstable_yieldValue(`${this.props.text} [commit]`); + } + render() { + Scheduler.unstable_yieldValue(`${this.props.text} [render]`); + return ; + } + } + + function interrupt() { + ReactNoop.flushSync(() => { + ReactNoop.renderToRootWithID(null, 'other-root'); + }); + } + + // First, show what happens for updates in two separate events. + // Schedule an update. + ReactNoop.render(); + // Advance the timer. + Scheduler.unstable_advanceTime(2000); + // Partially flush the the first update, then interrupt it. + expect(Scheduler).toFlushAndYieldThrough(['A [render]']); + interrupt(); + + // Don't advance time by enough to expire the first update. + expect(Scheduler).toHaveYielded([]); + expect(ReactNoop.getChildren()).toEqual([]); + + // Schedule another update. + ReactNoop.render(); + // The updates should flush in separate batches, since sufficient time + // passed in between them *and* they occurred in separate events. + // Note: This isn't necessarily the ideal behavior. It might be better to + // batch these two updates together. The fact that they aren't batched + // is an implementation detail. The important part of this unit test is that + // they are batched if it's possible that they happened in the same event. + expect(Scheduler).toFlushAndYield([ + 'A [render]', + 'A [commit]', + 'B [render]', + 'B [commit]', + ]); + expect(ReactNoop.getChildren()).toEqual([span('B')]); + + // Now do the same thing again, except this time don't flush any work in + // between the two updates. + ReactNoop.render(); + Scheduler.unstable_advanceTime(2000); + expect(Scheduler).toHaveYielded([]); + expect(ReactNoop.getChildren()).toEqual([span('B')]); + + // Perform some synchronous work. The scheduler must assume we're inside + // the same event. + interrupt(); + + // Schedule another update. + ReactNoop.render(); + // The updates should flush in the same batch, since as far as the scheduler + // knows, they may have occurred inside the same event. + expect(Scheduler).toFlushAndYield(['B [render]', 'B [commit]']); + }, + ); + + it('should measure callback timeout relative to current time, not start-up time', () => { + // Corresponds to a bugfix: https://github.com/facebook/react/pull/15479 + // The bug wasn't caught by other tests because we use virtual times that + // default to 0, and most tests don't advance time. + + // Before scheduling an update, advance the current time. + Scheduler.unstable_advanceTime(10000); + + ReactNoop.render('Hi'); + expect(Scheduler).toFlushExpired([]); + expect(ReactNoop).toMatchRenderedOutput(null); + + // Advancing by ~5 seconds should be sufficient to expire the update. (I + // used a slightly larger number to allow for possible rounding.) + Scheduler.unstable_advanceTime(6000); + + ReactNoop.render('Hi'); + expect(Scheduler).toFlushExpired([]); + expect(ReactNoop).toMatchRenderedOutput('Hi'); + }); }); }); diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js index 8ded101a09e1d..8ba2c88305ae9 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js @@ -15,6 +15,7 @@ let React; let ReactCache; let TextResource; +let resolveText; let ReactFeatureFlags; let ReactNoop; let Scheduler; @@ -63,14 +64,27 @@ describe('ReactHooksWithNoopRenderer', () => { Suspense = React.Suspense; act = ReactNoop.act; + const textCache = new Map(); + + resolveText = text => { + const resolve = textCache.get(text); + if (resolve !== undefined) { + textCache.delete(text); + Scheduler.unstable_yieldValue(`Promise resolved [${text}]`); + resolve(); + } + }; + TextResource = ReactCache.unstable_createResource( - ([text, ms = 0]) => { - return new Promise((resolve, reject) => - setTimeout(() => { - Scheduler.unstable_yieldValue(`Promise resolved [${text}]`); - resolve(text); - }, ms), - ); + ([text, ms]) => { + return new Promise(resolve => { + textCache.set(text, resolve); + if (typeof ms === 'number') { + setTimeout(() => { + resolveText(text); + }, ms); + } + }); }, ([text, ms]) => text, ); @@ -545,6 +559,132 @@ describe('ReactHooksWithNoopRenderer', () => { ]); expect(ReactNoop.getChildren()).toEqual([span(22)]); }); + + it('discards render phase updates if something suspends', () => { + const thenable = {then() {}}; + function Foo({signal}) { + return ( + + + + ); + } + + function Bar({signal: newSignal}) { + let [counter, setCounter] = useState(0); + let [signal, setSignal] = useState(true); + + // Increment a counter every time the signal changes + if (signal !== newSignal) { + setCounter(c => c + 1); + setSignal(newSignal); + if (counter === 0) { + // We're suspending during a render that includes render phase + // updates. Those updates should not persist to the next render. + Scheduler.unstable_yieldValue('Suspend!'); + throw thenable; + } + } + + return ; + } + + const root = ReactNoop.createRoot(); + root.render(); + + expect(Scheduler).toFlushAndYield([0]); + expect(root).toMatchRenderedOutput(); + + root.render(); + expect(Scheduler).toFlushAndYield(['Suspend!']); + expect(root).toMatchRenderedOutput(); + + // Rendering again should suspend again. + root.render(); + expect(Scheduler).toFlushAndYield(['Suspend!']); + }); + + it('discards render phase updates if something suspends, but not other updates in the same component', async () => { + const thenable = {then() {}}; + function Foo({signal}) { + return ( + + + + ); + } + + let setLabel; + function Bar({signal: newSignal}) { + let [counter, setCounter] = useState(0); + + if (counter === 1) { + // We're suspending during a render that includes render phase + // updates. Those updates should not persist to the next render. + Scheduler.unstable_yieldValue('Suspend!'); + throw thenable; + } + + let [signal, setSignal] = useState(true); + + // Increment a counter every time the signal changes + if (signal !== newSignal) { + setCounter(c => c + 1); + setSignal(newSignal); + } + + let [label, _setLabel] = useState('A'); + setLabel = _setLabel; + + return ; + } + + const root = ReactNoop.createRoot(); + root.render(); + + expect(Scheduler).toFlushAndYield(['A:0']); + expect(root).toMatchRenderedOutput(); + + await ReactNoop.act(async () => { + root.render(); + setLabel('B'); + }); + expect(Scheduler).toHaveYielded(['Suspend!']); + expect(root).toMatchRenderedOutput(); + + // Rendering again should suspend again. + root.render(); + expect(Scheduler).toFlushAndYield(['Suspend!']); + + // Flip the signal back to "cancel" the update. However, the update to + // label should still proceed. It shouldn't have been dropped. + root.render(); + expect(Scheduler).toFlushAndYield(['B:0']); + expect(root).toMatchRenderedOutput(); + }); + + // TODO: This should probably warn + it.experimental('calling startTransition inside render phase', async () => { + let startTransition; + function App() { + let [counter, setCounter] = useState(0); + let [_startTransition] = useTransition(); + startTransition = _startTransition; + + if (counter === 0) { + startTransition(() => { + setCounter(c => c + 1); + }); + } + + return ; + } + + const root = ReactNoop.createRoot(); + root.render(); + expect(Scheduler).toFlushAndYield([1]); + expect(root).toMatchRenderedOutput(); + }); }); describe('useReducer', () => { @@ -2073,6 +2213,9 @@ describe('ReactHooksWithNoopRenderer', () => { span('Before... Pending: true'), ]); + // 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); expect(Scheduler).toHaveYielded([ @@ -2083,13 +2226,16 @@ describe('ReactHooksWithNoopRenderer', () => { span('Before... Pending: true'), ]); - Scheduler.unstable_advanceTime(1000); - await advanceTimers(1000); + // Advance time until just before the `busyMinDuration` threshold. + Scheduler.unstable_advanceTime(999); + await advanceTimers(999); expect(ReactNoop.getChildren()).toEqual([ span('Before... Pending: true'), ]); - Scheduler.unstable_advanceTime(250); - await advanceTimers(250); + + // Advance time just a bit more. Now we complete the transition. + Scheduler.unstable_advanceTime(1); + await advanceTimers(1); expect(ReactNoop.getChildren()).toEqual([ span('After... Pending: false'), ]); @@ -2099,7 +2245,7 @@ describe('ReactHooksWithNoopRenderer', () => { describe('useDeferredValue', () => { it.experimental('defers text value until specified timeout', async () => { function TextBox({text}) { - return ; + return ; } let _setText; @@ -2126,8 +2272,7 @@ describe('ReactHooksWithNoopRenderer', () => { expect(Scheduler).toHaveYielded(['A', 'Suspend! [A]', 'Loading']); expect(ReactNoop.getChildren()).toEqual([span('A'), span('Loading')]); - Scheduler.unstable_advanceTime(1000); - await advanceTimers(1000); + await resolveText('A'); expect(Scheduler).toHaveYielded(['Promise resolved [A]']); expect(Scheduler).toFlushAndYield(['A']); expect(ReactNoop.getChildren()).toEqual([span('A'), span('A')]); @@ -2158,8 +2303,7 @@ describe('ReactHooksWithNoopRenderer', () => { span('Loading'), ]); - Scheduler.unstable_advanceTime(250); - await advanceTimers(250); + await resolveText('B'); expect(Scheduler).toHaveYielded(['Promise resolved [B]']); act(() => { diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js index 519c2d3f3a850..fb1c581e5d9e8 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js @@ -230,18 +230,19 @@ describe('ReactIncrementalErrorHandling', () => { } function interrupt() { + // Perform a sync render on a different, abitrary root. This is a trick + // to interrupt an in-progress async render so that it's forced to + // start over. ReactNoop.flushSync(() => { ReactNoop.renderToRootWithID(null, 'other-root'); }); } ReactNoop.render(, onCommit); - Scheduler.unstable_advanceTime(1000); expect(Scheduler).toFlushAndYieldThrough(['error']); - interrupt(); - // This update is in a separate batch ReactNoop.render(, onCommit); + interrupt(); expect(Scheduler).toFlushAndYieldThrough([ // The first render fails. But because there's a lower priority pending @@ -287,6 +288,9 @@ describe('ReactIncrementalErrorHandling', () => { } function interrupt() { + // Perform a sync render on a different, abitrary root. This is a trick + // to interrupt an in-progress async render so that it's forced to + // start over. ReactNoop.flushSync(() => { ReactNoop.renderToRootWithID(null, 'other-root'); }); @@ -295,12 +299,10 @@ describe('ReactIncrementalErrorHandling', () => { ReactNoop.render(, onCommit); Scheduler.unstable_advanceTime(1000); expect(Scheduler).toFlushAndYieldThrough(['error']); - interrupt(); - expect(ReactNoop).toMatchRenderedOutput(null); - // This update is in a separate batch ReactNoop.render(, onCommit); + interrupt(); expect(Scheduler).toFlushAndYieldThrough([ // The first render fails. But because there's a lower priority pending diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.internal.js index bcb9f7189c05a..594273096d607 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.internal.js @@ -457,7 +457,9 @@ describe('ReactIncrementalUpdates', () => { expect(ReactNoop.getChildren()).toEqual([span('derived state')]); }); - it('flushes all expired updates in a single batch', () => { + // TODO: This tests the old expiration times model. Doesn't make sense in + // new model. Probably should delete. + it.skip('flushes all expired updates in a single batch', () => { const {useEffect} = React; function App({label}) { @@ -546,7 +548,9 @@ describe('ReactIncrementalUpdates', () => { }); }); - it('flushes all expired updates in a single batch across multiple roots', () => { + // TODO: This tests the old expiration times model. Doesn't make sense in + // new model. Probably should delete. + it.skip('flushes all expired updates in a single batch across multiple roots', () => { // Same as previous test, but with two roots. const {useEffect} = React; diff --git a/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js index 0a1d209d41f52..f9459d5b68cbf 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js @@ -328,8 +328,9 @@ describe('ReactSuspense', () => { 'a parent', () => { function interrupt() { - // React has a heuristic to batch all updates that occur within the same - // event. This is a trick to circumvent that heuristic. + // Perform a sync render on a different, abitrary root. This is a trick + // to interrupt an in-progress async render so that it's forced to + // start over. ReactTestRenderer.create('whatever'); } @@ -361,12 +362,12 @@ describe('ReactSuspense', () => { Scheduler.unstable_advanceTime(1000); // Do a bit of work, then interrupt to trigger a restart. expect(Scheduler).toFlushAndYieldThrough(['A1']); - interrupt(); - - // Schedule another update. This will have lower priority because of - // the interrupt trick above. + // Schedule another update. This will have lower priority than the + // currently in progress render. root.update(); + interrupt(); + expect(Scheduler).toFlushAndYieldThrough([ // Should have restarted the first update, because of the interruption 'A1', diff --git a/packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js index 012511d7654cf..1f26fc7923702 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js @@ -498,6 +498,13 @@ describe('ReactSuspensePlaceholder', () => { , ); + + // TODO: This is here only to shift us into the next JND bucket. A + // consequence of AsyncText relying on the same timer queue as React's + // internal Suspense timer. We should decouple our AsyncText helpers + // from timers. + Scheduler.unstable_advanceTime(100); + expect(Scheduler).toFlushAndYield([ 'App', 'Suspending', diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js index d4997de38f705..676a9f3eb9fb4 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js @@ -7,6 +7,7 @@ let ReactCache; let Suspense; let TextResource; +let resolveText; let textResourceShouldFail; describe('ReactSuspenseWithNoopRenderer', () => { @@ -28,22 +29,37 @@ describe('ReactSuspenseWithNoopRenderer', () => { ReactCache = require('react-cache'); Suspense = React.Suspense; + const textCache = new Map(); + + resolveText = text => { + const resolvers = textCache.get(text); + if (resolvers !== undefined) { + textCache.delete(text); + const [resolve, reject] = resolvers; + if (textResourceShouldFail) { + Scheduler.unstable_yieldValue(`Promise rejected [${text}]`); + reject(new Error('Failed to load: ' + text)); + } else { + Scheduler.unstable_yieldValue(`Promise resolved [${text}]`); + resolve(text); + } + } + }; + TextResource = ReactCache.unstable_createResource( - ([text, ms = 0]) => { - return new Promise((resolve, reject) => - setTimeout(() => { - if (textResourceShouldFail) { - Scheduler.unstable_yieldValue(`Promise rejected [${text}]`); - reject(new Error('Failed to load: ' + text)); - } else { - Scheduler.unstable_yieldValue(`Promise resolved [${text}]`); - resolve(text); - } - }, ms), - ); + ([text, ms]) => { + return new Promise((resolve, reject) => { + textCache.set(text, [resolve, reject]); + if (typeof ms === 'number') { + setTimeout(() => { + resolveText(text); + }, ms); + } + }); }, ([text, ms]) => text, ); + textResourceShouldFail = false; }); @@ -297,7 +313,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { expect(ReactNoop.getChildren()).toEqual([]); // Wait for data to resolve - await advanceTimers(100); + await resolveText('B'); // Renders successfully expect(Scheduler).toHaveYielded(['Promise resolved [B]']); expect(Scheduler).toFlushAndYield(['A', 'B', 'C', 'D']); @@ -434,7 +450,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { // Initial mount ReactNoop.render(); expect(Scheduler).toFlushAndYield(['A', 'Suspend! [1]', 'Loading...']); - await advanceTimers(0); + await resolveText('1'); expect(Scheduler).toHaveYielded(['Promise resolved [1]']); expect(Scheduler).toFlushAndYield(['A', '1']); expect(ReactNoop.getChildren()).toEqual([span('A'), span('1')]); @@ -457,12 +473,14 @@ describe('ReactSuspenseWithNoopRenderer', () => { expect(ReactNoop.getChildren()).toEqual([span('B'), span('1')]); // Unblock the low-pri text and finish - await advanceTimers(0); + await resolveText('2'); expect(Scheduler).toHaveYielded(['Promise resolved [2]']); expect(ReactNoop.getChildren()).toEqual([span('B'), span('1')]); }); - it('keeps working on lower priority work after being pinged', async () => { + // TODO: This tests the old expiration times model. Doesn't make sense in + // new model. Probably should delete. + it.skip('keeps working on lower priority work after being pinged', async () => { // Advance the virtual time so that we're close to the edge of a bucket. ReactNoop.expire(149); @@ -526,7 +544,9 @@ describe('ReactSuspenseWithNoopRenderer', () => { expect(ReactNoop.getChildren()).toEqual([span('(empty)')]); }); - it('tries each subsequent level after suspending', async () => { + // TODO: This tests the old expiration times model. Doesn't make sense in + // new model. Probably should delete. + it.skip('tries each subsequent level after suspending', async () => { const root = ReactNoop.createRoot(); function App({step, shouldSuspend}) { @@ -704,7 +724,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { ]); }); - it('renders an expiration boundary synchronously', async () => { + it('renders an Suspense boundary synchronously', async () => { spyOnDev(console, 'error'); // Synchronously render a tree that suspends ReactNoop.flushSync(() => @@ -729,13 +749,13 @@ describe('ReactSuspenseWithNoopRenderer', () => { expect(ReactNoop.getChildren()).toEqual([span('Loading...'), span('Sync')]); // Once the promise resolves, we render the suspended view - await advanceTimers(0); + await resolveText('Async'); expect(Scheduler).toHaveYielded(['Promise resolved [Async]']); expect(Scheduler).toFlushAndYield(['Async']); expect(ReactNoop.getChildren()).toEqual([span('Async'), span('Sync')]); }); - it('suspending inside an expired expiration boundary will bubble to the next one', async () => { + it('suspending inside an expired Suspense boundary will bubble to the next one', async () => { ReactNoop.flushSync(() => ReactNoop.render( @@ -2018,8 +2038,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { await ReactNoop.act(async () => show(true)); expect(Scheduler).toHaveYielded(['Suspend! [A]']); - Scheduler.unstable_advanceTime(100); - await advanceTimers(100); + await resolveText('A'); expect(Scheduler).toHaveYielded(['Promise resolved [A]']); }); @@ -2044,8 +2063,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { await ReactNoop.act(async () => _setShow(true)); expect(Scheduler).toHaveYielded(['Suspend! [A]']); - Scheduler.unstable_advanceTime(100); - await advanceTimers(100); + await resolveText('A'); expect(Scheduler).toHaveYielded(['Promise resolved [A]']); }); @@ -2076,8 +2094,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { expect(ReactNoop.getChildren()).toEqual([span('Initial load...')]); // Eventually we resolve and show the data. - Scheduler.unstable_advanceTime(5000); - await advanceTimers(5000); + await resolveText('A'); expect(Scheduler).toHaveYielded(['Promise resolved [A]']); expect(Scheduler).toFlushAndYield(['A', 'B']); expect(ReactNoop.getChildren()).toEqual([span('A'), span('B')]); @@ -2103,8 +2120,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { ]); // Later we load the data. - Scheduler.unstable_advanceTime(5000); - await advanceTimers(5000); + await resolveText('C'); expect(Scheduler).toHaveYielded(['Promise resolved [C]']); expect(Scheduler).toFlushAndYield(['A', 'C']); expect(ReactNoop.getChildren()).toEqual([span('A'), span('C'), span('B')]); @@ -2260,8 +2276,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { expect(ReactNoop.getChildren()).toEqual([span('Loading...')]); // Later we load the data. - Scheduler.unstable_advanceTime(5000); - await advanceTimers(5000); + await resolveText('A'); expect(Scheduler).toHaveYielded(['Promise resolved [A]']); expect(Scheduler).toFlushAndYield(['A']); expect(ReactNoop.getChildren()).toEqual([span('A')]); @@ -2285,8 +2300,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { span('Loading...'), ]); // Later we load the data. - Scheduler.unstable_advanceTime(3000); - await advanceTimers(3000); + await resolveText('B'); expect(Scheduler).toHaveYielded(['Promise resolved [B]']); expect(Scheduler).toFlushAndYield(['B']); expect(ReactNoop.getChildren()).toEqual([span('B')]); @@ -2325,8 +2339,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { }); // Later we load the data. - Scheduler.unstable_advanceTime(5000); - await advanceTimers(5000); + await resolveText('A'); expect(Scheduler).toHaveYielded(['Promise resolved [A]']); expect(Scheduler).toFlushAndYield(['A']); expect(ReactNoop.getChildren()).toEqual([span('A')]); @@ -2352,8 +2365,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { ]); }); // Later we load the data. - Scheduler.unstable_advanceTime(3000); - await advanceTimers(3000); + await resolveText('B'); expect(Scheduler).toHaveYielded(['Promise resolved [B]']); expect(Scheduler).toFlushAndYield(['B']); expect(ReactNoop.getChildren()).toEqual([span('B')]); @@ -2371,7 +2383,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { } return ( }> - + ); } @@ -2395,8 +2407,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { }); // Later we load the data. - Scheduler.unstable_advanceTime(5000); - await advanceTimers(5000); + await resolveText('A'); expect(Scheduler).toHaveYielded(['Promise resolved [A]']); expect(Scheduler).toFlushAndYield(['A']); expect(ReactNoop.getChildren()).toEqual([span('A')]); @@ -2422,8 +2433,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { ]); }); // Later we load the data. - Scheduler.unstable_advanceTime(3000); - await advanceTimers(3000); + await resolveText('B'); expect(Scheduler).toHaveYielded(['Promise resolved [B]']); expect(Scheduler).toFlushAndYield(['B']); expect(ReactNoop.getChildren()).toEqual([span('B')]); @@ -2434,7 +2444,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { function App({page}) { return ( }> - + ); } @@ -2442,8 +2452,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { // Initial render. ReactNoop.render(); expect(Scheduler).toFlushAndYield(['Suspend! [A]', 'Loading...']); - Scheduler.unstable_advanceTime(2000); - await advanceTimers(2000); + await resolveText('A'); expect(Scheduler).toHaveYielded(['Promise resolved [A]']); expect(Scheduler).toFlushAndYield(['A']); expect(ReactNoop.getChildren()).toEqual([span('A')]); @@ -2471,8 +2480,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { span('Loading...'), ]); - Scheduler.unstable_advanceTime(2000); - await advanceTimers(2000); + await resolveText('B'); expect(Scheduler).toHaveYielded(['Promise resolved [B]']); expect(Scheduler).toFlushAndYield(['B']); expect(ReactNoop.getChildren()).toEqual([span('B')]); @@ -2489,12 +2497,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { }, {timeoutMs: 2000}, ); - expect(Scheduler).toFlushAndYield([ - 'Suspend! [C]', - 'Loading...', - 'Suspend! [C]', - 'Loading...', - ]); + expect(Scheduler).toFlushAndYield(['Suspend! [C]', 'Loading...']); expect(ReactNoop.getChildren()).toEqual([span('B')]); Scheduler.unstable_advanceTime(1200); await advanceTimers(1200); @@ -2517,7 +2520,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { } unstable_avoidThisFallback={true}> - + ); @@ -2526,8 +2529,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { // Initial render. ReactNoop.render(); expect(Scheduler).toFlushAndYield(['Hi!', 'Suspend! [A]', 'Loading...']); - Scheduler.unstable_advanceTime(3000); - await advanceTimers(3000); + await resolveText('A'); expect(Scheduler).toHaveYielded(['Promise resolved [A]']); expect(Scheduler).toFlushAndYield(['Hi!', 'A']); expect(ReactNoop.getChildren()).toEqual([span('Hi!'), span('A')]); @@ -2549,7 +2551,6 @@ describe('ReactSuspenseWithNoopRenderer', () => { expect(ReactNoop.getChildren()).toEqual([span('Hi!'), span('A')]); Scheduler.unstable_advanceTime(1500); await advanceTimers(1500); - expect(Scheduler).toHaveYielded(['Promise resolved [B]']); expect(ReactNoop.getChildren()).toEqual([ span('Hi!'), hiddenSpan('A'), @@ -2605,23 +2606,6 @@ describe('ReactSuspenseWithNoopRenderer', () => { }); it('supports delaying a busy spinner from disappearing', async () => { - function useLoadingIndicator(config) { - let [isLoading, setLoading] = React.useState(false); - let start = React.useCallback( - cb => { - setLoading(true); - Scheduler.unstable_next(() => - React.unstable_withSuspenseConfig(() => { - setLoading(false); - cb(); - }, config), - ); - }, - [setLoading, config], - ); - return [isLoading, start]; - } - const SUSPENSE_CONFIG = { timeoutMs: 10000, busyDelayMs: 500, @@ -2632,12 +2616,12 @@ describe('ReactSuspenseWithNoopRenderer', () => { function App() { let [page, setPage] = React.useState('A'); - let [isLoading, startLoading] = useLoadingIndicator(SUSPENSE_CONFIG); - transitionToPage = nextPage => startLoading(() => setPage(nextPage)); + let [startTransition, isPending] = React.useTransition(SUSPENSE_CONFIG); + transitionToPage = nextPage => startTransition(() => setPage(nextPage)); return ( - {isLoading ? : null} + {isPending ? : null} ); } @@ -2663,7 +2647,8 @@ describe('ReactSuspenseWithNoopRenderer', () => { // the loading indicator. Scheduler.unstable_advanceTime(600); await advanceTimers(600); - expect(Scheduler).toFlushAndYield(['B', 'L', 'C']); + expect(Scheduler).toHaveYielded(['B', 'L']); + expect(Scheduler).toFlushAndYield(['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. @@ -2679,7 +2664,8 @@ describe('ReactSuspenseWithNoopRenderer', () => { // the loading indicator. Scheduler.unstable_advanceTime(1000); await advanceTimers(1000); - expect(Scheduler).toFlushAndYield(['C', 'L', 'D']); + expect(Scheduler).toHaveYielded(['C', 'L']); + expect(Scheduler).toFlushAndYield(['D']); // However, since we exceeded the minimum time to show // the loading indicator, we commit immediately. expect(ReactNoop.getChildren()).toEqual([span('D')]); @@ -2813,7 +2799,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { () => { ReactNoop.render(); }, - {timeoutMs: 2000}, + {timeoutMs: 2500}, ); expect(Scheduler).toFlushAndYield(['Suspend! [A]', 'Loading...']); diff --git a/packages/react-reconciler/src/__tests__/ReactTransition-test.internal.js b/packages/react-reconciler/src/__tests__/ReactTransition-test.internal.js index 22451a3bbee40..7c3f8a43019eb 100644 --- a/packages/react-reconciler/src/__tests__/ReactTransition-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactTransition-test.internal.js @@ -88,8 +88,10 @@ describe('ReactTransition', () => { start(); expect(Scheduler).toFlushAndYield([ + // Render pending state 'Pending...', '(empty)', + // Try rendering transition 'Suspend! [Async]', 'Loading...', ]); @@ -102,4 +104,762 @@ describe('ReactTransition', () => { expect(root).toMatchRenderedOutput('Async'); }, ); + + it.experimental( + 'isPending turns off immediately if `startTransition` does not include any updates', + async () => { + let startTransition; + function App() { + const [_startTransition, isPending] = useTransition(); + startTransition = _startTransition; + return ; + } + + const root = ReactNoop.createRoot(); + + await act(async () => { + root.render(); + }); + expect(Scheduler).toHaveYielded(['Pending: false']); + expect(root).toMatchRenderedOutput('Pending: false'); + + await act(async () => { + startTransition(() => { + // No-op + }); + }); + expect(Scheduler).toHaveYielded([ + // Pending state is turned on then immediately back off + // TODO: As an optimization, we could avoid turning on the pending + // state entirely. + 'Pending: true', + 'Pending: false', + ]); + expect(root).toMatchRenderedOutput('Pending: false'); + }, + ); + + it.experimental( + 'works if two transitions happen one right after the other', + async () => { + // Tests an implementation path where two transitions get batched into the + // same render. This is an edge case in our current expiration times + // implementation but will be the normal case if/when we replace expiration + // times with a different model that puts all transitions into the same + // batch by default. + const CONFIG = { + timeoutMs: 100000, + }; + + let setTab; + let startTransition; + function App() { + const [tab, _setTab] = useState(1); + const [_startTransition, isPending] = useTransition(CONFIG); + startTransition = _startTransition; + setTab = _setTab; + return ( + + ); + } + + const root = ReactNoop.createRoot(); + + await act(async () => { + root.render(); + }); + expect(Scheduler).toHaveYielded(['Tab 1']); + expect(root).toMatchRenderedOutput('Tab 1'); + + await act(async () => { + startTransition(() => setTab(2)); + }); + expect(Scheduler).toHaveYielded(['Tab 1 (pending...)', 'Tab 2']); + expect(root).toMatchRenderedOutput('Tab 2'); + + // Because time has not advanced, this will fall into the same bucket + // as the previous transition. + await act(async () => { + startTransition(() => setTab(3)); + }); + expect(Scheduler).toHaveYielded(['Tab 2 (pending...)', 'Tab 3']); + expect(root).toMatchRenderedOutput('Tab 3'); + }, + ); + + it.experimental( + 'when multiple transitions update the same queue, only the most recent one is considered pending', + async () => { + const CONFIG = { + timeoutMs: 100000, + }; + + const Tab = React.forwardRef(({label, setTab}, ref) => { + const [startTransition, isPending] = useTransition(CONFIG); + + React.useImperativeHandle( + ref, + () => ({ + go() { + startTransition(() => setTab(label)); + }, + }), + [label], + ); + + return ( + + ); + }); + + const tabButtonA = React.createRef(null); + const tabButtonB = React.createRef(null); + const tabButtonC = React.createRef(null); + + const ContentA = createAsyncText('A'); + const ContentB = createAsyncText('B'); + const ContentC = createAsyncText('C'); + + function App() { + const [tab, setTab] = useState('A'); + + let content; + switch (tab) { + case 'A': + content = ; + break; + case 'B': + content = ; + break; + case 'C': + content = ; + break; + default: + content = ; + break; + } + + return ( + <> +
    +
  • + +
  • +
  • + +
  • +
  • + +
  • +
+ }>{content} + + ); + } + + // Initial render + const root = ReactNoop.createRoot(); + await act(async () => { + root.render(); + await ContentA.resolve(); + }); + expect(Scheduler).toHaveYielded(['Tab A', 'Tab B', 'Tab C', 'A']); + expect(root).toMatchRenderedOutput( + <> +
    +
  • Tab A
  • +
  • Tab B
  • +
  • Tab C
  • +
+ A + , + ); + + // Navigate to tab B + await act(async () => { + tabButtonB.current.go(); + }); + expect(Scheduler).toHaveYielded([ + 'Tab B (pending...)', + 'Tab A', + 'Tab B', + 'Tab C', + 'Suspend! [B]', + 'Loading...', + ]); + expect(root).toMatchRenderedOutput( + <> +
    +
  • Tab A
  • +
  • Tab B (pending...)
  • +
  • Tab C
  • +
+ A + , + ); + + // Before B resolves, navigate to tab C. B should no longer be pending. + await act(async () => { + tabButtonC.current.go(); + }); + expect(Scheduler).toHaveYielded([ + // Turn `isPending` off for tab B, and on for tab C + 'Tab B', + 'Tab C (pending...)', + // Try finishing the transition + 'Tab A', + 'Tab B', + 'Tab C', + 'Suspend! [C]', + 'Loading...', + ]); + // Tab B is no longer pending. Only C. + expect(root).toMatchRenderedOutput( + <> +
    +
  • Tab A
  • +
  • Tab B
  • +
  • Tab C (pending...)
  • +
+ A + , + ); + + // Finish loading C + await act(async () => { + ContentC.resolve(); + }); + expect(Scheduler).toHaveYielded(['Tab A', 'Tab B', 'Tab C', 'C']); + expect(root).toMatchRenderedOutput( + <> +
    +
  • Tab A
  • +
  • Tab B
  • +
  • Tab C
  • +
+ C + , + ); + }, + ); + + // Same as previous test, but for class update queue. + it.experimental( + 'when multiple transitions update the same queue, only the most recent one is considered pending (classes)', + async () => { + const CONFIG = { + timeoutMs: 100000, + }; + + const Tab = React.forwardRef(({label, setTab}, ref) => { + const [startTransition, isPending] = useTransition(CONFIG); + + React.useImperativeHandle( + ref, + () => ({ + go() { + startTransition(() => setTab(label)); + }, + }), + [label], + ); + + return ( + + ); + }); + + const tabButtonA = React.createRef(null); + const tabButtonB = React.createRef(null); + const tabButtonC = React.createRef(null); + + const ContentA = createAsyncText('A'); + const ContentB = createAsyncText('B'); + const ContentC = createAsyncText('C'); + + class App extends React.Component { + state = {tab: 'A'}; + setTab = tab => { + this.setState({tab}); + }; + + render() { + let content; + switch (this.state.tab) { + case 'A': + content = ; + break; + case 'B': + content = ; + break; + case 'C': + content = ; + break; + default: + content = ; + break; + } + + return ( + <> +
    +
  • + +
  • +
  • + +
  • +
  • + +
  • +
+ }> + {content} + + + ); + } + } + + // Initial render + const root = ReactNoop.createRoot(); + await act(async () => { + root.render(); + await ContentA.resolve(); + }); + expect(Scheduler).toHaveYielded(['Tab A', 'Tab B', 'Tab C', 'A']); + expect(root).toMatchRenderedOutput( + <> +
    +
  • Tab A
  • +
  • Tab B
  • +
  • Tab C
  • +
+ A + , + ); + + // Navigate to tab B + await act(async () => { + tabButtonB.current.go(); + }); + expect(Scheduler).toHaveYielded([ + 'Tab B (pending...)', + 'Tab A', + 'Tab B', + 'Tab C', + 'Suspend! [B]', + 'Loading...', + ]); + expect(root).toMatchRenderedOutput( + <> +
    +
  • Tab A
  • +
  • Tab B (pending...)
  • +
  • Tab C
  • +
+ A + , + ); + + // Before B resolves, navigate to tab C. B should no longer be pending. + await act(async () => { + tabButtonC.current.go(); + }); + expect(Scheduler).toHaveYielded([ + // Turn `isPending` off for tab B, and on for tab C + 'Tab B', + 'Tab C (pending...)', + // Try finishing the transition + 'Tab A', + 'Tab B', + 'Tab C', + 'Suspend! [C]', + 'Loading...', + ]); + // Tab B is no longer pending. Only C. + expect(root).toMatchRenderedOutput( + <> +
    +
  • Tab A
  • +
  • Tab B
  • +
  • Tab C (pending...)
  • +
+ A + , + ); + + // Finish loading C + await act(async () => { + ContentC.resolve(); + }); + expect(Scheduler).toHaveYielded(['Tab A', 'Tab B', 'Tab C', 'C']); + expect(root).toMatchRenderedOutput( + <> +
    +
  • Tab A
  • +
  • Tab B
  • +
  • Tab C
  • +
+ C + , + ); + }, + ); + + it.experimental( + 'when multiple transitions update the same queue, only the most recent ' + + 'one is allowed to finish (no intermediate states)', + async () => { + const CONFIG = { + timeoutMs: 100000, + }; + + const Tab = React.forwardRef(({label, setTab}, ref) => { + const [startTransition, isPending] = useTransition(CONFIG); + + React.useImperativeHandle( + ref, + () => ({ + go() { + startTransition(() => setTab(label)); + }, + }), + [label], + ); + + return ( + + ); + }); + + const tabButtonA = React.createRef(null); + const tabButtonB = React.createRef(null); + const tabButtonC = React.createRef(null); + + const ContentA = createAsyncText('A'); + const ContentB = createAsyncText('B'); + const ContentC = createAsyncText('C'); + + function App() { + Scheduler.unstable_yieldValue('App'); + + const [tab, setTab] = useState('A'); + + let content; + switch (tab) { + case 'A': + content = ; + break; + case 'B': + content = ; + break; + case 'C': + content = ; + break; + default: + content = ; + break; + } + + return ( + <> +
    +
  • + +
  • +
  • + +
  • +
  • + +
  • +
+ }>{content} + + ); + } + + // Initial render + const root = ReactNoop.createRoot(); + await act(async () => { + root.render(); + await ContentA.resolve(); + }); + expect(Scheduler).toHaveYielded(['App', 'Tab A', 'Tab B', 'Tab C', 'A']); + expect(root).toMatchRenderedOutput( + <> +
    +
  • Tab A
  • +
  • Tab B
  • +
  • Tab C
  • +
+ A + , + ); + + // Navigate to tab B + await act(async () => { + tabButtonB.current.go(); + expect(Scheduler).toFlushAndYieldThrough([ + // Turn on B's pending state + 'Tab B (pending...)', + // Partially render B + 'App', + 'Tab A', + 'Tab B', + ]); + + // While we're still in the middle of rendering B, switch to C. + tabButtonC.current.go(); + }); + expect(Scheduler).toHaveYielded([ + // Toggle the pending flags + 'Tab B', + 'Tab C (pending...)', + + // Start rendering B... + 'App', + // ...but bail out, since C is more recent. These should not be logged: + // 'Tab A', + // 'Tab B', + // 'Tab C (pending...)', + // 'Suspend! [B]', + // 'Loading...', + + // Now render C + 'App', + 'Tab A', + 'Tab B', + 'Tab C', + 'Suspend! [C]', + 'Loading...', + ]); + expect(root).toMatchRenderedOutput( + <> +
    +
  • Tab A
  • +
  • Tab B
  • +
  • Tab C (pending...)
  • +
+ A + , + ); + + // Finish loading B + await act(async () => { + ContentB.resolve(); + }); + // Should not switch to tab B because we've since clicked on C. + expect(Scheduler).toHaveYielded([]); + expect(root).toMatchRenderedOutput( + <> +
    +
  • Tab A
  • +
  • Tab B
  • +
  • Tab C (pending...)
  • +
+ A + , + ); + + // Finish loading C + await act(async () => { + ContentC.resolve(); + }); + expect(Scheduler).toHaveYielded(['App', 'Tab A', 'Tab B', 'Tab C', 'C']); + expect(root).toMatchRenderedOutput( + <> +
    +
  • Tab A
  • +
  • Tab B
  • +
  • Tab C
  • +
+ C + , + ); + }, + ); + + // Same as previous test, but for class update queue. + it.experimental( + 'when multiple transitions update the same queue, only the most recent ' + + 'one is allowed to finish (no intermediate states) (classes)', + async () => { + const CONFIG = { + timeoutMs: 100000, + }; + + const Tab = React.forwardRef(({label, setTab}, ref) => { + const [startTransition, isPending] = useTransition(CONFIG); + + React.useImperativeHandle( + ref, + () => ({ + go() { + startTransition(() => setTab(label)); + }, + }), + [label], + ); + + return ( + + ); + }); + + const tabButtonA = React.createRef(null); + const tabButtonB = React.createRef(null); + const tabButtonC = React.createRef(null); + + const ContentA = createAsyncText('A'); + const ContentB = createAsyncText('B'); + const ContentC = createAsyncText('C'); + + class App extends React.Component { + state = {tab: 'A'}; + setTab = tab => this.setState({tab}); + render() { + Scheduler.unstable_yieldValue('App'); + + let content; + switch (this.state.tab) { + case 'A': + content = ; + break; + case 'B': + content = ; + break; + case 'C': + content = ; + break; + default: + content = ; + break; + } + + return ( + <> +
    +
  • + +
  • +
  • + +
  • +
  • + +
  • +
+ }> + {content} + + + ); + } + } + + // Initial render + const root = ReactNoop.createRoot(); + await act(async () => { + root.render(); + await ContentA.resolve(); + }); + expect(Scheduler).toHaveYielded(['App', 'Tab A', 'Tab B', 'Tab C', 'A']); + expect(root).toMatchRenderedOutput( + <> +
    +
  • Tab A
  • +
  • Tab B
  • +
  • Tab C
  • +
+ A + , + ); + + // Navigate to tab B + await act(async () => { + tabButtonB.current.go(); + expect(Scheduler).toFlushAndYieldThrough([ + // Turn on B's pending state + 'Tab B (pending...)', + // Partially render B + 'App', + 'Tab A', + 'Tab B', + ]); + + // While we're still in the middle of rendering B, switch to C. + tabButtonC.current.go(); + }); + expect(Scheduler).toHaveYielded([ + // Toggle the pending flags + 'Tab B', + 'Tab C (pending...)', + + // Start rendering B... + // NOTE: This doesn't get logged like in the hooks version of this + // test because the update queue bails out before entering the render + // method. + // 'App', + // ...but bail out, since C is more recent. These should not be logged: + // 'Tab A', + // 'Tab B', + // 'Tab C (pending...)', + // 'Suspend! [B]', + // 'Loading...', + + // Now render C + 'App', + 'Tab A', + 'Tab B', + 'Tab C', + 'Suspend! [C]', + 'Loading...', + ]); + expect(root).toMatchRenderedOutput( + <> +
    +
  • Tab A
  • +
  • Tab B
  • +
  • Tab C (pending...)
  • +
+ A + , + ); + + // Finish loading B + await act(async () => { + ContentB.resolve(); + }); + // Should not switch to tab B because we've since clicked on C. + expect(Scheduler).toHaveYielded([]); + expect(root).toMatchRenderedOutput( + <> +
    +
  • Tab A
  • +
  • Tab B
  • +
  • Tab C (pending...)
  • +
+ A + , + ); + + // Finish loading C + await act(async () => { + ContentC.resolve(); + }); + expect(Scheduler).toHaveYielded(['App', 'Tab A', 'Tab B', 'Tab C', 'C']); + expect(root).toMatchRenderedOutput( + <> +
    +
  • Tab A
  • +
  • Tab B
  • +
  • Tab C
  • +
+ C + , + ); + }, + ); }); diff --git a/packages/react/src/__tests__/ReactProfilerDevToolsIntegration-test.internal.js b/packages/react/src/__tests__/ReactProfilerDevToolsIntegration-test.internal.js index d2f52dea1b11a..c8534cee18014 100644 --- a/packages/react/src/__tests__/ReactProfilerDevToolsIntegration-test.internal.js +++ b/packages/react/src/__tests__/ReactProfilerDevToolsIntegration-test.internal.js @@ -178,6 +178,8 @@ describe('ReactProfiler DevTools integration', () => { ]); }); + // TODO: This tests the old expiration times model. Doesn't make sense in + // new model. Probably should delete. it('regression test: #17159', () => { function Text({text}) { Scheduler.unstable_yieldValue(text); diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 70d46bc0734bc..4b11d83b72f0b 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -97,3 +97,5 @@ export const enableTrustedTypesIntegration = false; // Flag to turn event.target and event.currentTarget in ReactNative from a reactTag to a component instance export const enableNativeTargetAsInstance = false; + +export const preventIntermediateStates = __EXPERIMENTAL__; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 4ebed5ecb744d..e9ae0561f428c 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -45,6 +45,7 @@ export const disableLegacyContext = false; export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const enableTrainModelFix = false; export const enableTrustedTypesIntegration = false; +export const preventIntermediateStates = false; // Only used in www builds. export function addUserTimingListener() { diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 8b4b894aae2ad..322cc38e37b13 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -40,6 +40,7 @@ export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const enableTrainModelFix = false; export const enableTrustedTypesIntegration = false; export const enableNativeTargetAsInstance = false; +export const preventIntermediateStates = __EXPERIMENTAL__; // Only used in www builds. export function addUserTimingListener() { diff --git a/packages/shared/forks/ReactFeatureFlags.persistent.js b/packages/shared/forks/ReactFeatureFlags.persistent.js index b4d5489328a9c..573107a6d2f2c 100644 --- a/packages/shared/forks/ReactFeatureFlags.persistent.js +++ b/packages/shared/forks/ReactFeatureFlags.persistent.js @@ -40,6 +40,7 @@ export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const enableTrainModelFix = false; export const enableTrustedTypesIntegration = false; export const enableNativeTargetAsInstance = false; +export const preventIntermediateStates = __EXPERIMENTAL__; // Only used in www builds. export function addUserTimingListener() { diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 848305f9ab271..80f1ea7db5027 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -40,6 +40,7 @@ export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const enableTrainModelFix = false; export const enableTrustedTypesIntegration = false; export const enableNativeTargetAsInstance = false; +export const preventIntermediateStates = __EXPERIMENTAL__; // Only used in www builds. export function addUserTimingListener() { diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index 92743530e4a18..221b43686cb81 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -38,6 +38,7 @@ export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const enableTrainModelFix = false; export const enableTrustedTypesIntegration = false; export const enableNativeTargetAsInstance = false; +export const preventIntermediateStates = __EXPERIMENTAL__; // Only used in www builds. export function addUserTimingListener() { diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 6696c8e05ebab..d9d76eb7a3301 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -17,6 +17,7 @@ export const { enableTrustedTypesIntegration, enableSelectiveHydration, enableTrainModelFix, + preventIntermediateStates, } = require('ReactFeatureFlags'); // In www, we have experimental support for gathering data