From 0ab31d82a017a73c85ed1ba37e7367cd9212a7d3 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 19 Nov 2019 15:37:04 -0800 Subject: [PATCH 1/5] Replace useTransition update queue w/ forked impl The `isPending` state managed by the `useTransition` queue can be modeled using the same queue as useState/useReducer; that's how it's implemented today. However, since there's only ever a single pending transition per `useTransition` hook, and the most recent one always wins, we don't really need to maintain an entire queue structure. This replaces the internal `useState` queue with a specialized implementation. It tracks the most recent transition expiration time on shared instance object, and the last processed expiration time on each hook copy. When the processed expiration time catches up to the pending one, we know that the transition is no longer pending. At a high level, this is also how the `useState` queue works, but without the overhead of an actual queue. The implementation is also inspired by Suspense boundaries, which also have an internal state for whether the boundary is displaying a fallback, but does not use an actual queue to manage that state. --- .../react-reconciler/src/ReactFiberHooks.js | 215 +++++++++++++++--- .../ReactTransition-test.internal.js | 84 +++++++ 2 files changed, 271 insertions(+), 28 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 6022e54a138d..3cffd130bc79 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -118,6 +118,10 @@ type UpdateQueue = {| lastRenderedState: S | null, |}; +type TransitionInstance = {| + pendingExpirationTime: ExpirationTime, +|}; + export type HookType = | 'useState' | 'useReducer' @@ -1204,21 +1208,74 @@ function rerenderDeferredValue( return prevValue; } -function startTransition(setPending, config, callback) { +function startTransition(fiber, instance, config, callback) { + let resolvedConfig: SuspenseConfig | null = + config === undefined ? null : config; + + // 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, () => { - setPending(true); + const currentTime = requestCurrentTimeForUpdate(); + const expirationTime = computeExpirationForFiber( + currentTime, + fiber, + null, + ); + scheduleWork(fiber, expirationTime); }, ); runWithPriority( priorityLevel > NormalPriority ? NormalPriority : priorityLevel, () => { + const currentTime = requestCurrentTimeForUpdate(); + 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. + // TODO: This trick depends on transition expiration times being + // monotonically decreasing in priority, but since expiration times + // currently correspond to `timeoutMs`, that might not be true if + // `timeoutMs` changes to something smaller before the previous transition + // resolves. But this is a temporary edge case, since we're about to + // remove the correspondence between `timeoutMs` and the expiration time. + const oldPendingExpirationTime = instance.pendingExpirationTime; + while ( + oldPendingExpirationTime !== NoWork && + oldPendingExpirationTime <= expirationTime + ) { + // Temporary hack to make pendingExpirationTime monotonically decreasing + if (resolvedConfig === null) { + resolvedConfig = { + timeoutMs: 5250, + }; + } else { + resolvedConfig = { + timeoutMs: (resolvedConfig.timeoutMs | 0 || 5000) + 250, + busyDelayMs: resolvedConfig.busyDelayMs, + busyMinDurationMs: resolvedConfig.busyMinDurationMs, + }; + } + expirationTime = computeExpirationForFiber( + currentTime, + fiber, + resolvedConfig, + ); + } + instance.pendingExpirationTime = expirationTime; + + scheduleWork(fiber, expirationTime); const previousConfig = ReactCurrentBatchConfig.suspense; - ReactCurrentBatchConfig.suspense = config === undefined ? null : config; + ReactCurrentBatchConfig.suspense = resolvedConfig; try { - setPending(false); callback(); } finally { ReactCurrentBatchConfig.suspense = previousConfig; @@ -1230,34 +1287,136 @@ function startTransition(setPending, config, callback) { function mountTransition( config: SuspenseConfig | void | null, ): [(() => void) => void, boolean] { - const [isPending, setPending] = mountState(false); - const start = mountCallback(startTransition.bind(null, setPending, config), [ - setPending, - config, - ]); + const hook = mountWorkInProgressHook(); + + const instance: TransitionInstance = { + pendingExpirationTime: NoWork, + }; + // 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, + ((currentlyRenderingFiber: any): Fiber), + 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, - config, - ]); - return [start, isPending]; -} + const hook = updateWorkInProgressHook(); -function rerenderTransition( - config: SuspenseConfig | void | null, -): [(() => void) => void, boolean] { - const [isPending, setPending] = rerenderState(false); - const start = updateCallback(startTransition.bind(null, setPending, config), [ - setPending, - config, - ]); - return [start, isPending]; + 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, + ((currentlyRenderingFiber: any): Fiber), + instance, + config, + ), + [config], + ); + + return [start, newIsPending]; } function dispatchAction( @@ -1438,7 +1597,7 @@ const HooksDispatcherOnRerender: Dispatcher = { useDebugValue: updateDebugValue, useResponder: createDeprecatedResponderListener, useDeferredValue: rerenderDeferredValue, - useTransition: rerenderTransition, + useTransition: updateTransition, }; let HooksDispatcherOnMountInDEV: Dispatcher | null = null; @@ -1937,7 +2096,7 @@ if (__DEV__) { ): [(() => void) => void, boolean] { currentHookNameInDev = 'useTransition'; updateHookTypesDev(); - return rerenderTransition(config); + return updateTransition(config); }, }; @@ -2330,7 +2489,7 @@ if (__DEV__) { currentHookNameInDev = 'useTransition'; warnInvalidHookAccess(); updateHookTypesDev(); - return rerenderTransition(config); + return updateTransition(config); }, }; } diff --git a/packages/react-reconciler/src/__tests__/ReactTransition-test.internal.js b/packages/react-reconciler/src/__tests__/ReactTransition-test.internal.js index 22451a3bbee4..6349ea98aedf 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,86 @@ 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'); + }, + ); }); From 1865f23eb816f2bcaf8896a3275b8a8c9340eac5 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Wed, 20 Nov 2019 14:04:48 -0800 Subject: [PATCH 2/5] Only most recent transition is pending, per queue When multiple transitions update the same queue, only the most recent one should be considered pending. Example: If I switch tabs multiple times, only the last tab I click should display a pending state (e.g. an inline spinner). --- .../react-reconciler/src/ReactFiberHooks.js | 136 ++------ .../src/ReactFiberSuspenseConfig.js | 1 + .../src/ReactFiberTransition.js | 140 ++++++++ .../react-reconciler/src/ReactUpdateQueue.js | 26 +- .../ReactTransition-test.internal.js | 320 ++++++++++++++++++ 5 files changed, 518 insertions(+), 105 deletions(-) create mode 100644 packages/react-reconciler/src/ReactFiberTransition.js diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 3cffd130bc79..c5fb34cd2b39 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -16,6 +16,7 @@ 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 ReactSharedInternals from 'shared/ReactSharedInternals'; @@ -51,11 +52,11 @@ import is from 'shared/objectIs'; import {markWorkInProgressReceivedUpdate} from './ReactFiberBeginWork'; import {requestCurrentSuspenseConfig} from './ReactFiberSuspenseConfig'; import { - UserBlockingPriority, - NormalPriority, - runWithPriority, - getCurrentPriorityLevel, -} from './SchedulerWithReactIntegration'; + startTransition, + requestCurrentTransition, + cancelPendingTransition, +} from './ReactFiberTransition'; +import {getCurrentPriorityLevel} from './SchedulerWithReactIntegration'; const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals; @@ -114,14 +115,11 @@ type Update = {| type UpdateQueue = {| pending: Update | null, dispatch: (A => mixed) | null, + pendingTransition: TransitionInstance | null, lastRenderedReducer: ((S, A) => S) | null, lastRenderedState: S | null, |}; -type TransitionInstance = {| - pendingExpirationTime: ExpirationTime, -|}; - export type HookType = | 'useState' | 'useReducer' @@ -646,6 +644,7 @@ function mountReducer( const queue = (hook.queue = { pending: null, dispatch: null, + pendingTransition: null, lastRenderedReducer: reducer, lastRenderedState: (initialState: any), }); @@ -853,6 +852,7 @@ function mountState( const queue = (hook.queue = { pending: null, dispatch: null, + pendingTransition: null, lastRenderedReducer: basicStateReducer, lastRenderedState: (initialState: any), }); @@ -1208,89 +1208,14 @@ function rerenderDeferredValue( return prevValue; } -function startTransition(fiber, instance, config, callback) { - let resolvedConfig: SuspenseConfig | null = - config === undefined ? null : config; - - // 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, - () => { - const currentTime = requestCurrentTimeForUpdate(); - const expirationTime = computeExpirationForFiber( - currentTime, - fiber, - null, - ); - scheduleWork(fiber, expirationTime); - }, - ); - runWithPriority( - priorityLevel > NormalPriority ? NormalPriority : priorityLevel, - () => { - const currentTime = requestCurrentTimeForUpdate(); - 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. - // TODO: This trick depends on transition expiration times being - // monotonically decreasing in priority, but since expiration times - // currently correspond to `timeoutMs`, that might not be true if - // `timeoutMs` changes to something smaller before the previous transition - // resolves. But this is a temporary edge case, since we're about to - // remove the correspondence between `timeoutMs` and the expiration time. - const oldPendingExpirationTime = instance.pendingExpirationTime; - while ( - oldPendingExpirationTime !== NoWork && - oldPendingExpirationTime <= expirationTime - ) { - // Temporary hack to make pendingExpirationTime monotonically decreasing - if (resolvedConfig === null) { - resolvedConfig = { - timeoutMs: 5250, - }; - } else { - resolvedConfig = { - timeoutMs: (resolvedConfig.timeoutMs | 0 || 5000) + 250, - busyDelayMs: resolvedConfig.busyDelayMs, - busyMinDurationMs: resolvedConfig.busyMinDurationMs, - }; - } - expirationTime = computeExpirationForFiber( - currentTime, - fiber, - resolvedConfig, - ); - } - instance.pendingExpirationTime = expirationTime; - - scheduleWork(fiber, expirationTime); - const previousConfig = ReactCurrentBatchConfig.suspense; - ReactCurrentBatchConfig.suspense = resolvedConfig; - try { - callback(); - } finally { - ReactCurrentBatchConfig.suspense = previousConfig; - } - }, - ); -} - function mountTransition( config: SuspenseConfig | void | null, ): [(() => void) => void, boolean] { 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. @@ -1302,15 +1227,9 @@ function mountTransition( // 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, - ((currentlyRenderingFiber: any): Fiber), - instance, - config, - ), - [config], - ); + const start = mountCallback(startTransition.bind(null, instance, config), [ + config, + ]); const resolvedExpirationTime = NoWork; hook.memoizedState = { @@ -1406,15 +1325,9 @@ function updateTransition( resolvedExpirationTime: newResolvedExpirationTime, }; - const start = updateCallback( - startTransition.bind( - null, - ((currentlyRenderingFiber: any): Fiber), - instance, - config, - ), - [config], - ); + const start = updateCallback(startTransition.bind(null, instance, config), [ + config, + ]); return [start, newIsPending]; } @@ -1436,6 +1349,7 @@ function dispatchAction( const currentTime = requestCurrentTimeForUpdate(); const suspenseConfig = requestCurrentSuspenseConfig(); + const transition = requestCurrentTransition(); const expirationTime = computeExpirationForFiber( currentTime, fiber, @@ -1466,6 +1380,19 @@ function dispatchAction( } 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 || @@ -1524,6 +1451,7 @@ function dispatchAction( warnIfNotCurrentlyActingUpdatesInDev(fiber); } } + scheduleWork(fiber, expirationTime); } } diff --git a/packages/react-reconciler/src/ReactFiberSuspenseConfig.js b/packages/react-reconciler/src/ReactFiberSuspenseConfig.js index 4dabb29c93a1..2fc5f8c0f5ee 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/ReactFiberTransition.js b/packages/react-reconciler/src/ReactFiberTransition.js new file mode 100644 index 000000000000..f2cb57f0e25e --- /dev/null +++ b/packages/react-reconciler/src/ReactFiberTransition.js @@ -0,0 +1,140 @@ +/** + * 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; + + let resolvedConfig: SuspenseConfig | null = + config === undefined ? null : config; + + // 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, + () => { + const currentTime = requestCurrentTimeForUpdate(); + userBlockingExpirationTime = computeExpirationForFiber( + currentTime, + fiber, + null, + ); + scheduleUpdateOnFiber(fiber, userBlockingExpirationTime); + }, + ); + runWithPriority( + priorityLevel > NormalPriority ? NormalPriority : priorityLevel, + () => { + const currentTime = requestCurrentTimeForUpdate(); + 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. + // TODO: This trick depends on transition expiration times being + // monotonically decreasing in priority, but since expiration times + // currently correspond to `timeoutMs`, that might not be true if + // `timeoutMs` changes to something smaller before the previous transition + // resolves. But this is a temporary edge case, since we're about to + // remove the correspondence between `timeoutMs` and the expiration time. + const oldPendingExpirationTime = transitionInstance.pendingExpirationTime; + while ( + oldPendingExpirationTime !== NoWork && + oldPendingExpirationTime <= expirationTime + ) { + // Temporary hack to make pendingExpirationTime monotonically decreasing + if (resolvedConfig === null) { + resolvedConfig = { + timeoutMs: 5250, + }; + } else { + resolvedConfig = { + timeoutMs: (resolvedConfig.timeoutMs | 0 || 5000) + 250, + busyDelayMs: resolvedConfig.busyDelayMs, + busyMinDurationMs: resolvedConfig.busyMinDurationMs, + }; + } + expirationTime = computeExpirationForFiber( + currentTime, + fiber, + resolvedConfig, + ); + } + 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/ReactUpdateQueue.js b/packages/react-reconciler/src/ReactUpdateQueue.js index dba13e3cf2a3..36f1b5751a6b 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'; @@ -107,6 +108,11 @@ import { import invariant from 'shared/invariant'; import {getCurrentPriorityLevel} from './SchedulerWithReactIntegration'; +import { + requestCurrentTransition, + cancelPendingTransition, +} from './ReactFiberTransition'; + export type Update = {| expirationTime: ExpirationTime, suspenseConfig: null | SuspenseConfig, @@ -121,7 +127,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 +166,7 @@ export function initializeUpdateQueue(fiber: Fiber): void { baseQueue: null, shared: { pending: null, + pendingTransition: null, }, effects: null, }; @@ -220,6 +230,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 && diff --git a/packages/react-reconciler/src/__tests__/ReactTransition-test.internal.js b/packages/react-reconciler/src/__tests__/ReactTransition-test.internal.js index 6349ea98aedf..5eb280ff684e 100644 --- a/packages/react-reconciler/src/__tests__/ReactTransition-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactTransition-test.internal.js @@ -186,4 +186,324 @@ describe('ReactTransition', () => { 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 + , + ); + }, + ); }); From 55f5f0f2d5b448282dec9e2982d9264a0522b0c1 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Wed, 20 Nov 2019 17:56:11 -0800 Subject: [PATCH 3/5] Only show most recent transition, per queue When multiple transitions update the same queue, only the most recent one should be allowed to finish. Do not display intermediate states. For example, if you click on multiple tabs in quick succession, we should not switch to any tab that isn't the last one you clicked. --- .../react-reconciler/src/ReactFiberHooks.js | 41 +- .../react-reconciler/src/ReactFiberThrow.js | 6 + .../src/ReactFiberWorkLoop.js | 54 ++- .../react-reconciler/src/ReactUpdateQueue.js | 45 ++- .../ReactTransition-test.internal.js | 356 ++++++++++++++++++ packages/shared/ReactFeatureFlags.js | 2 + .../forks/ReactFeatureFlags.native-fb.js | 1 + .../forks/ReactFeatureFlags.native-oss.js | 1 + .../forks/ReactFeatureFlags.persistent.js | 1 + .../forks/ReactFeatureFlags.test-renderer.js | 1 + .../ReactFeatureFlags.test-renderer.www.js | 1 + .../shared/forks/ReactFeatureFlags.www.js | 1 + 12 files changed, 491 insertions(+), 19 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index c5fb34cd2b39..003e0859d2d5 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -19,6 +19,7 @@ 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'; @@ -50,6 +51,7 @@ 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 { startTransition, @@ -700,7 +702,10 @@ 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 @@ -725,6 +730,16 @@ 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. @@ -746,10 +761,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(updateExpirationTime, suspenseConfig); // Process this update. if (update.eagerReducer === reducer) { @@ -760,10 +772,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 { diff --git a/packages/react-reconciler/src/ReactFiberThrow.js b/packages/react-reconciler/src/ReactFiberThrow.js index f56b1a04b33a..5bf24d4ea300 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.js +++ b/packages/react-reconciler/src/ReactFiberThrow.js @@ -61,6 +61,12 @@ import {Sync} 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, diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index a4664613ac22..6808b38fb15a 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -127,6 +127,7 @@ import { throwException, createRootErrorUpdate, createClassErrorUpdate, + SuspendOnTask, } from './ReactFiberThrow'; import { commitBeforeMutationLifeCycles as commitBeforeMutationEffectOnFiber, @@ -201,13 +202,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, @@ -238,7 +240,7 @@ 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. @@ -708,7 +710,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 +756,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 @@ -1036,7 +1044,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, @@ -1264,6 +1277,7 @@ function prepareFreshStack(root, expirationTime) { workInProgressRootLatestSuspenseTimeout = Sync; workInProgressRootCanSuspendUsingConfig = null; workInProgressRootNextUnprocessedUpdateTime = NoWork; + workInProgressRootRestartTime = NoWork; workInProgressRootHasPendingPing = false; if (enableSchedulerTracing) { @@ -1284,6 +1298,20 @@ function handleError(root, thrownValue) { 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 @@ -2624,15 +2652,17 @@ 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(); diff --git a/packages/react-reconciler/src/ReactUpdateQueue.js b/packages/react-reconciler/src/ReactUpdateQueue.js index 36f1b5751a6b..5e37a7c1521a 100644 --- a/packages/react-reconciler/src/ReactUpdateQueue.js +++ b/packages/react-reconciler/src/ReactUpdateQueue.js @@ -97,13 +97,17 @@ 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'; @@ -413,15 +417,18 @@ 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, + expirationTime: updateExpirationTime, + suspenseConfig, tag: update.tag, payload: update.payload, @@ -439,6 +446,16 @@ 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. @@ -487,6 +504,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; @@ -502,6 +530,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__/ReactTransition-test.internal.js b/packages/react-reconciler/src/__tests__/ReactTransition-test.internal.js index 5eb280ff684e..7c3f8a43019e 100644 --- a/packages/react-reconciler/src/__tests__/ReactTransition-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactTransition-test.internal.js @@ -506,4 +506,360 @@ describe('ReactTransition', () => { ); }, ); + + 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/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 7a19c29ea8ae..96185329de2e 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -91,6 +91,8 @@ 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__; + // -------------------------- // Future APIs to be deprecated // -------------------------- diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 92790eb21182..5bf940a7fccc 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -47,6 +47,7 @@ export const enableTrainModelFix = false; export const enableTrustedTypesIntegration = false; export const disableCreateFactory = false; export const disableTextareaChildren = false; +export const preventIntermediateStates = __EXPERIMENTAL__; // 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 9facb94125d3..2a6a9240fb05 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -42,6 +42,7 @@ export const enableTrustedTypesIntegration = false; export const enableNativeTargetAsInstance = false; export const disableCreateFactory = false; export const disableTextareaChildren = 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 53547ef390bf..10aaecf9f8aa 100644 --- a/packages/shared/forks/ReactFeatureFlags.persistent.js +++ b/packages/shared/forks/ReactFeatureFlags.persistent.js @@ -42,6 +42,7 @@ export const enableTrustedTypesIntegration = false; export const enableNativeTargetAsInstance = false; export const disableCreateFactory = false; export const disableTextareaChildren = 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 efb730ca7b54..e3d065785551 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -42,6 +42,7 @@ export const enableTrustedTypesIntegration = false; export const enableNativeTargetAsInstance = false; export const disableCreateFactory = false; export const disableTextareaChildren = 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 7fb5145ea723..092ba85fc6ca 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -40,6 +40,7 @@ export const enableTrustedTypesIntegration = false; export const enableNativeTargetAsInstance = false; export const disableCreateFactory = false; export const disableTextareaChildren = 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 1abc4719ca26..0e7e51879e33 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 From edd241f6226543d136c0843c7c936acc0bd3962f Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Thu, 19 Dec 2019 14:04:20 -0800 Subject: [PATCH 4/5] Decouple expiration times and transition timeouts We currently use the expiration time to represent the timeout of a transition. Since we intend to stop treating work priority as a timeline, we can no longer use this trick. In this commit, I've changed it to store the event time on the update object instead. Long term, we will store event time on the root as a map of transition -> event time. I'm only storing it on the update object as a temporary workaround to unblock the rest of the changes. --- .../src/ReactFiberClassComponent.js | 6 +- .../src/ReactFiberExpirationTime.js | 11 +- .../react-reconciler/src/ReactFiberHooks.js | 9 +- .../src/ReactFiberNewContext.js | 2 +- .../src/ReactFiberReconciler.js | 2 +- .../react-reconciler/src/ReactFiberThrow.js | 8 +- .../src/ReactFiberTransition.js | 35 +---- .../src/ReactFiberWorkLoop.js | 121 +++++++----------- .../react-reconciler/src/ReactUpdateQueue.js | 14 +- ...eactHooksWithNoopRenderer-test.internal.js | 14 +- .../ReactSuspensePlaceholder-test.internal.js | 7 + ...tSuspenseWithNoopRenderer-test.internal.js | 38 ++---- 12 files changed, 107 insertions(+), 160 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.js b/packages/react-reconciler/src/ReactFiberClassComponent.js index 6b28bf900a89..54622014870a 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 5f4660dd5175..25a576117b59 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 003e0859d2d5..aabb6b7ed890 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -105,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, @@ -712,6 +715,7 @@ function updateReducer( // 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, @@ -743,8 +747,10 @@ function updateReducer( } 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, @@ -761,7 +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, suspenseConfig); + markRenderEventTimeAndConfig(eventTime, suspenseConfig); // Process this update. if (update.eagerReducer === reducer) { @@ -1390,6 +1396,7 @@ function dispatchAction( ); const update: Update = { + eventTime: currentTime, expirationTime, suspenseConfig, action, diff --git a/packages/react-reconciler/src/ReactFiberNewContext.js b/packages/react-reconciler/src/ReactFiberNewContext.js index 510c1eb674ad..37b683cedaf1 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 fb46524645ae..fb521cca5ae4 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/ReactFiberThrow.js b/packages/react-reconciler/src/ReactFiberThrow.js index 5bf24d4ea300..6f7b0766751c 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.js +++ b/packages/react-reconciler/src/ReactFiberThrow.js @@ -57,7 +57,7 @@ import { checkForWrongSuspensePriorityInDEV, } from './ReactFiberWorkLoop'; -import {Sync} from './ReactFiberExpirationTime'; +import {Sync, NoWork} from './ReactFiberExpirationTime'; const PossiblyWeakMap = typeof WeakMap === 'function' ? WeakMap : Map; @@ -72,7 +72,7 @@ function createRootErrorUpdate( 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 @@ -91,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') { @@ -267,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 index f2cb57f0e25e..2a9a07b39208 100644 --- a/packages/react-reconciler/src/ReactFiberTransition.js +++ b/packages/react-reconciler/src/ReactFiberTransition.js @@ -53,9 +53,11 @@ export function startTransition( ) { const fiber = transitionInstance.fiber; - let resolvedConfig: SuspenseConfig | null = + 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. @@ -63,7 +65,6 @@ export function startTransition( runWithPriority( priorityLevel < UserBlockingPriority ? UserBlockingPriority : priorityLevel, () => { - const currentTime = requestCurrentTimeForUpdate(); userBlockingExpirationTime = computeExpirationForFiber( currentTime, fiber, @@ -75,7 +76,6 @@ export function startTransition( runWithPriority( priorityLevel > NormalPriority ? NormalPriority : priorityLevel, () => { - const currentTime = requestCurrentTimeForUpdate(); let expirationTime = computeExpirationForFiber( currentTime, fiber, @@ -85,34 +85,9 @@ export function startTransition( // 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. - // TODO: This trick depends on transition expiration times being - // monotonically decreasing in priority, but since expiration times - // currently correspond to `timeoutMs`, that might not be true if - // `timeoutMs` changes to something smaller before the previous transition - // resolves. But this is a temporary edge case, since we're about to - // remove the correspondence between `timeoutMs` and the expiration time. const oldPendingExpirationTime = transitionInstance.pendingExpirationTime; - while ( - oldPendingExpirationTime !== NoWork && - oldPendingExpirationTime <= expirationTime - ) { - // Temporary hack to make pendingExpirationTime monotonically decreasing - if (resolvedConfig === null) { - resolvedConfig = { - timeoutMs: 5250, - }; - } else { - resolvedConfig = { - timeoutMs: (resolvedConfig.timeoutMs | 0 || 5000) + 250, - busyDelayMs: resolvedConfig.busyDelayMs, - busyMinDurationMs: resolvedConfig.busyMinDurationMs, - }; - } - expirationTime = computeExpirationForFiber( - currentTime, - fiber, - resolvedConfig, - ); + if (oldPendingExpirationTime === expirationTime) { + expirationTime -= 1; } transitionInstance.pendingExpirationTime = expirationTime; diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 6808b38fb15a..ecc6ff94d134 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'; @@ -183,6 +183,7 @@ import { clearCaughtError, } from 'shared/ReactErrorUtils'; import {onCommitRoot} from './ReactFiberDevToolsHook'; +import {requestCurrentTransition} from './ReactFiberTransition'; const ceil = Math.ceil; @@ -234,7 +235,7 @@ 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 @@ -333,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) { @@ -794,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 @@ -906,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. @@ -961,7 +949,7 @@ function finishConcurrentRender( flushSuspenseFallbacksInTests && IsThisRendererActing.current ) && - workInProgressRootLatestProcessedExpirationTime !== Sync && + workInProgressRootLatestProcessedEventTime !== Sync && workInProgressRootCanSuspendUsingConfig !== null ) { // If we have exceeded the minimum loading delay, which probably @@ -969,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, ); @@ -1273,7 +1261,7 @@ function prepareFreshStack(root, expirationTime) { renderExpirationTime = expirationTime; workInProgressRootExitStatus = RootIncomplete; workInProgressRootFatalError = null; - workInProgressRootLatestProcessedExpirationTime = Sync; + workInProgressRootLatestProcessedEventTime = Sync; workInProgressRootLatestSuspenseTimeout = Sync; workInProgressRootCanSuspendUsingConfig = null; workInProgressRootNextUnprocessedUpdateTime = NoWork; @@ -1384,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; + } } } } @@ -1458,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() { @@ -2374,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 @@ -2519,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 diff --git a/packages/react-reconciler/src/ReactUpdateQueue.js b/packages/react-reconciler/src/ReactUpdateQueue.js index 5e37a7c1521a..70908de85325 100644 --- a/packages/react-reconciler/src/ReactUpdateQueue.js +++ b/packages/react-reconciler/src/ReactUpdateQueue.js @@ -118,6 +118,9 @@ import { } 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, @@ -196,10 +199,12 @@ export function cloneUpdateQueue( } export function createUpdate( + eventTime: ExpirationTime, expirationTime: ExpirationTime, suspenseConfig: null | SuspenseConfig, ): Update<*> { let update: Update<*> = { + eventTime, expirationTime, suspenseConfig, @@ -427,6 +432,7 @@ export function processUpdateQueue( // skipped update, the previous update/state is the new base // update/state. const clone: Update = { + eventTime: update.eventTime, expirationTime: updateExpirationTime, suspenseConfig, @@ -458,9 +464,10 @@ export function processUpdateQueue( } } 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, @@ -479,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( diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js index ca4d3090215e..90eac8a02f02 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js @@ -2199,6 +2199,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([ @@ -2209,13 +2212,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'), ]); diff --git a/packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js index 012511d7654c..1f26fc792370 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 d4997de38f70..fdae88692f9c 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js @@ -2489,12 +2489,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); @@ -2605,23 +2600,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 +2610,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 +2641,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 +2658,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 +2793,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { () => { ReactNoop.render(); }, - {timeoutMs: 2000}, + {timeoutMs: 2500}, ); expect(Scheduler).toFlushAndYield(['Suspend! [A]', 'Loading...']); From 734285d8147d7ae519f24f2dfca71a656eea84a5 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Thu, 16 Jan 2020 11:38:37 -0800 Subject: [PATCH 5/5] Prevent intermediate transition states When multiple transitions update the same queue, only the most recent one should be allowed to finish. Do not display intermediate states. For example, if you click on multiple tabs in quick succession, we should not switch to any tab that isn't the last one you clicked. --- .../src/ReactFiberClassComponent.js | 106 +-- .../react-reconciler/src/ReactFiberHooks.js | 372 ++++------- .../react-reconciler/src/ReactFiberThrow.js | 6 - .../src/ReactFiberTransition.js | 602 ++++++++++++++++-- .../src/ReactFiberWorkLoop.js | 73 +-- .../react-reconciler/src/ReactUpdateQueue.js | 62 +- .../ReactTransition-test.internal.js | 210 +++--- 7 files changed, 792 insertions(+), 639 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.js b/packages/react-reconciler/src/ReactFiberClassComponent.js index 54622014870a..0a4666040eaa 100644 --- a/packages/react-reconciler/src/ReactFiberClassComponent.js +++ b/packages/react-reconciler/src/ReactFiberClassComponent.js @@ -19,8 +19,7 @@ import { warnAboutDeprecatedLifecycles, } from 'shared/ReactFeatureFlags'; import ReactStrictModeWarnings from './ReactStrictModeWarnings'; -import {isMounted} from 'react-reconciler/reflection'; -import {get as getInstance, set as setInstance} from 'shared/ReactInstanceMap'; +import {set as setInstance} from 'shared/ReactInstanceMap'; import shallowEqual from 'shared/shallowEqual'; import getComponentName from 'shared/getComponentName'; import invariant from 'shared/invariant'; @@ -31,13 +30,9 @@ import {resolveDefaultProps} from './ReactFiberLazyComponent'; import {StrictMode} from './ReactTypeOfMode'; import { - enqueueUpdate, processUpdateQueue, checkHasForceUpdateAfterProcessing, resetHasForceUpdateBeforeProcessing, - createUpdate, - ReplaceState, - ForceUpdate, initializeUpdateQueue, cloneUpdateQueue, } from './ReactUpdateQueue'; @@ -50,12 +45,7 @@ import { emptyContextObject, } from './ReactFiberContext'; import {readContext} from './ReactFiberNewContext'; -import { - requestCurrentTimeForUpdate, - computeExpirationForFiber, - scheduleWork, -} from './ReactFiberWorkLoop'; -import {requestCurrentSuspenseConfig} from './ReactFiberSuspenseConfig'; +import {classComponentUpdater} from './ReactFiberTransition'; const fakeInternalInstance = {}; const isArray = Array.isArray; @@ -70,7 +60,6 @@ let didWarnAboutGetSnapshotBeforeUpdateWithoutDidUpdate; let didWarnAboutLegacyLifecyclesAndDerivedState; let didWarnAboutUndefinedDerivedState; let warnOnUndefinedDerivedState; -let warnOnInvalidCallback; let didWarnAboutDirectlyAssigningPropsToState; let didWarnAboutContextTypeAndContextTypes; let didWarnAboutInvalidateContextType; @@ -85,24 +74,6 @@ if (__DEV__) { didWarnAboutContextTypeAndContextTypes = new Set(); didWarnAboutInvalidateContextType = new Set(); - const didWarnOnInvalidCallback = new Set(); - - warnOnInvalidCallback = function(callback: mixed, callerName: string) { - if (callback === null || typeof callback === 'function') { - return; - } - const key = `${callerName}_${(callback: any)}`; - if (!didWarnOnInvalidCallback.has(key)) { - didWarnOnInvalidCallback.add(key); - console.error( - '%s(...): Expected the last optional `callback` argument to be a ' + - 'function. Instead received: %s.', - callerName, - callback, - ); - } - }; - warnOnUndefinedDerivedState = function(type, partialState) { if (partialState === undefined) { const componentName = getComponentName(type) || 'Component'; @@ -178,79 +149,6 @@ export function applyDerivedStateFromProps( } } -const classComponentUpdater = { - isMounted, - enqueueSetState(inst, payload, callback) { - const fiber = getInstance(inst); - const currentTime = requestCurrentTimeForUpdate(); - const suspenseConfig = requestCurrentSuspenseConfig(); - const expirationTime = computeExpirationForFiber( - currentTime, - fiber, - suspenseConfig, - ); - - const update = createUpdate(currentTime, expirationTime, suspenseConfig); - update.payload = payload; - if (callback !== undefined && callback !== null) { - if (__DEV__) { - warnOnInvalidCallback(callback, 'setState'); - } - update.callback = callback; - } - - enqueueUpdate(fiber, update); - scheduleWork(fiber, expirationTime); - }, - enqueueReplaceState(inst, payload, callback) { - const fiber = getInstance(inst); - const currentTime = requestCurrentTimeForUpdate(); - const suspenseConfig = requestCurrentSuspenseConfig(); - const expirationTime = computeExpirationForFiber( - currentTime, - fiber, - suspenseConfig, - ); - - const update = createUpdate(currentTime, expirationTime, suspenseConfig); - update.tag = ReplaceState; - update.payload = payload; - - if (callback !== undefined && callback !== null) { - if (__DEV__) { - warnOnInvalidCallback(callback, 'replaceState'); - } - update.callback = callback; - } - - enqueueUpdate(fiber, update); - scheduleWork(fiber, expirationTime); - }, - enqueueForceUpdate(inst, callback) { - const fiber = getInstance(inst); - const currentTime = requestCurrentTimeForUpdate(); - const suspenseConfig = requestCurrentSuspenseConfig(); - const expirationTime = computeExpirationForFiber( - currentTime, - fiber, - suspenseConfig, - ); - - const update = createUpdate(currentTime, expirationTime, suspenseConfig); - update.tag = ForceUpdate; - - if (callback !== undefined && callback !== null) { - if (__DEV__) { - warnOnInvalidCallback(callback, 'forceUpdate'); - } - update.callback = callback; - } - - enqueueUpdate(fiber, update); - scheduleWork(fiber, expirationTime); - }, -}; - function checkShouldComponentUpdate( workInProgress, ctor, diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index aabb6b7ed890..3ef9a20c997f 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -19,7 +19,6 @@ 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'; @@ -37,12 +36,7 @@ import { MountPassive, } from './ReactHookEffectTags'; import { - scheduleWork, - computeExpirationForFiber, - requestCurrentTimeForUpdate, warnIfNotCurrentlyActingEffectsInDEV, - warnIfNotCurrentlyActingUpdatesInDev, - warnIfNotScopedWithMatchingAct, markRenderEventTimeAndConfig, markUnprocessedUpdateTime, } from './ReactFiberWorkLoop'; @@ -51,13 +45,7 @@ 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 { - startTransition, - requestCurrentTransition, - cancelPendingTransition, -} from './ReactFiberTransition'; +import {dispatch, startTransition} from './ReactFiberTransition'; import {getCurrentPriorityLevel} from './SchedulerWithReactIntegration'; const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals; @@ -104,7 +92,7 @@ export type Dispatcher = {| ): [(() => void) => void, boolean], |}; -type Update = {| +export type Update = {| // TODO: Temporary field. Will remove this by storing a map of // transition -> start time on the root. eventTime: ExpirationTime, @@ -117,7 +105,7 @@ type Update = {| priority?: ReactPriorityLevel, |}; -type UpdateQueue = {| +export type UpdateQueue = {| pending: Update | null, dispatch: (A => mixed) | null, pendingTransition: TransitionInstance | null, @@ -501,6 +489,35 @@ export function bailoutHooks( } } +export function requestRenderPhaseUpdate( + fiber: Fiber, + action: A, +): Update | null { + if ( + currentlyRenderingFiber === fiber || + (currentlyRenderingFiber !== null && + currentlyRenderingFiber.alternate === fiber) + ) { + currentlyRenderingFiber.expirationTime = renderExpirationTime; + didScheduleRenderPhaseUpdate = true; + + const renderPhaseUpdate: Update = { + eventTime: NoWork, + expirationTime: renderExpirationTime, + suspenseConfig: null, + action, + eagerReducer: null, + eagerState: null, + next: (null: any), + }; + if (__DEV__) { + renderPhaseUpdate.priority = getCurrentPriorityLevel(); + } + return renderPhaseUpdate; + } + return null; +} + 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. @@ -653,12 +670,12 @@ function mountReducer( lastRenderedReducer: reducer, lastRenderedState: (initialState: any), }); - const dispatch: Dispatch = (queue.dispatch = (dispatchAction.bind( + const boundDispatch: Dispatch = (queue.dispatch = (dispatch.bind( null, currentlyRenderingFiber, queue, ): any)); - return [hook.memoizedState, dispatch]; + return [hook.memoizedState, boundDispatch]; } function updateReducer( @@ -705,8 +722,6 @@ 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; @@ -716,8 +731,8 @@ function updateReducer( // update/state. const clone: Update = { eventTime: update.eventTime, - expirationTime: update.expirationTime, - suspenseConfig: update.suspenseConfig, + expirationTime: updateExpirationTime, + suspenseConfig: suspenseConfig, action: update.action, eagerReducer: update.eagerReducer, eagerState: update.eagerState, @@ -734,16 +749,6 @@ 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. @@ -778,31 +783,10 @@ 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,8 +806,8 @@ function updateReducer( queue.lastRenderedState = newState; } - const dispatch: Dispatch = (queue.dispatch: any); - return [hook.memoizedState, dispatch]; + const dispatchMethod: Dispatch = (queue.dispatch: any); + return [hook.memoizedState, dispatchMethod]; } function rerenderReducer( @@ -842,7 +826,7 @@ function rerenderReducer( // 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 dispatchMethod: Dispatch = (queue.dispatch: any); const lastRenderPhaseUpdate = queue.pending; let newState = hook.memoizedState; if (lastRenderPhaseUpdate !== null) { @@ -877,7 +861,7 @@ function rerenderReducer( queue.lastRenderedState = newState; } - return [newState, dispatch]; + return [newState, dispatchMethod]; } function mountState( @@ -895,14 +879,14 @@ function mountState( lastRenderedReducer: basicStateReducer, lastRenderedState: (initialState: any), }); - const dispatch: Dispatch< + const dispatchMethod: Dispatch< BasicStateAction, - > = (queue.dispatch = (dispatchAction.bind( + > = (queue.dispatch = (dispatch.bind( null, currentlyRenderingFiber, queue, ): any)); - return [hook.memoizedState, dispatch]; + return [hook.memoizedState, dispatchMethod]; } function updateState( @@ -1253,7 +1237,9 @@ function mountTransition( const hook = mountWorkInProgressHook(); const fiber = ((currentlyRenderingFiber: any): Fiber); const instance: TransitionInstance = { - pendingExpirationTime: NoWork, + version: 0, + pendingTime: NoWork, + resolvedTime: NoWork, fiber, }; // TODO: Intentionally storing this on the queue field to avoid adding a new/ @@ -1270,12 +1256,9 @@ function mountTransition( config, ]); - const resolvedExpirationTime = NoWork; hook.memoizedState = { isPending, - - // Represents the last processed expiration time. - resolvedExpirationTime, + version: 0, }; return [start, isPending]; @@ -1286,83 +1269,72 @@ function updateTransition( ): [(() => void) => void, boolean] { 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; + const oldVersion = oldState.version; - if (pendingExpirationTime === NoWork) { - // There are no pending transitions. Reset all fields. + const instance: TransitionInstance = (hook.queue: any); + const newVersion = instance.version; + + // 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) the level at which the most recent transition is + // pending 2) the level at which it resolves 3) a version number that gets + // bumps for each new transition. The version stored in state is only updated + // once the transition has fully resolved. + // TODO: This is maaaaaybe too clever. I think it works, but we can go back to + // a queue if needed. + let newIsPending; + if (oldVersion === newVersion) { + // Already resolved newIsPending = false; - newResolvedExpirationTime = NoWork; + if (instance.pendingTime !== NoWork) { + // The resolved state already committed, so we can reset these fields. + instance.pendingTime = instance.resolvedTime = 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. + // There's a pending transition. + const pendingTime = instance.pendingTime; + const resolvedTime = instance.resolvedTime; + const oldIsPending = oldState.isPending; + + let remainingExpirationTime; + let memoizedVersion; + if (renderExpirationTime <= resolvedTime) { + // Transition has finished. newIsPending = false; - newResolvedExpirationTime = oldResolvedExpirationTime; + remainingExpirationTime = NoWork; + // Only update the memoized version number once the transition finishes. + memoizedVersion = newVersion; + } else if (renderExpirationTime <= pendingTime) { + // Transition is pending. + newIsPending = true; + remainingExpirationTime = resolvedTime; + memoizedVersion = oldVersion; + } else { + // Outside of pending range. Reuse the old state. + newIsPending = oldIsPending; + remainingExpirationTime = pendingTime; + memoizedVersion = oldVersion; } - } - 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; - } + if (remainingExpirationTime > currentlyRenderingFiber.expirationTime) { + // Mark that there's remaining work + currentlyRenderingFiber.expirationTime = remainingExpirationTime; + markUnprocessedUpdateTime(resolvedTime); + } - hook.memoizedState = { - isPending: newIsPending, - resolvedExpirationTime: newResolvedExpirationTime, - }; + if (newIsPending !== oldIsPending) { + markWorkInProgressReceivedUpdate(); + } + + hook.memoizedState = { + version: memoizedVersion, + isPending: newIsPending, + }; + } const start = updateCallback(startTransition.bind(null, instance, config), [ config, @@ -1371,131 +1343,6 @@ function updateTransition( return [start, newIsPending]; } -function dispatchAction( - fiber: Fiber, - queue: UpdateQueue, - action: A, -) { - if (__DEV__) { - if (typeof arguments[3] === 'function') { - console.error( - "State updates from the useState() and useReducer() Hooks don't support the " + - 'second callback argument. To execute a side effect after ' + - 'rendering, declare it in the component body with useEffect().', - ); - } - } - - 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 || - (alternate !== null && alternate === currentlyRenderingFiber) - ) { - // This is a render phase update. Stash it in a lazily-created map of - // 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; - update.expirationTime = renderExpirationTime; - currentlyRenderingFiber.expirationTime = renderExpirationTime; - } else { - if ( - fiber.expirationTime === NoWork && - (alternate === null || alternate.expirationTime === NoWork) - ) { - // The queue is currently empty, which means we can eagerly compute the - // next state before entering the render phase. If the new state is the - // same as the current state, we may be able to bail out entirely. - const lastRenderedReducer = queue.lastRenderedReducer; - if (lastRenderedReducer !== null) { - let prevDispatcher; - if (__DEV__) { - prevDispatcher = ReactCurrentDispatcher.current; - ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnUpdateInDEV; - } - try { - const currentState: S = (queue.lastRenderedState: any); - const eagerState = lastRenderedReducer(currentState, action); - // Stash the eagerly computed state, and the reducer used to compute - // it, on the update object. If the reducer hasn't changed by the - // time we enter the render phase, then the eager state can be used - // without calling the reducer again. - update.eagerReducer = lastRenderedReducer; - update.eagerState = eagerState; - if (is(eagerState, currentState)) { - // Fast path. We can bail out without scheduling React to re-render. - // It's still possible that we'll need to rebase this update later, - // if the component re-renders for a different reason and by that - // time the reducer has changed. - return; - } - } catch (error) { - // Suppress the error. It will throw again in the render phase. - } finally { - if (__DEV__) { - ReactCurrentDispatcher.current = prevDispatcher; - } - } - } - } - if (__DEV__) { - // $FlowExpectedError - jest isn't a global, and isn't recognized outside of tests - if ('undefined' !== typeof jest) { - warnIfNotScopedWithMatchingAct(fiber); - warnIfNotCurrentlyActingUpdatesInDev(fiber); - } - } - - scheduleWork(fiber, expirationTime); - } -} - export const ContextOnlyDispatcher: Dispatcher = { readContext, @@ -1576,6 +1423,15 @@ let InvalidNestedHooksDispatcherOnMountInDEV: Dispatcher | null = null; let InvalidNestedHooksDispatcherOnUpdateInDEV: Dispatcher | null = null; let InvalidNestedHooksDispatcherOnRerenderInDEV: Dispatcher | null = null; +export function setInvalidNestedHooksDispatcher() { + if (__DEV__) { + const prevDispatcher = ReactCurrentDispatcher.current; + ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnUpdateInDEV; + return prevDispatcher; + } + return null; +} + if (__DEV__) { const warnInvalidContextAccess = () => { console.error( diff --git a/packages/react-reconciler/src/ReactFiberThrow.js b/packages/react-reconciler/src/ReactFiberThrow.js index 6f7b0766751c..7809223e83f3 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.js +++ b/packages/react-reconciler/src/ReactFiberThrow.js @@ -61,12 +61,6 @@ 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, diff --git a/packages/react-reconciler/src/ReactFiberTransition.js b/packages/react-reconciler/src/ReactFiberTransition.js index 2a9a07b39208..d78d1d9659ab 100644 --- a/packages/react-reconciler/src/ReactFiberTransition.js +++ b/packages/react-reconciler/src/ReactFiberTransition.js @@ -8,10 +8,12 @@ */ import type {Fiber} from './ReactFiber'; +import type {UpdateQueue, Update} from './ReactFiberHooks'; import type {ExpirationTime} from './ReactFiberExpirationTime'; import type {SuspenseConfig} from './ReactFiberSuspenseConfig'; import ReactSharedInternals from 'shared/ReactSharedInternals'; +import is from 'shared/objectIs'; import { UserBlockingPriority, @@ -23,27 +25,487 @@ import { scheduleUpdateOnFiber, computeExpirationForFiber, requestCurrentTimeForUpdate, + warnIfNotScopedWithMatchingAct, + warnIfNotCurrentlyActingUpdatesInDev, + scheduleWork, } from './ReactFiberWorkLoop'; import {NoWork} from './ReactFiberExpirationTime'; +import {requestCurrentSuspenseConfig} from './ReactFiberSuspenseConfig'; +import { + requestRenderPhaseUpdate, + setInvalidNestedHooksDispatcher, +} from './ReactFiberHooks'; +import { + createUpdate, + ReplaceState, + ForceUpdate, + enqueueUpdate, +} from './ReactUpdateQueue'; +import {get as getInstance} from 'shared/ReactInstanceMap'; +import {isMounted} from 'react-reconciler/reflection'; +import {preventIntermediateStates} from 'shared/ReactFeatureFlags'; -const {ReactCurrentBatchConfig} = ReactSharedInternals; +const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals; export type TransitionInstance = {| - pendingExpirationTime: ExpirationTime, + version: number, + pendingTime: ExpirationTime, + resolvedTime: ExpirationTime, fiber: Fiber, |}; +type Dispatch = ( + fiber: Fiber, + queue: UpdateQueue, + action: A, +) => void; + +type ClassSetState = ( + inst: any, + payload: mixed, + callback: ?() => mixed, +) => void; +type ClassReplaceState = ( + inst: any, + payload: mixed, + callback: ?() => mixed, +) => void; +type ClassForceUpdate = (inst: any, callback: ?() => mixed) => void; + +// The implementation of dispatch, setState, et al can be swapped out at +// runtime, e.g. when calling `startTransition`. These references point to +// the current implementation. +let dispatchImpl: Dispatch = dispatchImplDefault; +let classSetStateImpl: ClassSetState = classSetStateImplDefault; +let classReplaceStateImpl: ClassReplaceState = classReplaceStateImplDefault; +let classForceUpdateImpl: ClassForceUpdate = classForceUpdateImplDefault; + // Inside `startTransition`, this is the transition instance that corresponds to // the `useTransition` hook. let currentTransition: TransitionInstance | null = null; - +// The event time of the current transition. +let currentTransitionEventTime: ExpirationTime = NoWork; // 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; +let currentTransitionPendingTime: ExpirationTime = NoWork; +// The expiration time of the current transition. This is accumulated during +// `startTransition` because it depends on whether the current transition +// overlaps with any previous transitions. +let currentTransitionResolvedTime: ExpirationTime = NoWork; + +let dispatchContinuations: Array< + (ExpirationTime, ExpirationTime, SuspenseConfig | null) => void, +> | null = null; + +let warnOnInvalidCallback; +if (__DEV__) { + const didWarnOnInvalidCallback = new Set(); + + warnOnInvalidCallback = function(callback: mixed, callerName: string) { + if (callback === null || typeof callback === 'function') { + return; + } + const key = callerName + '_' + (callback: any); + if (!didWarnOnInvalidCallback.has(key)) { + didWarnOnInvalidCallback.add(key); + console.error( + '%s(...): Expected the last optional `callback` argument to be a ' + + 'function. Instead received: %s.', + callerName, + callback, + ); + } + }; +} + +export function dispatch( + fiber: Fiber, + queue: UpdateQueue, + action: A, +): void { + if (__DEV__) { + if (typeof arguments[3] === 'function') { + console.error( + "State updates from the useState() and useReducer() Hooks don't " + + 'support the second callback argument. To execute a side effect ' + + 'after rendering, declare it in the component body with useEffect().', + ); + } + } + + dispatchImpl(fiber, queue, action); +} + +export const classComponentUpdater = { + isMounted, + enqueueSetState(inst: any, payload: mixed, callback: ?() => mixed) { + classSetStateImpl(inst, payload, callback); + }, + enqueueReplaceState(inst: any, payload: mixed, callback: ?() => mixed) { + classReplaceStateImpl(inst, payload, callback); + }, + enqueueForceUpdate(inst: any, callback: ?() => mixed) { + classForceUpdateImpl(inst, callback); + }, +}; + +function dispatchImplDefault( + fiber: Fiber, + queue: UpdateQueue, + action: A, +) { + if (__DEV__) { + if (typeof arguments[3] === 'function') { + console.error( + "State updates from the useState() and useReducer() Hooks don't support the " + + 'second callback argument. To execute a side effect after ' + + 'rendering, declare it in the component body with useEffect().', + ); + } + } + const eventTime = requestCurrentTimeForUpdate(); + const suspenseConfig = requestCurrentSuspenseConfig(); + const expirationTime = computeExpirationForFiber( + eventTime, + fiber, + suspenseConfig, + ); + dispatchForExpirationTime( + fiber, + queue, + action, + eventTime, + expirationTime, + suspenseConfig, + ); +} + +function dispatchForExpirationTime( + fiber: Fiber, + queue: UpdateQueue, + action: A, + eventTime: ExpirationTime, + expirationTime: ExpirationTime, + suspenseConfig: SuspenseConfig | null, +) { + const update: Update = { + eventTime, + 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; + + const alternate = fiber.alternate; + if ( + fiber.expirationTime === NoWork && + (alternate === null || alternate.expirationTime === NoWork) + ) { + // The queue is currently empty, which means we can eagerly compute the + // next state before entering the render phase. If the new state is the + // same as the current state, we may be able to bail out entirely. + const lastRenderedReducer = queue.lastRenderedReducer; + if (lastRenderedReducer !== null) { + const prevDispatcher = __DEV__ ? setInvalidNestedHooksDispatcher() : null; + try { + const currentState: S = (queue.lastRenderedState: any); + const eagerState = lastRenderedReducer(currentState, action); + // Stash the eagerly computed state, and the reducer used to compute + // it, on the update object. If the reducer hasn't changed by the + // time we enter the render phase, then the eager state can be used + // without calling the reducer again. + update.eagerReducer = lastRenderedReducer; + update.eagerState = eagerState; + if (is(eagerState, currentState)) { + // Fast path. We can bail out without scheduling React to re-render. + // It's still possible that we'll need to rebase this update later, + // if the component re-renders for a different reason and by that + // time the reducer has changed. + return; + } + } catch (error) { + // Suppress the error. It will throw again in the render phase. + } finally { + if (__DEV__) { + ReactCurrentDispatcher.current = prevDispatcher; + } + } + } + } + if (__DEV__) { + // $FlowExpectedError - jest isn't a global, and isn't recognized outside of tests + if ('undefined' !== typeof jest) { + warnIfNotScopedWithMatchingAct(fiber); + warnIfNotCurrentlyActingUpdatesInDev(fiber); + } + } + + scheduleWork(fiber, expirationTime); +} + +function dispatchImplRenderPhase( + fiber: Fiber, + queue: UpdateQueue, + action: A, +) { + const update = requestRenderPhaseUpdate(fiber, action); + if (update === null) { + // This is an update on a fiber other than the one that is currently + // rendering. Fallback to default implementation + // TODO: This is undefined behavior. It should either warn or throw. + dispatchImplDefault(fiber, queue, action); + return; + } + + // 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; +} + +function dispatchImplInsideTransition( + fiber: Fiber, + queue: UpdateQueue, + action: A, +) { + const transition: TransitionInstance = (currentTransition: any); + setTransition(queue, transition); + + const continuation = dispatchForExpirationTime.bind( + null, + fiber, + queue, + action, + ); + if (dispatchContinuations === null) { + dispatchContinuations = [continuation]; + } else { + dispatchContinuations.push(continuation); + } +} + +export function setRenderPhaseDispatchImpl(): Dispatch { + const prev = dispatchImpl; + dispatchImpl = dispatchImplRenderPhase; + return prev; +} + +export function restoreDispatchImpl(prevDispatchImpl: Dispatch): void { + dispatchImpl = prevDispatchImpl; +} + +function classSetStateImplDefault(inst, payload, callback) { + const fiber = getInstance(inst); + const eventTime = requestCurrentTimeForUpdate(); + const suspenseConfig = requestCurrentSuspenseConfig(); + const expirationTime = computeExpirationForFiber( + eventTime, + fiber, + suspenseConfig, + ); + classSetStateForExpirationTime( + fiber, + payload, + callback, + eventTime, + expirationTime, + suspenseConfig, + ); +} + +function classReplaceStateImplDefault(inst, payload, callback) { + const fiber = getInstance(inst); + const eventTime = requestCurrentTimeForUpdate(); + const suspenseConfig = requestCurrentSuspenseConfig(); + const expirationTime = computeExpirationForFiber( + eventTime, + fiber, + suspenseConfig, + ); + classReplaceStateForExpirationTime( + fiber, + payload, + callback, + eventTime, + expirationTime, + suspenseConfig, + ); +} + +function classForceUpdateImplDefault(inst, callback) { + const fiber = getInstance(inst); + const eventTime = requestCurrentTimeForUpdate(); + const suspenseConfig = requestCurrentSuspenseConfig(); + const expirationTime = computeExpirationForFiber( + eventTime, + fiber, + suspenseConfig, + ); + classForceUpdateForExpirationTime( + fiber, + callback, + eventTime, + expirationTime, + suspenseConfig, + ); +} + +function classSetStateForExpirationTime( + fiber, + payload, + callback, + eventTime, + expirationTime, + suspenseConfig, +) { + const update = createUpdate(eventTime, expirationTime, suspenseConfig); + update.payload = payload; + if (callback !== undefined && callback !== null) { + if (__DEV__) { + warnOnInvalidCallback(callback, 'setState'); + } + update.callback = callback; + } + enqueueUpdate(fiber, update); + scheduleWork(fiber, expirationTime); +} + +function classReplaceStateForExpirationTime( + fiber, + payload, + callback, + eventTime, + expirationTime, + suspenseConfig, +) { + const update = createUpdate(eventTime, expirationTime, suspenseConfig); + update.tag = ReplaceState; + update.payload = payload; + if (callback !== undefined && callback !== null) { + if (__DEV__) { + warnOnInvalidCallback(callback, 'replaceState'); + } + update.callback = callback; + } + enqueueUpdate(fiber, update); + scheduleWork(fiber, expirationTime); +} + +function classForceUpdateForExpirationTime( + fiber, + callback, + eventTime, + expirationTime, + suspenseConfig, +) { + const update = createUpdate(eventTime, expirationTime, suspenseConfig); + update.tag = ForceUpdate; + if (callback !== undefined && callback !== null) { + if (__DEV__) { + warnOnInvalidCallback(callback, 'forceUpdate'); + } + update.callback = callback; + } + enqueueUpdate(fiber, update); + scheduleWork(fiber, expirationTime); +} + +function classSetStateImplInsideTransition(inst, payload, callback) { + const fiber = getInstance(inst); + const updateQueue = fiber.updateQueue; + const sharedQueue = updateQueue !== null ? updateQueue.shared : null; + if (sharedQueue === null) { + // TODO: Fire warning for update on unmounted component + return; + } + + const transition: TransitionInstance = (currentTransition: any); + setTransition(sharedQueue, transition); + + const continuation = classSetStateForExpirationTime.bind( + null, + fiber, + payload, + callback, + ); + if (dispatchContinuations === null) { + dispatchContinuations = [continuation]; + } else { + dispatchContinuations.push(continuation); + } +} + +function classReplaceStateImplInsideTransition(inst, payload, callback) { + const fiber = getInstance(inst); + const updateQueue = fiber.updateQueue; + const sharedQueue = updateQueue !== null ? updateQueue.shared : null; + if (sharedQueue === null) { + // TODO: Fire warning for update on unmounted component + return; + } + + const transition: TransitionInstance = (currentTransition: any); + setTransition(sharedQueue, transition); -export function requestCurrentTransition(): TransitionInstance | null { - return currentTransition; + const continuation = classReplaceStateForExpirationTime.bind( + null, + fiber, + payload, + callback, + ); + if (dispatchContinuations === null) { + dispatchContinuations = [continuation]; + } else { + dispatchContinuations.push(continuation); + } +} + +function classForceUpdateImplInsideTransition(inst, callback) { + const fiber = getInstance(inst); + const updateQueue = fiber.updateQueue; + const sharedQueue = updateQueue !== null ? updateQueue.shared : null; + if (sharedQueue === null) { + // TODO: Fire warning for update on unmounted component + return; + } + + const transition: TransitionInstance = (currentTransition: any); + setTransition(sharedQueue, transition); + + const continuation = classForceUpdateForExpirationTime.bind( + null, + fiber, + callback, + ); + if (dispatchContinuations === null) { + dispatchContinuations = [continuation]; + } else { + dispatchContinuations.push(continuation); + } } export function startTransition( @@ -51,12 +513,21 @@ export function startTransition( config: SuspenseConfig | null | void, callback: () => void, ) { - const fiber = transitionInstance.fiber; + if (dispatchImpl === dispatchImplRenderPhase) { + // Wrapping an update in `startTransition` has no effect in the + // render phase. + // TODO: This should warn. + callback(); + return; + } - const resolvedConfig: SuspenseConfig | null = - config === undefined ? null : config; + const suspenseConfig = config === undefined ? null : config; + const fiber = transitionInstance.fiber; - const currentTime = requestCurrentTimeForUpdate(); + // Don't need to reset this at the end because it's impossible to read + // from outside of a `startTransition` callback. + currentTransitionEventTime = requestCurrentTimeForUpdate(); + currentTransitionResolvedTime = NoWork; // TODO: runWithPriority shouldn't be necessary here. React should manage its // own concept of priority, and only consult Scheduler for updates that are @@ -65,51 +536,110 @@ export function startTransition( runWithPriority( priorityLevel < UserBlockingPriority ? UserBlockingPriority : priorityLevel, () => { - userBlockingExpirationTime = computeExpirationForFiber( - currentTime, + currentTransitionPendingTime = computeExpirationForFiber( + currentTransitionEventTime, 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; + const previousDispatchContinuations = dispatchContinuations; + const previousDispatchImpl = dispatchImpl; + const previousClassSetStateImpl = classSetStateImpl; + const previousClassReplaceStateImpl = classReplaceStateImpl; + const previousClassForceUpdateImpl = classForceUpdateImpl; + ReactCurrentBatchConfig.suspense = suspenseConfig; currentTransition = transitionInstance; + dispatchContinuations = null; + dispatchImpl = dispatchImplInsideTransition; + classSetStateImpl = classSetStateImplInsideTransition; + classReplaceStateImpl = classReplaceStateImplInsideTransition; + classForceUpdateImpl = classForceUpdateImplInsideTransition; try { callback(); } finally { + if (currentTransitionResolvedTime === NoWork) { + // This transition did not overlap with any previous transitions. + // Compute a new concurrent expiration time. + currentTransitionResolvedTime = computeExpirationForFiber( + currentTransitionEventTime, + fiber, + suspenseConfig, + ); + } + + // 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. + transitionInstance.pendingTime = currentTransitionPendingTime; + transitionInstance.resolvedTime = currentTransitionResolvedTime; + transitionInstance.version++; + + const continuations = dispatchContinuations; + const eventTime = currentTransitionEventTime; + const pendingTime = currentTransitionPendingTime; + const resolvedTime = currentTransitionResolvedTime; + ReactCurrentBatchConfig.suspense = previousConfig; currentTransition = previousTransition; - userBlockingExpirationTime = NoWork; + dispatchContinuations = previousDispatchContinuations; + dispatchImpl = previousDispatchImpl; + classSetStateImpl = previousClassSetStateImpl; + classReplaceStateImpl = previousClassReplaceStateImpl; + classForceUpdateImpl = previousClassForceUpdateImpl; + currentTransitionPendingTime = NoWork; + + if (continuations !== null) { + // Don't need to schedule work at the resolved time because the + // pending time is always higher priority. + scheduleUpdateOnFiber(fiber, pendingTime); + + // These continuations are internal functions that should never throw, + // but just in case, do them at the very end, after all the + // clean-up code. + for (let i = 0; i < continuations.length; i++) { + const continuation = continuations[i]; + continuation(eventTime, resolvedTime, suspenseConfig); + } + } } }, ); } -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); +function setTransition( + queue: {pendingTransition: TransitionInstance | null}, + transition: TransitionInstance, +) { + const prevTransition = queue.pendingTransition; + if (transition !== prevTransition) { + queue.pendingTransition = transition; + if (prevTransition !== 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. + + // Track the expiration time of the superseded transition. If there are + // multiple, choose the highest priority one. + if (preventIntermediateStates) { + const resolvedTime = prevTransition.resolvedTime; + if (currentTransitionResolvedTime < resolvedTime) { + currentTransitionResolvedTime = resolvedTime; + } + } + + // 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.resolvedTime = currentTransitionPendingTime; + prevTransition.version++; + scheduleUpdateOnFiber(prevTransition.fiber, currentTransitionPendingTime); + } + } } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index ecc6ff94d134..3ebe8074dfe7 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -127,7 +127,6 @@ import { throwException, createRootErrorUpdate, createClassErrorUpdate, - SuspendOnTask, } from './ReactFiberThrow'; import { commitBeforeMutationLifeCycles as commitBeforeMutationEffectOnFiber, @@ -183,7 +182,10 @@ import { clearCaughtError, } from 'shared/ReactErrorUtils'; import {onCommitRoot} from './ReactFiberDevToolsHook'; -import {requestCurrentTransition} from './ReactFiberTransition'; +import { + setRenderPhaseDispatchImpl, + restoreDispatchImpl, +} from './ReactFiberTransition'; const ceil = Math.ceil; @@ -203,14 +205,13 @@ const LegacyUnbatchedContext = /* */ 0b001000; const RenderContext = /* */ 0b010000; const CommitContext = /* */ 0b100000; -type RootExitStatus = 0 | 1 | 2 | 3 | 4 | 5 | 6; +type RootExitStatus = 0 | 1 | 2 | 3 | 4 | 5; const RootIncomplete = 0; const RootFatalErrored = 1; -const RootSuspendedOnTask = 2; -const RootErrored = 3; -const RootSuspended = 4; -const RootSuspendedWithDelay = 5; -const RootCompleted = 6; +const RootErrored = 2; +const RootSuspended = 3; +const RootSuspendedWithDelay = 4; +const RootCompleted = 5; export type Thenable = { then(resolve: () => mixed, reject?: () => mixed): Thenable | void, @@ -241,7 +242,7 @@ 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. @@ -334,13 +335,9 @@ export function computeExpirationForFiber( let expirationTime; if (suspenseConfig !== null) { - // This is a transition - const transitionInstance = requestCurrentTransition(); - if (transitionInstance !== null) { - expirationTime = transitionInstance.pendingExpirationTime; - } else { - expirationTime = computeAsyncExpiration(currentTime); - } + // If there's a SuspenseConfig, treat this as a concurrent update regardless + // of the priority. + expirationTime = computeAsyncExpiration(currentTime); } else { // Compute an expiration time based on the Scheduler priority. switch (priorityLevel) { @@ -687,6 +684,7 @@ function performConcurrentWorkOnRoot(root, didTimeout) { const prevExecutionContext = executionContext; executionContext |= RenderContext; const prevDispatcher = pushDispatcher(root); + const prevDispatchImpl = setRenderPhaseDispatchImpl(); const prevInteractions = pushInteractions(root); startWorkLoopTimer(workInProgress); do { @@ -700,6 +698,7 @@ function performConcurrentWorkOnRoot(root, didTimeout) { resetContextDependencies(); executionContext = prevExecutionContext; popDispatcher(prevDispatcher); + restoreDispatchImpl(prevDispatchImpl); if (enableSchedulerTracing) { popInteractions(((prevInteractions: any): Set)); } @@ -713,12 +712,7 @@ function performConcurrentWorkOnRoot(root, didTimeout) { throw fatalError; } - 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) { + if (workInProgress !== null) { // There's still work left over. Exit without committing. stopInterruptedWorkLoopTimer(); } else { @@ -759,8 +753,7 @@ function finishConcurrentRender( switch (exitStatus) { case RootIncomplete: - case RootFatalErrored: - case RootSuspendedOnTask: { + case RootFatalErrored: { invariant(false, 'Root did not complete. This is a bug in React.'); } // Flow knows about invariant, so it complains if I add a break @@ -1006,6 +999,7 @@ function performSyncWorkOnRoot(root) { executionContext |= RenderContext; const prevDispatcher = pushDispatcher(root); const prevInteractions = pushInteractions(root); + const prevDispatchImpl = setRenderPhaseDispatchImpl(); startWorkLoopTimer(workInProgress); do { @@ -1019,6 +1013,7 @@ function performSyncWorkOnRoot(root) { resetContextDependencies(); executionContext = prevExecutionContext; popDispatcher(prevDispatcher); + restoreDispatchImpl(prevDispatchImpl); if (enableSchedulerTracing) { popInteractions(((prevInteractions: any): Set)); } @@ -1032,12 +1027,7 @@ function performSyncWorkOnRoot(root) { throw fatalError; } - 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) { + if (workInProgress !== null) { // This is a sync render, so we should have finished the whole tree. invariant( false, @@ -1265,7 +1255,6 @@ function prepareFreshStack(root, expirationTime) { workInProgressRootLatestSuspenseTimeout = Sync; workInProgressRootCanSuspendUsingConfig = null; workInProgressRootNextUnprocessedUpdateTime = NoWork; - workInProgressRootRestartTime = NoWork; workInProgressRootHasPendingPing = false; if (enableSchedulerTracing) { @@ -1286,20 +1275,6 @@ function handleError(root, thrownValue) { 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 @@ -2623,17 +2598,15 @@ if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) { try { return originalBeginWork(current, unitOfWork, expirationTime); } catch (originalError) { - // Filter out special exception types if ( originalError !== null && typeof originalError === 'object' && - // Promise - (typeof originalError.then === 'function' || - // SuspendOnTask exception - originalError instanceof SuspendOnTask) + typeof originalError.then === 'function' ) { + // 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(); diff --git a/packages/react-reconciler/src/ReactUpdateQueue.js b/packages/react-reconciler/src/ReactUpdateQueue.js index 70908de85325..c7077b894590 100644 --- a/packages/react-reconciler/src/ReactUpdateQueue.js +++ b/packages/react-reconciler/src/ReactUpdateQueue.js @@ -97,26 +97,17 @@ import { } from './ReactFiberNewContext'; import {Callback, ShouldCapture, DidCapture} from 'shared/ReactSideEffectTags'; -import { - debugRenderPhaseSideEffectsForStrictMode, - preventIntermediateStates, -} from 'shared/ReactFeatureFlags'; +import {debugRenderPhaseSideEffectsForStrictMode} 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. @@ -239,20 +230,6 @@ 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 && @@ -422,11 +399,8 @@ 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 @@ -434,7 +408,7 @@ export function processUpdateQueue( const clone: Update = { eventTime: update.eventTime, expirationTime: updateExpirationTime, - suspenseConfig, + suspenseConfig: update.suspenseConfig, tag: update.tag, payload: update.payload, @@ -452,16 +426,6 @@ 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; @@ -508,17 +472,6 @@ 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; @@ -534,17 +487,6 @@ 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__/ReactTransition-test.internal.js b/packages/react-reconciler/src/__tests__/ReactTransition-test.internal.js index 7c3f8a43019e..6adc99dfa392 100644 --- a/packages/react-reconciler/src/__tests__/ReactTransition-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactTransition-test.internal.js @@ -106,7 +106,7 @@ describe('ReactTransition', () => { ); it.experimental( - 'isPending turns off immediately if `startTransition` does not include any updates', + 'nothing is scheduled if `startTransition` does not include any updates', async () => { let startTransition; function App() { @@ -128,13 +128,8 @@ describe('ReactTransition', () => { // 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', - ]); + // Nothing is scheduled + expect(Scheduler).toHaveYielded([]); expect(root).toMatchRenderedOutput('Pending: false'); }, ); @@ -598,76 +593,60 @@ describe('ReactTransition', () => { , ); - // Navigate to tab B await act(async () => { + // Navigate to tab B tabButtonB.current.go(); - expect(Scheduler).toFlushAndYieldThrough([ + expect(Scheduler).toFlushAndYield([ // Turn on B's pending state 'Tab B (pending...)', - // Partially render B 'App', 'Tab A', 'Tab B', + 'Tab C', + 'Suspend! [B]', + 'Loading...', ]); - // 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...)', + jest.advanceTimersByTime(2000); + Scheduler.unstable_advanceTime(2000); - // 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 - , - ); + // Navigate to tab C + tabButtonC.current.go(); + expect(Scheduler).toFlushAndYield([ + // Turn off B's pending state, and turn on C's + 'Tab B', + 'Tab C (pending...)', + // Partially render B + 'App', + 'Tab A', + 'Tab B', + 'Tab C', + 'Suspend! [C]', + 'Loading...', + ]); - // 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 B. + await ContentB.resolve(); + // B is not able to finish because C is in the same batch. + expect(Scheduler).toFlushAndYield([ + 'App', + 'Tab A', + 'Tab B', + 'Tab C', + 'Suspend! [C]', + 'Loading...', + ]); - // Finish loading C - await act(async () => { - ContentC.resolve(); + // Finish loading C. + await ContentC.resolve(); + expect(Scheduler).toFlushAndYield([ + 'App', + 'Tab A', + 'Tab B', + 'Tab C', + 'C', + ]); }); - expect(Scheduler).toHaveYielded(['App', 'Tab A', 'Tab B', 'Tab C', 'C']); expect(root).toMatchRenderedOutput( <>
    @@ -777,79 +756,60 @@ describe('ReactTransition', () => { , ); - // Navigate to tab B await act(async () => { + // Navigate to tab B tabButtonB.current.go(); - expect(Scheduler).toFlushAndYieldThrough([ + expect(Scheduler).toFlushAndYield([ // Turn on B's pending state 'Tab B (pending...)', - // Partially render B 'App', 'Tab A', 'Tab B', + 'Tab C', + 'Suspend! [B]', + 'Loading...', ]); - // 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...)', + jest.advanceTimersByTime(2000); + Scheduler.unstable_advanceTime(2000); - // 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 - , - ); + // Navigate to tab C + tabButtonC.current.go(); + expect(Scheduler).toFlushAndYield([ + // Turn off B's pending state, and turn on C's + 'Tab B', + 'Tab C (pending...)', + // Partially render B + 'App', + 'Tab A', + 'Tab B', + 'Tab C', + 'Suspend! [C]', + 'Loading...', + ]); - // 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 B. + await ContentB.resolve(); + // B is not able to finish because C is in the same batch. + expect(Scheduler).toFlushAndYield([ + 'App', + 'Tab A', + 'Tab B', + 'Tab C', + 'Suspend! [C]', + 'Loading...', + ]); - // Finish loading C - await act(async () => { - ContentC.resolve(); + // Finish loading C. + await ContentC.resolve(); + expect(Scheduler).toFlushAndYield([ + 'App', + 'Tab A', + 'Tab B', + 'Tab C', + 'C', + ]); }); - expect(Scheduler).toHaveYielded(['App', 'Tab A', 'Tab B', 'Tab C', 'C']); expect(root).toMatchRenderedOutput( <>