diff --git a/packages/react-reconciler/src/ReactFiberAsyncAction.js b/packages/react-reconciler/src/ReactFiberAsyncAction.js index 4c1b67e11eed2..ef721ca77bae3 100644 --- a/packages/react-reconciler/src/ReactFiberAsyncAction.js +++ b/packages/react-reconciler/src/ReactFiberAsyncAction.js @@ -7,44 +7,36 @@ * @flow */ -import type {Wakeable} from 'shared/ReactTypes'; +import type { + Thenable, + PendingThenable, + FulfilledThenable, + RejectedThenable, +} from 'shared/ReactTypes'; import type {Lane} from './ReactFiberLane'; -import {requestTransitionLane} from './ReactFiberRootScheduler'; - -interface AsyncActionImpl { - lane: Lane; - listeners: Array<(false) => mixed>; - count: number; - then( - onFulfill: (value: boolean) => mixed, - onReject: (error: mixed) => mixed, - ): void; -} - -interface PendingAsyncAction extends AsyncActionImpl { - status: 'pending'; -} - -interface FulfilledAsyncAction extends AsyncActionImpl { - status: 'fulfilled'; - value: boolean; -} -interface RejectedAsyncAction extends AsyncActionImpl { - status: 'rejected'; - reason: mixed; -} +import {requestTransitionLane} from './ReactFiberRootScheduler'; +import {NoLane} from './ReactFiberLane'; -type AsyncAction = - | PendingAsyncAction - | FulfilledAsyncAction - | RejectedAsyncAction; +// If there are multiple, concurrent async actions, they are entangled. All +// transition updates that occur while the async action is still in progress +// are treated as part of the action. +// +// The ideal behavior would be to treat each async function as an independent +// action. However, without a mechanism like AsyncContext, we can't tell which +// action an update corresponds to. So instead, we entangle them all into one. -let currentAsyncAction: AsyncAction | null = null; +// The listeners to notify once the entangled scope completes. +let currentEntangledListeners: Array<() => mixed> | null = null; +// The number of pending async actions in the entangled scope. +let currentEntangledPendingCount: number = 0; +// The transition lane shared by all updates in the entangled scope. +let currentEntangledLane: Lane = NoLane; -export function requestAsyncActionContext( +export function requestAsyncActionContext( actionReturnValue: mixed, -): AsyncAction | false { + finishedState: S, +): Thenable | S { if ( actionReturnValue !== null && typeof actionReturnValue === 'object' && @@ -53,78 +45,131 @@ export function requestAsyncActionContext( // 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. The fulfilled - // value is `false` to represent that the action is not pending. - const thenable: Wakeable = (actionReturnValue: any); - if (currentAsyncAction === null) { + // function passed to startTransition) has finished running. + + const thenable: Thenable = (actionReturnValue: any); + let entangledListeners; + if (currentEntangledListeners === null) { // There's no outer async action scope. Create a new one. - const asyncAction: AsyncAction = { - lane: requestTransitionLane(), - listeners: [], - count: 0, - status: 'pending', - value: false, - reason: undefined, - then(resolve: boolean => mixed) { - asyncAction.listeners.push(resolve); - }, - }; - attachPingListeners(thenable, asyncAction); - currentAsyncAction = asyncAction; - return asyncAction; + entangledListeners = currentEntangledListeners = []; + currentEntangledPendingCount = 0; + currentEntangledLane = requestTransitionLane(); } else { - // Inherit the outer scope. - const asyncAction: AsyncAction = (currentAsyncAction: any); - attachPingListeners(thenable, asyncAction); - return asyncAction; + entangledListeners = currentEntangledListeners; } + + currentEntangledPendingCount++; + let resultStatus = 'pending'; + let rejectedReason; + thenable.then( + () => { + resultStatus = 'fulfilled'; + pingEngtangledActionScope(); + }, + error => { + resultStatus = 'rejected'; + rejectedReason = error; + pingEngtangledActionScope(); + }, + ); + + // 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); + + // Attach a listener to fill in the result. + entangledListeners.push(() => { + switch (resultStatus) { + case 'fulfilled': { + const fulfilledThenable: FulfilledThenable = (resultThenable: any); + fulfilledThenable.status = 'fulfilled'; + fulfilledThenable.value = finishedState; + 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; } else { // This is not an async action, but it may be part of an outer async action. - if (currentAsyncAction === null) { - // There's no outer async action scope. - return false; + if (currentEntangledListeners === null) { + return finishedState; } else { - // Inherit the outer scope. - return currentAsyncAction; + // 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 = finishedState; + }); + return resultThenable; } } } -export function peekAsyncActionContext(): AsyncAction | null { - return currentAsyncAction; +function pingEngtangledActionScope() { + if ( + currentEntangledListeners !== null && + --currentEntangledPendingCount === 0 + ) { + // All the actions have finished. Close the entangled async action scope + // and notify all the listeners. + const listeners = currentEntangledListeners; + currentEntangledListeners = null; + currentEntangledLane = NoLane; + for (let i = 0; i < listeners.length; i++) { + const listener = listeners[i]; + listener(); + } + } } -function attachPingListeners(thenable: Wakeable, asyncAction: AsyncAction) { - asyncAction.count++; - thenable.then( - () => { - if (--asyncAction.count === 0) { - const fulfilledAsyncAction: FulfilledAsyncAction = (asyncAction: any); - fulfilledAsyncAction.status = 'fulfilled'; - completeAsyncActionScope(asyncAction); - } - }, - (error: mixed) => { - if (--asyncAction.count === 0) { - const rejectedAsyncAction: RejectedAsyncAction = (asyncAction: any); - rejectedAsyncAction.status = 'rejected'; - rejectedAsyncAction.reason = error; - completeAsyncActionScope(asyncAction); - } +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 = { + 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); }, - ); - return asyncAction; + }; + return resultThenable; } -function completeAsyncActionScope(action: AsyncAction) { - if (currentAsyncAction === action) { - currentAsyncAction = null; - } - - const listeners = action.listeners; - action.listeners = []; - for (let i = 0; i < listeners.length; i++) { - const listener = listeners[i]; - listener(false); - } +export function peekEntangledActionLane(): Lane { + return currentEntangledLane; } diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index b4b193233b401..f6576273aa71d 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -2431,8 +2431,10 @@ function updateDeferredValueImpl(hook: Hook, prevValue: T, value: T): T { } } -function startTransition( - setPending: (Thenable | boolean) => void, +function startTransition( + pendingState: S, + finishedState: S, + setPending: (Thenable | S) => void, callback: () => mixed, options?: StartTransitionOptions, ): void { @@ -2443,7 +2445,7 @@ function startTransition( const prevTransition = ReactCurrentBatchConfig.transition; ReactCurrentBatchConfig.transition = null; - setPending(true); + setPending(pendingState); const currentTransition = (ReactCurrentBatchConfig.transition = ({}: BatchConfigTransition)); @@ -2462,15 +2464,18 @@ function startTransition( if (enableAsyncActions) { const returnValue = callback(); - // `isPending` is either `false` or a thenable that resolves to `false`, - // depending on whether the action scope is an async function. In the - // async case, the resulting render will suspend until the async action - // scope has finished. - const isPending = requestAsyncActionContext(returnValue); - setPending(isPending); + // This is either `finishedState` or a thenable that resolves to + // `finishedState`, depending on whether the action scope is an async + // function. In the async case, the resulting render will suspend until + // the async action scope has finished. + const maybeThenable = requestAsyncActionContext( + returnValue, + finishedState, + ); + setPending(maybeThenable); } else { // Async actions are not enabled. - setPending(false); + setPending(finishedState); callback(); } } catch (error) { @@ -2478,7 +2483,7 @@ function startTransition( // This is a trick to get the `useTransition` hook to rethrow the error. // When it unwraps the thenable with the `use` algorithm, the error // will be thrown. - const rejectedThenable: RejectedThenable = { + const rejectedThenable: RejectedThenable = { then() {}, status: 'rejected', reason: error, @@ -2594,6 +2599,8 @@ export function startHostTransition( } startTransition( + true, + false, setPending, // TODO: We can avoid this extra wrapper, somehow. Figure out layering // once more of this function is implemented. @@ -2607,7 +2614,7 @@ function mountTransition(): [ ] { const [, setPending] = mountState((false: Thenable | boolean)); // The `start` method never changes. - const start = startTransition.bind(null, setPending); + const start = startTransition.bind(null, true, false, setPending); const hook = mountWorkInProgressHook(); hook.memoizedState = start; return [false, start]; diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 89b890fc75476..7e06999512a94 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -279,7 +279,7 @@ import { requestTransitionLane, } from './ReactFiberRootScheduler'; import {getMaskedContext, getUnmaskedContext} from './ReactFiberContext'; -import {peekAsyncActionContext} from './ReactFiberAsyncAction'; +import {peekEntangledActionLane} from './ReactFiberAsyncAction'; const PossiblyWeakMap = typeof WeakMap === 'function' ? WeakMap : Map; @@ -632,10 +632,10 @@ export function requestUpdateLane(fiber: Fiber): Lane { transition._updatedFibers.add(fiber); } - const asyncAction = peekAsyncActionContext(); - return asyncAction !== null + const actionScopeLane = peekEntangledActionLane(); + return actionScopeLane !== NoLane ? // We're inside an async action scope. Reuse the same lane. - asyncAction.lane + actionScopeLane : // We may or may not be inside an async action scope. If we are, this // is the first update in that scope. Either way, we need to get a // fresh transition lane. diff --git a/packages/react-reconciler/src/__tests__/ReactAsyncActions-test.js b/packages/react-reconciler/src/__tests__/ReactAsyncActions-test.js index 3556c096918bd..654ebf62e05f3 100644 --- a/packages/react-reconciler/src/__tests__/ReactAsyncActions-test.js +++ b/packages/react-reconciler/src/__tests__/ReactAsyncActions-test.js @@ -527,4 +527,121 @@ describe('ReactAsyncActions', () => { assertLog(['Pending: true', 'Pending: false']); expect(root).toMatchRenderedOutput('Pending: false'); }); + + // @gate enableAsyncActions + test('if there are multiple entangled actions, and one of them errors, it only affects that action', async () => { + class ErrorBoundary extends React.Component { + state = {error: null}; + static getDerivedStateFromError(error) { + return {error}; + } + render() { + if (this.state.error) { + return ; + } + return this.props.children; + } + } + + let startTransitionA; + function ActionA() { + const [isPendingA, start] = useTransition(); + startTransitionA = start; + return ; + } + + let startTransitionB; + function ActionB() { + const [isPending, start] = useTransition(); + startTransitionB = start; + return ; + } + + let startTransitionC; + function ActionC() { + const [isPending, start] = useTransition(); + startTransitionC = start; + return ; + } + + const root = ReactNoop.createRoot(); + await act(() => { + root.render( + <> +
+ + + +
+
+ + + +
+
+ + + +
+ , + ); + }); + assertLog(['Pending A: false', 'Pending B: false', 'Pending C: false']); + expect(root).toMatchRenderedOutput( + <> +
Pending A: false
+
Pending B: false
+
Pending C: false
+ , + ); + + // Start a bunch of entangled transitions. A and C throw errors, but B + // doesn't. A and should surface their respective errors, but B should + // finish successfully. + await act(() => { + startTransitionC(async () => { + startTransitionB(async () => { + startTransitionA(async () => { + await getText('Wait for A'); + throw new Error('Oops A!'); + }); + await getText('Wait for B'); + }); + await getText('Wait for C'); + throw new Error('Oops C!'); + }); + }); + assertLog(['Pending A: true', 'Pending B: true', 'Pending C: true']); + + // Finish action A. We can't commit the result yet because it's entangled + // with B and C. + await act(() => resolveText('Wait for A')); + assertLog([]); + + // Finish action B. Same as above. + await act(() => resolveText('Wait for B')); + assertLog([]); + + // Now finish action C. This is the last action in the entangled set, so + // rendering can proceed. + await act(() => resolveText('Wait for C')); + assertLog([ + // A and C result in (separate) errors, but B does not. + 'Oops A!', + 'Pending B: false', + 'Oops C!', + + // Because there was an error, React will try rendering one more time. + 'Oops A!', + 'Pending B: false', + 'Oops C!', + ]); + expect(root).toMatchRenderedOutput( + <> +
Oops A!
+
Pending B: false
+
Oops C!
+ , + ); + }); }); diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index 50579ff47a039..9f378cea27489 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -462,5 +462,6 @@ "474": "Suspense Exception: This is not a real error, and should not leak into userspace. If you're seeing this, it's likely a bug in React.", "475": "Internal React Error: suspendedState null when it was expected to exists. Please report this as a React bug.", "476": "Expected the form instance to be a HostComponent. This is a bug in React.", - "477": "React Internal Error: processHintChunk is not implemented for Native-Relay. The fact that this method was called means there is a bug in React." + "477": "React Internal Error: processHintChunk is not implemented for Native-Relay. The fact that this method was called means there is a bug in React.", + "478": "Thenable should have already resolved. This is a bug in React." }