From 8717cc306cf061aef9003de047ff1fba2f09de1d Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Wed, 24 Jan 2024 23:54:53 -0500 Subject: [PATCH] Batch async actions even if useTransition is unmounted (#28078) If there are multiple updates inside an async action, they should all be rendered in the same batch, even if they are separate by an async operation (`await`). We currently implement this by suspending in the `useTransition` hook to block the update from committing until all possible updates have been scheduled by the action. The reason we did it this way is so you can "cancel" an action by navigating away from the UI that triggered it. The problem with that approach, though, is that even if you navigate away from the `useTransition` hook, the action may have updated shared parts of the UI that are still in the tree. So we may need to continue suspending even after the `useTransition` hook is deleted. In other words, the lifetime of an async action scope is longer than the lifetime of a particular `useTransition` hook. The solution is to suspend whenever _any_ update that is part of the async action scope is unwrapped during render. So, inside useState and useReducer. This fixes a related issue where an optimistic update is reverted before the async action has finished, because we were relying on the `useTransition` hook to prevent the optimistic update from finishing. This also prepares us to support async actions being passed to the non-hook form of `startTransition` (though this isn't implemented yet). --- .../src/ReactFiberAsyncAction.js | 187 ++++----- .../src/ReactFiberBeginWork.js | 7 + .../src/ReactFiberClassComponent.js | 4 + .../src/ReactFiberClassUpdateQueue.js | 37 ++ .../react-reconciler/src/ReactFiberHooks.js | 56 ++- .../react-reconciler/src/ReactFiberThrow.js | 27 +- .../src/ReactFiberWorkLoop.js | 66 ++-- .../src/__tests__/ReactAsyncActions-test.js | 371 ++++++++++++++++++ 8 files changed, 582 insertions(+), 173 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberAsyncAction.js b/packages/react-reconciler/src/ReactFiberAsyncAction.js index 7382aeee89e9f..98f7ca175c0a8 100644 --- a/packages/react-reconciler/src/ReactFiberAsyncAction.js +++ b/packages/react-reconciler/src/ReactFiberAsyncAction.js @@ -9,7 +9,6 @@ import type { Thenable, - PendingThenable, FulfilledThenable, RejectedThenable, } from 'shared/ReactTypes'; @@ -32,111 +31,32 @@ let currentEntangledListeners: Array<() => mixed> | null = null; let currentEntangledPendingCount: number = 0; // The transition lane shared by all updates in the entangled scope. let currentEntangledLane: Lane = NoLane; +// A thenable that resolves when the entangled scope completes. It does not +// resolve to a particular value because it's only used for suspending the UI +// until the async action scope has completed. +let currentEntangledActionThenable: Thenable | null = null; -export function requestAsyncActionContext( - actionReturnValue: Thenable, - // If this is provided, this resulting thenable resolves to this value instead - // of the return value of the action. This is a perf trick to avoid composing - // an extra async function. - overrideReturnValue: S | null, -): Thenable { - // This is an async action. - // - // Return a thenable that resolves once the action scope (i.e. the async - // function passed to startTransition) has finished running. - - const thenable: Thenable = (actionReturnValue: any); - let entangledListeners; +export function entangleAsyncAction(thenable: Thenable): Thenable { + // `thenable` is the return value of the async action scope function. Create + // a combined thenable that resolves once every entangled scope function + // has finished. if (currentEntangledListeners === null) { // There's no outer async action scope. Create a new one. - entangledListeners = currentEntangledListeners = []; + const entangledListeners = (currentEntangledListeners = []); currentEntangledPendingCount = 0; currentEntangledLane = requestTransitionLane(); - } else { - entangledListeners = currentEntangledListeners; + const entangledThenable: Thenable = { + status: 'pending', + value: undefined, + then(resolve: void => mixed) { + entangledListeners.push(resolve); + }, + }; + currentEntangledActionThenable = entangledThenable; } - currentEntangledPendingCount++; - - // Create a thenable that represents the result of this action, but doesn't - // resolve until the entire entangled scope has finished. - // - // Expressed using promises: - // const [thisResult] = await Promise.all([thisAction, entangledAction]); - // return thisResult; - const resultThenable = createResultThenable(entangledListeners); - - let resultStatus = 'pending'; - let resultValue; - let rejectedReason; - thenable.then( - (value: S) => { - resultStatus = 'fulfilled'; - resultValue = overrideReturnValue !== null ? overrideReturnValue : value; - pingEngtangledActionScope(); - }, - error => { - resultStatus = 'rejected'; - rejectedReason = error; - pingEngtangledActionScope(); - }, - ); - - // Attach a listener to fill in the result. - entangledListeners.push(() => { - switch (resultStatus) { - case 'fulfilled': { - const fulfilledThenable: FulfilledThenable = (resultThenable: any); - fulfilledThenable.status = 'fulfilled'; - fulfilledThenable.value = resultValue; - break; - } - case 'rejected': { - const rejectedThenable: RejectedThenable = (resultThenable: any); - rejectedThenable.status = 'rejected'; - rejectedThenable.reason = rejectedReason; - break; - } - case 'pending': - default: { - // The listener above should have been called first, so `resultStatus` - // should already be set to the correct value. - throw new Error( - 'Thenable should have already resolved. This ' + 'is a bug in React.', - ); - } - } - }); - - return resultThenable; -} - -export function requestSyncActionContext( - actionReturnValue: any, - // If this is provided, this resulting thenable resolves to this value instead - // of the return value of the action. This is a perf trick to avoid composing - // an extra async function. - overrideReturnValue: S | null, -): Thenable | S { - const resultValue: S = - overrideReturnValue !== null - ? overrideReturnValue - : (actionReturnValue: any); - // This is not an async action, but it may be part of an outer async action. - if (currentEntangledListeners === null) { - return resultValue; - } else { - // Return a thenable that does not resolve until the entangled actions - // have finished. - const entangledListeners = currentEntangledListeners; - const resultThenable = createResultThenable(entangledListeners); - entangledListeners.push(() => { - const fulfilledThenable: FulfilledThenable = (resultThenable: any); - fulfilledThenable.status = 'fulfilled'; - fulfilledThenable.value = resultValue; - }); - return resultThenable; - } + thenable.then(pingEngtangledActionScope, pingEngtangledActionScope); + return thenable; } function pingEngtangledActionScope() { @@ -146,9 +66,15 @@ function pingEngtangledActionScope() { ) { // All the actions have finished. Close the entangled async action scope // and notify all the listeners. + if (currentEntangledActionThenable !== null) { + const fulfilledThenable: FulfilledThenable = + (currentEntangledActionThenable: any); + fulfilledThenable.status = 'fulfilled'; + } const listeners = currentEntangledListeners; currentEntangledListeners = null; currentEntangledLane = NoLane; + currentEntangledActionThenable = null; for (let i = 0; i < listeners.length; i++) { const listener = listeners[i]; listener(); @@ -156,31 +82,58 @@ function pingEngtangledActionScope() { } } -function createResultThenable( - entangledListeners: Array<() => mixed>, -): Thenable { - // Waits for the entangled async action to complete, then resolves to the - // result of an individual action. - const resultThenable: PendingThenable = { +export function chainThenableValue( + thenable: Thenable, + result: T, +): Thenable { + // Equivalent to: Promise.resolve(thenable).then(() => result), except we can + // cheat a bit since we know that that this thenable is only ever consumed + // by React. + // + // We don't technically require promise support on the client yet, hence this + // extra code. + const listeners = []; + const thenableWithOverride: Thenable = { status: 'pending', value: null, reason: null, - then(resolve: S => mixed) { - // This is a bit of a cheat. `resolve` expects a value of type `S` to be - // passed, but because we're instrumenting the `status` field ourselves, - // and we know this thenable will only be used by React, we also know - // the value isn't actually needed. So we add the resolve function - // directly to the entangled listeners. - // - // This is also why we don't need to check if the thenable is still - // pending; the Suspense implementation already performs that check. - const ping: () => mixed = (resolve: any); - entangledListeners.push(ping); + then(resolve: T => mixed) { + listeners.push(resolve); }, }; - return resultThenable; + thenable.then( + (value: T) => { + const fulfilledThenable: FulfilledThenable = + (thenableWithOverride: any); + fulfilledThenable.status = 'fulfilled'; + fulfilledThenable.value = result; + for (let i = 0; i < listeners.length; i++) { + const listener = listeners[i]; + listener(result); + } + }, + error => { + const rejectedThenable: RejectedThenable = (thenableWithOverride: any); + rejectedThenable.status = 'rejected'; + rejectedThenable.reason = error; + for (let i = 0; i < listeners.length; i++) { + const listener = listeners[i]; + // This is a perf hack where we call the `onFulfill` ping function + // instead of `onReject`, because we know that React is the only + // consumer of these promises, and it passes the same listener to both. + // We also know that it will read the error directly off the + // `.reason` field. + listener((undefined: any)); + } + }, + ); + return thenableWithOverride; } export function peekEntangledActionLane(): Lane { return currentEntangledLane; } + +export function peekEntangledActionThenable(): Thenable | null { + return currentEntangledActionThenable; +} diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index dcdc62d223911..69c8811a8b0d2 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -137,6 +137,7 @@ import { cloneUpdateQueue, initializeUpdateQueue, enqueueCapturedUpdate, + suspendIfUpdateReadFromEntangledAsyncAction, } from './ReactFiberClassUpdateQueue'; import { NoLane, @@ -945,6 +946,7 @@ function updateCacheComponent( if (includesSomeLane(current.lanes, renderLanes)) { cloneUpdateQueue(current, workInProgress); processUpdateQueue(workInProgress, null, null, renderLanes); + suspendIfUpdateReadFromEntangledAsyncAction(); } const prevState: CacheComponentState = current.memoizedState; const nextState: CacheComponentState = workInProgress.memoizedState; @@ -1476,6 +1478,11 @@ function updateHostRoot( } } + // This would ideally go inside processUpdateQueue, but because it suspends, + // it needs to happen after the `pushCacheProvider` call above to avoid a + // context stack mismatch. A bit unfortunate. + suspendIfUpdateReadFromEntangledAsyncAction(); + // Caution: React DevTools currently depends on this property // being called "element". const nextChildren = nextState.element; diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.js b/packages/react-reconciler/src/ReactFiberClassComponent.js index c816fb0ba21ea..cceb78c8ed878 100644 --- a/packages/react-reconciler/src/ReactFiberClassComponent.js +++ b/packages/react-reconciler/src/ReactFiberClassComponent.js @@ -53,6 +53,7 @@ import { ForceUpdate, initializeUpdateQueue, cloneUpdateQueue, + suspendIfUpdateReadFromEntangledAsyncAction, } from './ReactFiberClassUpdateQueue'; import {NoLanes} from './ReactFiberLane'; import { @@ -892,6 +893,7 @@ function mountClassInstance( // If we had additional state updates during this life-cycle, let's // process them now. processUpdateQueue(workInProgress, newProps, instance, renderLanes); + suspendIfUpdateReadFromEntangledAsyncAction(); instance.state = workInProgress.memoizedState; } @@ -959,6 +961,7 @@ function resumeMountClassInstance( const oldState = workInProgress.memoizedState; let newState = (instance.state = oldState); processUpdateQueue(workInProgress, newProps, instance, renderLanes); + suspendIfUpdateReadFromEntangledAsyncAction(); newState = workInProgress.memoizedState; if ( oldProps === newProps && @@ -1109,6 +1112,7 @@ function updateClassInstance( const oldState = workInProgress.memoizedState; let newState = (instance.state = oldState); processUpdateQueue(workInProgress, newProps, instance, renderLanes); + suspendIfUpdateReadFromEntangledAsyncAction(); newState = workInProgress.memoizedState; if ( diff --git a/packages/react-reconciler/src/ReactFiberClassUpdateQueue.js b/packages/react-reconciler/src/ReactFiberClassUpdateQueue.js index 2e88c982022d3..d7a1efbb47f4a 100644 --- a/packages/react-reconciler/src/ReactFiberClassUpdateQueue.js +++ b/packages/react-reconciler/src/ReactFiberClassUpdateQueue.js @@ -125,6 +125,10 @@ import { import {setIsStrictModeForDevtools} from './ReactFiberDevToolsHook'; import assign from 'shared/assign'; +import { + peekEntangledActionLane, + peekEntangledActionThenable, +} from './ReactFiberAsyncAction'; export type Update = { lane: Lane, @@ -463,12 +467,38 @@ function getStateFromUpdate( return prevState; } +let didReadFromEntangledAsyncAction: boolean = false; + +// Each call to processUpdateQueue should be accompanied by a call to this. It's +// only in a separate function because in updateHostRoot, it must happen after +// all the context stacks have been pushed to, to prevent a stack mismatch. A +// bit unfortunate. +export function suspendIfUpdateReadFromEntangledAsyncAction() { + // Check if this update is part of a pending async action. If so, we'll + // need to suspend until the action has finished, so that it's batched + // together with future updates in the same action. + // TODO: Once we support hooks inside useMemo (or an equivalent + // memoization boundary like Forget), hoist this logic so that it only + // suspends if the memo boundary produces a new value. + if (didReadFromEntangledAsyncAction) { + const entangledActionThenable = peekEntangledActionThenable(); + if (entangledActionThenable !== null) { + // TODO: Instead of the throwing the thenable directly, throw a + // special object like `use` does so we can detect if it's captured + // by userspace. + throw entangledActionThenable; + } + } +} + export function processUpdateQueue( workInProgress: Fiber, props: any, instance: any, renderLanes: Lanes, ): void { + didReadFromEntangledAsyncAction = false; + // This is always non-null on a ClassComponent or HostRoot const queue: UpdateQueue = (workInProgress.updateQueue: any); @@ -571,6 +601,13 @@ export function processUpdateQueue( } else { // This update does have sufficient priority. + // Check if this update is part of a pending async action. If so, + // we'll need to suspend until the action has finished, so that it's + // batched together with future updates in the same action. + if (updateLane !== NoLane && updateLane === peekEntangledActionLane()) { + didReadFromEntangledAsyncAction = true; + } + if (newLastBaseUpdate !== null) { const clone: Update = { // This update is going to be committed so we never want uncommit diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 4d84caa737c5f..ce7d21d4e137f 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -145,9 +145,10 @@ import { import type {ThenableState} from './ReactFiberThenable'; import type {BatchConfigTransition} from './ReactFiberTracingMarkerComponent'; import { - requestAsyncActionContext, - requestSyncActionContext, + entangleAsyncAction, peekEntangledActionLane, + peekEntangledActionThenable, + chainThenableValue, } from './ReactFiberAsyncAction'; import {HostTransitionContext} from './ReactFiberHostContext'; import {requestTransitionLane} from './ReactFiberRootScheduler'; @@ -1274,6 +1275,7 @@ function updateReducerImpl( let newBaseQueueFirst = null; let newBaseQueueLast: Update | null = null; let update = first; + let didReadFromEntangledAsyncAction = false; do { // An extra OffscreenLane bit is added to updates that were made to // a hidden tree, so that we can distinguish them from updates that were @@ -1317,6 +1319,13 @@ function updateReducerImpl( } else { // This update does have sufficient priority. + // Check if this update is part of a pending async action. If so, + // we'll need to suspend until the action has finished, so that it's + // batched together with future updates in the same action. + if (updateLane !== NoLane && updateLane === peekEntangledActionLane()) { + didReadFromEntangledAsyncAction = true; + } + // Check if this is an optimistic update. const revertLane = update.revertLane; if (!enableAsyncActions || revertLane === NoLane) { @@ -1407,6 +1416,22 @@ function updateReducerImpl( // different from the current state. if (!is(newState, hook.memoizedState)) { markWorkInProgressReceivedUpdate(); + + // Check if this update is part of a pending async action. If so, we'll + // need to suspend until the action has finished, so that it's batched + // together with future updates in the same action. + // TODO: Once we support hooks inside useMemo (or an equivalent + // memoization boundary like Forget), hoist this logic so that it only + // suspends if the memo boundary produces a new value. + if (didReadFromEntangledAsyncAction) { + const entangledActionThenable = peekEntangledActionThenable(); + if (entangledActionThenable !== null) { + // TODO: Instead of the throwing the thenable directly, throw a + // special object like `use` does so we can detect if it's captured + // by userspace. + throw entangledActionThenable; + } + } } hook.memoizedState = newState; @@ -1964,13 +1989,10 @@ function runFormStateAction( () => finishRunningFormStateAction(actionQueue, (setState: any)), ); - const entangledResult = requestAsyncActionContext(thenable, null); - setState((entangledResult: any)); + entangleAsyncAction>(thenable); + setState((thenable: any)); } else { - // This is either `returnValue` or a thenable that resolves to - // `returnValue`, depending on whether we're inside an async action scope. - const entangledResult = requestSyncActionContext(returnValue, null); - setState((entangledResult: any)); + setState((returnValue: any)); const nextState = ((returnValue: any): Awaited); actionQueue.state = nextState; @@ -2832,22 +2854,16 @@ function startTransition( typeof returnValue.then === 'function' ) { const thenable = ((returnValue: any): Thenable); - // This is a thenable that resolves to `finishedState` once the async - // action scope has finished. - const entangledResult = requestAsyncActionContext( + entangleAsyncAction(thenable); + // Create a thenable that resolves to `finishedState` once the async + // action has completed. + const thenableForFinishedState = chainThenableValue( thenable, finishedState, ); - dispatchSetState(fiber, queue, entangledResult); + dispatchSetState(fiber, queue, (thenableForFinishedState: any)); } else { - // This is either `finishedState` or a thenable that resolves to - // `finishedState`, depending on whether we're inside an async - // action scope. - const entangledResult = requestSyncActionContext( - returnValue, - finishedState, - ); - dispatchSetState(fiber, queue, entangledResult); + dispatchSetState(fiber, queue, finishedState); } } else { // Async actions are not enabled. diff --git a/packages/react-reconciler/src/ReactFiberThrow.js b/packages/react-reconciler/src/ReactFiberThrow.js index 789be43b88f36..7633e1c676713 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.js +++ b/packages/react-reconciler/src/ReactFiberThrow.js @@ -206,7 +206,7 @@ function resetSuspendedComponent(sourceFiber: Fiber, rootRenderLanes: Lanes) { function markSuspenseBoundaryShouldCapture( suspenseBoundary: Fiber, - returnFiber: Fiber, + returnFiber: Fiber | null, sourceFiber: Fiber, root: FiberRoot, rootRenderLanes: Lanes, @@ -319,11 +319,11 @@ function markSuspenseBoundaryShouldCapture( function throwException( root: FiberRoot, - returnFiber: Fiber, + returnFiber: Fiber | null, sourceFiber: Fiber, value: mixed, rootRenderLanes: Lanes, -): void { +): boolean { // The source fiber did not complete. sourceFiber.flags |= Incomplete; @@ -446,7 +446,7 @@ function throwException( attachPingListener(root, wakeable, rootRenderLanes); } } - return; + return false; } case OffscreenComponent: { if (suspenseBoundary.mode & ConcurrentMode) { @@ -476,7 +476,7 @@ function throwException( attachPingListener(root, wakeable, rootRenderLanes); } - return; + return false; } } } @@ -497,7 +497,7 @@ function throwException( // and potentially log a warning. Revisit this for a future release. attachPingListener(root, wakeable, rootRenderLanes); renderDidSuspendDelayIfPossible(); - return; + return false; } else { // In a legacy root, suspending without a boundary is always an error. const uncaughtSuspenseError = new Error( @@ -537,7 +537,7 @@ function throwException( // Even though the user may not be affected by this error, we should // still log it so it can be fixed. queueHydrationError(createCapturedValueAtFiber(value, sourceFiber)); - return; + return false; } } else { // Otherwise, fall through to the error path. @@ -549,6 +549,13 @@ function throwException( // We didn't find a boundary that could handle this type of exception. Start // over and traverse parent path again, this time treating the exception // as an error. + + if (returnFiber === null) { + // There's no return fiber, which means the root errored. This should never + // happen. Return `true` to trigger a fatal error (panic). + return true; + } + let workInProgress: Fiber = returnFiber; do { switch (workInProgress.tag) { @@ -559,7 +566,7 @@ function throwException( workInProgress.lanes = mergeLanes(workInProgress.lanes, lane); const update = createRootErrorUpdate(workInProgress, errorInfo, lane); enqueueCapturedUpdate(workInProgress, update); - return; + return false; } case ClassComponent: // Capture and retry @@ -583,7 +590,7 @@ function throwException( lane, ); enqueueCapturedUpdate(workInProgress, update); - return; + return false; } break; default: @@ -592,6 +599,8 @@ function throwException( // $FlowFixMe[incompatible-type] we bail out when we get a null workInProgress = workInProgress.return; } while (workInProgress !== null); + + return false; } export {throwException, createRootErrorUpdate, createClassErrorUpdate}; diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 6aaf0e936bd89..d487fdb205c5d 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -1994,7 +1994,7 @@ function renderRootSync(root: FiberRoot, lanes: Lanes) { // Unwind then continue with the normal work loop. workInProgressSuspendedReason = NotSuspended; workInProgressThrownValue = null; - throwAndUnwindWorkLoop(unitOfWork, thrownValue); + throwAndUnwindWorkLoop(root, unitOfWork, thrownValue); break; } } @@ -2114,7 +2114,7 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { // Unwind then continue with the normal work loop. workInProgressSuspendedReason = NotSuspended; workInProgressThrownValue = null; - throwAndUnwindWorkLoop(unitOfWork, thrownValue); + throwAndUnwindWorkLoop(root, unitOfWork, thrownValue); break; } case SuspendedOnData: { @@ -2172,7 +2172,7 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { // Otherwise, unwind then continue with the normal work loop. workInProgressSuspendedReason = NotSuspended; workInProgressThrownValue = null; - throwAndUnwindWorkLoop(unitOfWork, thrownValue); + throwAndUnwindWorkLoop(root, unitOfWork, thrownValue); } break; } @@ -2229,7 +2229,7 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { // Otherwise, unwind then continue with the normal work loop. workInProgressSuspendedReason = NotSuspended; workInProgressThrownValue = null; - throwAndUnwindWorkLoop(unitOfWork, thrownValue); + throwAndUnwindWorkLoop(root, unitOfWork, thrownValue); break; } case SuspendedOnDeprecatedThrowPromise: { @@ -2239,7 +2239,7 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { // always unwind. workInProgressSuspendedReason = NotSuspended; workInProgressThrownValue = null; - throwAndUnwindWorkLoop(unitOfWork, thrownValue); + throwAndUnwindWorkLoop(root, unitOfWork, thrownValue); break; } case SuspendedOnHydration: { @@ -2464,7 +2464,11 @@ function replaySuspendedUnitOfWork(unitOfWork: Fiber): void { ReactCurrentOwner.current = null; } -function throwAndUnwindWorkLoop(unitOfWork: Fiber, thrownValue: mixed) { +function throwAndUnwindWorkLoop( + root: FiberRoot, + unitOfWork: Fiber, + thrownValue: mixed, +) { // This is a fork of performUnitOfWork specifcally for unwinding a fiber // that threw an exception. // @@ -2473,40 +2477,32 @@ function throwAndUnwindWorkLoop(unitOfWork: Fiber, thrownValue: mixed) { resetSuspendedWorkLoopOnUnwind(unitOfWork); const returnFiber = unitOfWork.return; - if (returnFiber === null || workInProgressRoot === 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 - // supposed to capture all errors that weren't caught by an error - // boundary. - workInProgressRootExitStatus = RootFatalErrored; - workInProgressRootFatalError = thrownValue; - // Set `workInProgress` to null. This represents advancing to the next - // sibling, or the parent if there are no siblings. But since the root - // has no siblings nor a parent, we set it to null. Usually this is - // handled by `completeUnitOfWork` or `unwindWork`, but since we're - // intentionally not calling those, we need set it here. - // TODO: Consider calling `unwindWork` to pop the contexts. - workInProgress = null; - return; - } - try { // Find and mark the nearest Suspense or error boundary that can handle // this "exception". - throwException( - workInProgressRoot, + const didFatal = throwException( + root, returnFiber, unitOfWork, thrownValue, workInProgressRootRenderLanes, ); + if (didFatal) { + panicOnRootError(thrownValue); + return; + } } catch (error) { // We had trouble processing the error. An example of this happening is // when accessing the `componentDidCatch` property of an error boundary // throws an error. A weird edge case. There's a regression test for this. // To prevent an infinite loop, bubble the error up to the next parent. - workInProgress = returnFiber; - throw error; + if (returnFiber !== null) { + workInProgress = returnFiber; + throw error; + } else { + panicOnRootError(thrownValue); + return; + } } if (unitOfWork.flags & Incomplete) { @@ -2526,6 +2522,22 @@ function throwAndUnwindWorkLoop(unitOfWork: Fiber, thrownValue: mixed) { } } +function panicOnRootError(error: mixed) { + // There's no ancestor that can handle this exception. This should never + // happen because the root is supposed to capture all errors that weren't + // caught by an error boundary. This is a fatal error, or panic condition, + // because we've run out of ways to recover. + workInProgressRootExitStatus = RootFatalErrored; + workInProgressRootFatalError = error; + // Set `workInProgress` to null. This represents advancing to the next + // sibling, or the parent if there are no siblings. But since the root + // has no siblings nor a parent, we set it to null. Usually this is + // handled by `completeUnitOfWork` or `unwindWork`, but since we're + // intentionally not calling those, we need set it here. + // TODO: Consider calling `unwindWork` to pop the contexts. + workInProgress = null; +} + function completeUnitOfWork(unitOfWork: Fiber): void { // Attempt to complete the current unit of work, then move to the next // sibling. If there are no more siblings, return to the parent fiber. diff --git a/packages/react-reconciler/src/__tests__/ReactAsyncActions-test.js b/packages/react-reconciler/src/__tests__/ReactAsyncActions-test.js index 1be91d36084e6..1f45fd84d0430 100644 --- a/packages/react-reconciler/src/__tests__/ReactAsyncActions-test.js +++ b/packages/react-reconciler/src/__tests__/ReactAsyncActions-test.js @@ -1270,4 +1270,375 @@ describe('ReactAsyncActions', () => { assertLog(['Loading... (25%)', 'A', 'B']); expect(root).toMatchRenderedOutput(
B
); }); + + // @gate enableAsyncActions + test( + 'optimistic state is not reverted until async action finishes, even if ' + + 'useTransition hook is unmounted', + async () => { + let startTransition; + function Updater() { + const [isPending, _start] = useTransition(); + startTransition = _start; + return ( + + + + ); + } + + let setText; + let setOptimisticText; + function Sibling() { + const [canonicalText, _setText] = useState('A'); + setText = _setText; + + const [text, _setOptimisticText] = useOptimistic( + canonicalText, + (_, optimisticText) => `${optimisticText} (loading...)`, + ); + setOptimisticText = _setOptimisticText; + + return ( + + + + ); + } + + function App({showUpdater}) { + return ( + <> + {showUpdater ? : null} + + + ); + } + + const root = ReactNoop.createRoot(); + await act(() => { + root.render(); + }); + assertLog(['Pending: false', 'A']); + expect(root).toMatchRenderedOutput( + <> + Pending: false + A + , + ); + + // Start an async action that has multiple updates with async + // operations in between. + await act(() => { + startTransition(async () => { + Scheduler.log('Async action started'); + + setOptimisticText('C'); + + startTransition(() => setText('B')); + + await getText('Wait before updating to C'); + + Scheduler.log('Async action ended'); + startTransition(() => setText('C')); + }); + }); + assertLog([ + 'Async action started', + 'Pending: true', + // Render an optimistic value + 'C (loading...)', + ]); + expect(root).toMatchRenderedOutput( + <> + Pending: true + C (loading...) + , + ); + + // Delete the component that contains the useTransition hook. This + // component no longer blocks the transition from completing. But the + // we're still showing an optimistic state, because the async action has + // not yet finished. + await act(() => { + root.render(); + }); + assertLog(['C (loading...)']); + expect(root).toMatchRenderedOutput(C (loading...)); + + // Finish the async action. Now the optimistic state is reverted and we + // switch to the canonical value. + await act(() => resolveText('Wait before updating to C')); + assertLog(['Async action ended', 'C']); + expect(root).toMatchRenderedOutput(C); + }, + ); + + // @gate enableAsyncActions + test( + 'updates in an async action are entangled even if useTransition hook ' + + 'is unmounted before it finishes', + async () => { + let startTransition; + function Updater() { + const [isPending, _start] = useTransition(); + startTransition = _start; + return ( + + + + ); + } + + let setText; + function Sibling() { + const [text, _setText] = useState('A'); + setText = _setText; + return ( + + + + ); + } + + function App({showUpdater}) { + return ( + <> + {showUpdater ? : null} + + + ); + } + + const root = ReactNoop.createRoot(); + await act(() => { + root.render(); + }); + assertLog(['Pending: false', 'A']); + expect(root).toMatchRenderedOutput( + <> + Pending: false + A + , + ); + + // Start an async action that has multiple updates with async + // operations in between. + await act(() => { + startTransition(async () => { + Scheduler.log('Async action started'); + startTransition(() => setText('B')); + + await getText('Wait before updating to C'); + + Scheduler.log('Async action ended'); + startTransition(() => setText('C')); + }); + }); + assertLog(['Async action started', 'Pending: true']); + expect(root).toMatchRenderedOutput( + <> + Pending: true + A + , + ); + + // Delete the component that contains the useTransition hook. This + // component no longer blocks the transition from completing. But the + // pending update to Sibling should not be allowed to finish, because it's + // part of the async action. + await act(() => { + root.render(); + }); + assertLog(['A']); + expect(root).toMatchRenderedOutput(A); + + // Finish the async action. Notice the intermediate B state was never + // shown, because it was batched with the update that came later in the + // same action. + await act(() => resolveText('Wait before updating to C')); + assertLog(['Async action ended', 'C']); + expect(root).toMatchRenderedOutput(C); + }, + ); + + // @gate enableAsyncActions + test( + 'updates in an async action are entangled even if useTransition hook ' + + 'is unmounted before it finishes (class component)', + async () => { + let startTransition; + function Updater() { + const [isPending, _start] = useTransition(); + startTransition = _start; + return ( + + + + ); + } + + let setText; + class Sibling extends React.Component { + state = {text: 'A'}; + render() { + setText = text => this.setState({text}); + return ( + + + + ); + } + } + + function App({showUpdater}) { + return ( + <> + {showUpdater ? : null} + + + ); + } + + const root = ReactNoop.createRoot(); + await act(() => { + root.render(); + }); + assertLog(['Pending: false', 'A']); + expect(root).toMatchRenderedOutput( + <> + Pending: false + A + , + ); + + // Start an async action that has multiple updates with async + // operations in between. + await act(() => { + startTransition(async () => { + Scheduler.log('Async action started'); + startTransition(() => setText('B')); + + await getText('Wait before updating to C'); + + Scheduler.log('Async action ended'); + startTransition(() => setText('C')); + }); + }); + assertLog(['Async action started', 'Pending: true']); + expect(root).toMatchRenderedOutput( + <> + Pending: true + A + , + ); + + // Delete the component that contains the useTransition hook. This + // component no longer blocks the transition from completing. But the + // pending update to Sibling should not be allowed to finish, because it's + // part of the async action. + await act(() => { + root.render(); + }); + assertLog(['A']); + expect(root).toMatchRenderedOutput(A); + + // Finish the async action. Notice the intermediate B state was never + // shown, because it was batched with the update that came later in the + // same action. + await act(() => resolveText('Wait before updating to C')); + assertLog(['Async action ended', 'C']); + expect(root).toMatchRenderedOutput(C); + + // Check that subsequent updates are unaffected. + await act(() => setText('D')); + assertLog(['D']); + expect(root).toMatchRenderedOutput(D); + }, + ); + + // @gate enableAsyncActions + test( + 'updates in an async action are entangled even if useTransition hook ' + + 'is unmounted before it finishes (root update)', + async () => { + let startTransition; + function Updater() { + const [isPending, _start] = useTransition(); + startTransition = _start; + return ( + + + + ); + } + + let setShowUpdater; + function App({text}) { + const [showUpdater, _setShowUpdater] = useState(true); + setShowUpdater = _setShowUpdater; + return ( + <> + {showUpdater ? : null} + + + + + ); + } + + const root = ReactNoop.createRoot(); + await act(() => { + root.render(); + }); + assertLog(['Pending: false', 'A']); + expect(root).toMatchRenderedOutput( + <> + Pending: false + A + , + ); + + // Start an async action that has multiple updates with async + // operations in between. + await act(() => { + startTransition(async () => { + Scheduler.log('Async action started'); + startTransition(() => root.render()); + + await getText('Wait before updating to C'); + + Scheduler.log('Async action ended'); + startTransition(() => root.render()); + }); + }); + assertLog(['Async action started', 'Pending: true']); + expect(root).toMatchRenderedOutput( + <> + Pending: true + A + , + ); + + // Delete the component that contains the useTransition hook. This + // component no longer blocks the transition from completing. But the + // pending update to Sibling should not be allowed to finish, because it's + // part of the async action. + await act(() => setShowUpdater(false)); + assertLog(['A']); + expect(root).toMatchRenderedOutput(A); + + // Finish the async action. Notice the intermediate B state was never + // shown, because it was batched with the update that came later in the + // same action. + await act(() => resolveText('Wait before updating to C')); + assertLog(['Async action ended', 'C']); + expect(root).toMatchRenderedOutput(C); + + // Check that subsequent updates are unaffected. + await act(() => root.render()); + assertLog(['D']); + expect(root).toMatchRenderedOutput(D); + }, + ); });