diff --git a/packages/react-reconciler/src/ReactFiberHooks.new.js b/packages/react-reconciler/src/ReactFiberHooks.new.js index 2a358efc0a5a8..0b6c540ecc741 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.new.js +++ b/packages/react-reconciler/src/ReactFiberHooks.new.js @@ -102,6 +102,7 @@ import { requestEventTime, markSkippedUpdateLanes, isInvalidExecutionContextForEventFunction, + getSuspendedThenableState, } from './ReactFiberWorkLoop.new'; import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber'; @@ -134,8 +135,8 @@ import { import {getTreeId} from './ReactFiberTreeContext.new'; import {now} from './Scheduler'; import { + prepareThenableState, trackUsedThenable, - getPreviouslyUsedThenableAtIndex, } from './ReactFiberThenable.new'; const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals; @@ -465,6 +466,9 @@ export function renderWithHooks( : HooksDispatcherOnUpdate; } + // If this is a replay, restore the thenable state from the previous attempt. + const prevThenableState = getSuspendedThenableState(); + prepareThenableState(prevThenableState); let children = Component(props, secondArg); // Check if there was a render phase update @@ -506,6 +510,7 @@ export function renderWithHooks( ? HooksDispatcherOnRerenderInDEV : HooksDispatcherOnRerender; + prepareThenableState(prevThenableState); children = Component(props, secondArg); } while (didScheduleRenderPhaseUpdateDuringThisPass); } @@ -770,8 +775,6 @@ if (enableUseMemoCacheHook) { }; } -function noop(): void {} - function use(usable: Usable): T { if (usable !== null && typeof usable === 'object') { // $FlowFixMe[method-unbinding] @@ -782,59 +785,7 @@ function use(usable: Usable): T { // Track the position of the thenable within this fiber. const index = thenableIndexCounter; thenableIndexCounter += 1; - - // TODO: Unify this switch statement with the one in trackUsedThenable. - switch (thenable.status) { - case 'fulfilled': { - const fulfilledValue: T = thenable.value; - return fulfilledValue; - } - case 'rejected': { - const rejectedError = thenable.reason; - throw rejectedError; - } - default: { - const prevThenableAtIndex: Thenable | null = getPreviouslyUsedThenableAtIndex( - index, - ); - if (prevThenableAtIndex !== null) { - if (thenable !== prevThenableAtIndex) { - // Avoid an unhandled rejection errors for the Promises that we'll - // intentionally ignore. - thenable.then(noop, noop); - } - switch (prevThenableAtIndex.status) { - case 'fulfilled': { - const fulfilledValue: T = prevThenableAtIndex.value; - return fulfilledValue; - } - case 'rejected': { - const rejectedError: mixed = prevThenableAtIndex.reason; - throw rejectedError; - } - default: { - // The thenable still hasn't resolved. Suspend with the same - // thenable as last time to avoid redundant listeners. - throw prevThenableAtIndex; - } - } - } else { - // This is the first time something has been used at this index. - // Stash the thenable at the current index so we can reuse it during - // the next attempt. - trackUsedThenable(thenable, index); - - // Suspend. - // TODO: Throwing here is an implementation detail that allows us to - // unwind the call stack. But we shouldn't allow it to leak into - // userspace. Throw an opaque placeholder value instead of the - // actual thenable. If it doesn't get captured by the work loop, log - // a warning, because that means something in userspace must have - // caught it. - throw thenable; - } - } - } + return trackUsedThenable(thenable, index); } else if ( usable.$$typeof === REACT_CONTEXT_TYPE || usable.$$typeof === REACT_SERVER_CONTEXT_TYPE diff --git a/packages/react-reconciler/src/ReactFiberHooks.old.js b/packages/react-reconciler/src/ReactFiberHooks.old.js index 3370f51cbd412..cb5eeb4adb459 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.old.js +++ b/packages/react-reconciler/src/ReactFiberHooks.old.js @@ -102,6 +102,7 @@ import { requestEventTime, markSkippedUpdateLanes, isInvalidExecutionContextForEventFunction, + getSuspendedThenableState, } from './ReactFiberWorkLoop.old'; import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber'; @@ -134,8 +135,8 @@ import { import {getTreeId} from './ReactFiberTreeContext.old'; import {now} from './Scheduler'; import { + prepareThenableState, trackUsedThenable, - getPreviouslyUsedThenableAtIndex, } from './ReactFiberThenable.old'; const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals; @@ -465,6 +466,9 @@ export function renderWithHooks( : HooksDispatcherOnUpdate; } + // If this is a replay, restore the thenable state from the previous attempt. + const prevThenableState = getSuspendedThenableState(); + prepareThenableState(prevThenableState); let children = Component(props, secondArg); // Check if there was a render phase update @@ -506,6 +510,7 @@ export function renderWithHooks( ? HooksDispatcherOnRerenderInDEV : HooksDispatcherOnRerender; + prepareThenableState(prevThenableState); children = Component(props, secondArg); } while (didScheduleRenderPhaseUpdateDuringThisPass); } @@ -770,8 +775,6 @@ if (enableUseMemoCacheHook) { }; } -function noop(): void {} - function use(usable: Usable): T { if (usable !== null && typeof usable === 'object') { // $FlowFixMe[method-unbinding] @@ -782,59 +785,7 @@ function use(usable: Usable): T { // Track the position of the thenable within this fiber. const index = thenableIndexCounter; thenableIndexCounter += 1; - - // TODO: Unify this switch statement with the one in trackUsedThenable. - switch (thenable.status) { - case 'fulfilled': { - const fulfilledValue: T = thenable.value; - return fulfilledValue; - } - case 'rejected': { - const rejectedError = thenable.reason; - throw rejectedError; - } - default: { - const prevThenableAtIndex: Thenable | null = getPreviouslyUsedThenableAtIndex( - index, - ); - if (prevThenableAtIndex !== null) { - if (thenable !== prevThenableAtIndex) { - // Avoid an unhandled rejection errors for the Promises that we'll - // intentionally ignore. - thenable.then(noop, noop); - } - switch (prevThenableAtIndex.status) { - case 'fulfilled': { - const fulfilledValue: T = prevThenableAtIndex.value; - return fulfilledValue; - } - case 'rejected': { - const rejectedError: mixed = prevThenableAtIndex.reason; - throw rejectedError; - } - default: { - // The thenable still hasn't resolved. Suspend with the same - // thenable as last time to avoid redundant listeners. - throw prevThenableAtIndex; - } - } - } else { - // This is the first time something has been used at this index. - // Stash the thenable at the current index so we can reuse it during - // the next attempt. - trackUsedThenable(thenable, index); - - // Suspend. - // TODO: Throwing here is an implementation detail that allows us to - // unwind the call stack. But we shouldn't allow it to leak into - // userspace. Throw an opaque placeholder value instead of the - // actual thenable. If it doesn't get captured by the work loop, log - // a warning, because that means something in userspace must have - // caught it. - throw thenable; - } - } - } + return trackUsedThenable(thenable, index); } else if ( usable.$$typeof === REACT_CONTEXT_TYPE || usable.$$typeof === REACT_SERVER_CONTEXT_TYPE diff --git a/packages/react-reconciler/src/ReactFiberThenable.new.js b/packages/react-reconciler/src/ReactFiberThenable.new.js index 0a8f9d8deaad6..c2de159f89ca9 100644 --- a/packages/react-reconciler/src/ReactFiberThenable.new.js +++ b/packages/react-reconciler/src/ReactFiberThenable.new.js @@ -17,33 +17,75 @@ import type { import ReactSharedInternals from 'shared/ReactSharedInternals'; const {ReactCurrentActQueue} = ReactSharedInternals; -let suspendedThenable: Thenable | null = null; -let usedThenables: Array | void> | null = null; +export opaque type ThenableState = Array>; -export function isTrackingSuspendedThenable(): boolean { - return suspendedThenable !== null; +let thenableState: ThenableState | null = null; + +export function createThenableState(): ThenableState { + // The ThenableState is created the first time a component suspends. If it + // suspends again, we'll reuse the same state. + return []; +} + +export function prepareThenableState(prevThenableState: ThenableState | null) { + // This function is called before every function that might suspend + // with `use`. Right now, that's only Hooks, but in the future we'll use the + // same mechanism for unwrapping promises during reconciliation. + thenableState = prevThenableState; } -export function suspendedThenableDidResolve(): boolean { - if (suspendedThenable !== null) { - const status = suspendedThenable.status; +export function getThenableStateAfterSuspending(): ThenableState | null { + // Called by the work loop so it can stash the thenable state. It will use + // the state to replay the component when the promise resolves. + if ( + thenableState !== null && + // If we only `use`-ed resolved promises, then there is no suspended state + // TODO: The only reason we do this is to distinguish between throwing a + // promise (old Suspense pattern) versus `use`-ing one. A better solution is + // for `use` to throw a special, opaque value instead of a promise. + !isThenableStateResolved(thenableState) + ) { + const state = thenableState; + thenableState = null; + return state; + } + return null; +} + +export function isThenableStateResolved(thenables: ThenableState): boolean { + const lastThenable = thenables[thenables.length - 1]; + if (lastThenable !== undefined) { + const status = lastThenable.status; return status === 'fulfilled' || status === 'rejected'; } - return false; + return true; } -export function trackUsedThenable(thenable: Thenable, index: number) { +function noop(): void {} + +export function trackUsedThenable(thenable: Thenable, index: number): T { if (__DEV__ && ReactCurrentActQueue.current !== null) { ReactCurrentActQueue.didUsePromise = true; } - if (usedThenables === null) { - usedThenables = [thenable]; + if (thenableState === null) { + thenableState = [thenable]; } else { - usedThenables[index] = thenable; - } + const previous = thenableState[index]; + if (previous === undefined) { + thenableState.push(thenable); + } else { + if (previous !== thenable) { + // Reuse the previous thenable, and drop the new one. We can assume + // they represent the same value, because components are idempotent. - suspendedThenable = thenable; + // Avoid an unhandled rejection errors for the Promises that we'll + // intentionally ignore. + thenable.then(noop, noop); + thenable = previous; + } + } + } // We use an expando to track the status and result of a thenable so that we // can synchronously unwrap the value. Think of this as an extension of the @@ -52,61 +94,48 @@ export function trackUsedThenable(thenable: Thenable, index: number) { // If the thenable doesn't have a status, set it to "pending" and attach // a listener that will update its status and result when it resolves. switch (thenable.status) { - case 'fulfilled': - case 'rejected': - // A thenable that already resolved shouldn't have been thrown, so this is - // unexpected. Suggests a mistake in a userspace data library. Don't track - // this thenable, because if we keep trying it will likely infinite loop - // without ever resolving. - // TODO: Log a warning? - suspendedThenable = null; - break; + case 'fulfilled': { + const fulfilledValue: T = thenable.value; + return fulfilledValue; + } + case 'rejected': { + const rejectedError = thenable.reason; + throw rejectedError; + } default: { if (typeof thenable.status === 'string') { // Only instrument the thenable if the status if not defined. If // it's defined, but an unknown value, assume it's been instrumented by // some custom userspace implementation. We treat it as "pending". - break; + } else { + const pendingThenable: PendingThenable = (thenable: any); + pendingThenable.status = 'pending'; + pendingThenable.then( + fulfilledValue => { + if (thenable.status === 'pending') { + const fulfilledThenable: FulfilledThenable = (thenable: any); + fulfilledThenable.status = 'fulfilled'; + fulfilledThenable.value = fulfilledValue; + } + }, + (error: mixed) => { + if (thenable.status === 'pending') { + const rejectedThenable: RejectedThenable = (thenable: any); + rejectedThenable.status = 'rejected'; + rejectedThenable.reason = error; + } + }, + ); } - const pendingThenable: PendingThenable = (thenable: any); - pendingThenable.status = 'pending'; - pendingThenable.then( - fulfilledValue => { - if (thenable.status === 'pending') { - const fulfilledThenable: FulfilledThenable = (thenable: any); - fulfilledThenable.status = 'fulfilled'; - fulfilledThenable.value = fulfilledValue; - } - }, - (error: mixed) => { - if (thenable.status === 'pending') { - const rejectedThenable: RejectedThenable = (thenable: any); - rejectedThenable.status = 'rejected'; - rejectedThenable.reason = error; - } - }, - ); - break; - } - } -} -export function resetWakeableStateAfterEachAttempt() { - suspendedThenable = null; -} - -export function resetThenableStateOnCompletion() { - usedThenables = null; -} - -export function getPreviouslyUsedThenableAtIndex( - index: number, -): Thenable | null { - if (usedThenables !== null) { - const thenable = usedThenables[index]; - if (thenable !== undefined) { - return thenable; + // Suspend. + // TODO: Throwing here is an implementation detail that allows us to + // unwind the call stack. But we shouldn't allow it to leak into + // userspace. Throw an opaque placeholder value instead of the + // actual thenable. If it doesn't get captured by the work loop, log + // a warning, because that means something in userspace must have + // caught it. + throw thenable; } } - return null; } diff --git a/packages/react-reconciler/src/ReactFiberThenable.old.js b/packages/react-reconciler/src/ReactFiberThenable.old.js index 0a8f9d8deaad6..c2de159f89ca9 100644 --- a/packages/react-reconciler/src/ReactFiberThenable.old.js +++ b/packages/react-reconciler/src/ReactFiberThenable.old.js @@ -17,33 +17,75 @@ import type { import ReactSharedInternals from 'shared/ReactSharedInternals'; const {ReactCurrentActQueue} = ReactSharedInternals; -let suspendedThenable: Thenable | null = null; -let usedThenables: Array | void> | null = null; +export opaque type ThenableState = Array>; -export function isTrackingSuspendedThenable(): boolean { - return suspendedThenable !== null; +let thenableState: ThenableState | null = null; + +export function createThenableState(): ThenableState { + // The ThenableState is created the first time a component suspends. If it + // suspends again, we'll reuse the same state. + return []; +} + +export function prepareThenableState(prevThenableState: ThenableState | null) { + // This function is called before every function that might suspend + // with `use`. Right now, that's only Hooks, but in the future we'll use the + // same mechanism for unwrapping promises during reconciliation. + thenableState = prevThenableState; } -export function suspendedThenableDidResolve(): boolean { - if (suspendedThenable !== null) { - const status = suspendedThenable.status; +export function getThenableStateAfterSuspending(): ThenableState | null { + // Called by the work loop so it can stash the thenable state. It will use + // the state to replay the component when the promise resolves. + if ( + thenableState !== null && + // If we only `use`-ed resolved promises, then there is no suspended state + // TODO: The only reason we do this is to distinguish between throwing a + // promise (old Suspense pattern) versus `use`-ing one. A better solution is + // for `use` to throw a special, opaque value instead of a promise. + !isThenableStateResolved(thenableState) + ) { + const state = thenableState; + thenableState = null; + return state; + } + return null; +} + +export function isThenableStateResolved(thenables: ThenableState): boolean { + const lastThenable = thenables[thenables.length - 1]; + if (lastThenable !== undefined) { + const status = lastThenable.status; return status === 'fulfilled' || status === 'rejected'; } - return false; + return true; } -export function trackUsedThenable(thenable: Thenable, index: number) { +function noop(): void {} + +export function trackUsedThenable(thenable: Thenable, index: number): T { if (__DEV__ && ReactCurrentActQueue.current !== null) { ReactCurrentActQueue.didUsePromise = true; } - if (usedThenables === null) { - usedThenables = [thenable]; + if (thenableState === null) { + thenableState = [thenable]; } else { - usedThenables[index] = thenable; - } + const previous = thenableState[index]; + if (previous === undefined) { + thenableState.push(thenable); + } else { + if (previous !== thenable) { + // Reuse the previous thenable, and drop the new one. We can assume + // they represent the same value, because components are idempotent. - suspendedThenable = thenable; + // Avoid an unhandled rejection errors for the Promises that we'll + // intentionally ignore. + thenable.then(noop, noop); + thenable = previous; + } + } + } // We use an expando to track the status and result of a thenable so that we // can synchronously unwrap the value. Think of this as an extension of the @@ -52,61 +94,48 @@ export function trackUsedThenable(thenable: Thenable, index: number) { // If the thenable doesn't have a status, set it to "pending" and attach // a listener that will update its status and result when it resolves. switch (thenable.status) { - case 'fulfilled': - case 'rejected': - // A thenable that already resolved shouldn't have been thrown, so this is - // unexpected. Suggests a mistake in a userspace data library. Don't track - // this thenable, because if we keep trying it will likely infinite loop - // without ever resolving. - // TODO: Log a warning? - suspendedThenable = null; - break; + case 'fulfilled': { + const fulfilledValue: T = thenable.value; + return fulfilledValue; + } + case 'rejected': { + const rejectedError = thenable.reason; + throw rejectedError; + } default: { if (typeof thenable.status === 'string') { // Only instrument the thenable if the status if not defined. If // it's defined, but an unknown value, assume it's been instrumented by // some custom userspace implementation. We treat it as "pending". - break; + } else { + const pendingThenable: PendingThenable = (thenable: any); + pendingThenable.status = 'pending'; + pendingThenable.then( + fulfilledValue => { + if (thenable.status === 'pending') { + const fulfilledThenable: FulfilledThenable = (thenable: any); + fulfilledThenable.status = 'fulfilled'; + fulfilledThenable.value = fulfilledValue; + } + }, + (error: mixed) => { + if (thenable.status === 'pending') { + const rejectedThenable: RejectedThenable = (thenable: any); + rejectedThenable.status = 'rejected'; + rejectedThenable.reason = error; + } + }, + ); } - const pendingThenable: PendingThenable = (thenable: any); - pendingThenable.status = 'pending'; - pendingThenable.then( - fulfilledValue => { - if (thenable.status === 'pending') { - const fulfilledThenable: FulfilledThenable = (thenable: any); - fulfilledThenable.status = 'fulfilled'; - fulfilledThenable.value = fulfilledValue; - } - }, - (error: mixed) => { - if (thenable.status === 'pending') { - const rejectedThenable: RejectedThenable = (thenable: any); - rejectedThenable.status = 'rejected'; - rejectedThenable.reason = error; - } - }, - ); - break; - } - } -} -export function resetWakeableStateAfterEachAttempt() { - suspendedThenable = null; -} - -export function resetThenableStateOnCompletion() { - usedThenables = null; -} - -export function getPreviouslyUsedThenableAtIndex( - index: number, -): Thenable | null { - if (usedThenables !== null) { - const thenable = usedThenables[index]; - if (thenable !== undefined) { - return thenable; + // Suspend. + // TODO: Throwing here is an implementation detail that allows us to + // unwind the call stack. But we shouldn't allow it to leak into + // userspace. Throw an opaque placeholder value instead of the + // actual thenable. If it doesn't get captured by the work loop, log + // a warning, because that means something in userspace must have + // caught it. + throw thenable; } } - return null; } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 32229d31851cd..d94060cdac43c 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -22,6 +22,7 @@ import type { TransitionAbort, } from './ReactFiberTracingMarkerComponent.new'; import type {OffscreenInstance} from './ReactFiberOffscreenComponent'; +import type {ThenableState} from './ReactFiberThenable.new'; import { warnAboutDeprecatedLifecycles, @@ -265,10 +266,8 @@ import { } from './ReactFiberAct.new'; import {processTransitionCallbacks} from './ReactFiberTracingMarkerComponent.new'; import { - resetWakeableStateAfterEachAttempt, - resetThenableStateOnCompletion, - suspendedThenableDidResolve, - isTrackingSuspendedThenable, + getThenableStateAfterSuspending, + isThenableStateResolved, } from './ReactFiberThenable.new'; import {schedulePostPaintCallback} from './ReactPostPaintCallback'; @@ -315,6 +314,7 @@ let workInProgressRootRenderLanes: Lanes = NoLanes; // immediately instead of unwinding the stack. let workInProgressIsSuspended: boolean = false; let workInProgressThrownValue: mixed = null; +let workInProgressSuspendedThenableState: ThenableState | null = null; // Whether a ping listener was attached during this render. This is slightly // different that whether something suspended, because we don't add multiple @@ -1686,8 +1686,6 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber { ); interruptedWork = interruptedWork.return; } - resetWakeableStateAfterEachAttempt(); - resetThenableStateOnCompletion(); } workInProgressRoot = root; const rootWorkInProgress = createWorkInProgress(root.current, null); @@ -1695,6 +1693,7 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber { workInProgressRootRenderLanes = renderLanes = lanes; workInProgressIsSuspended = false; workInProgressThrownValue = null; + workInProgressSuspendedThenableState = null; workInProgressRootDidAttachPingListener = false; workInProgressRootExitStatus = RootInProgress; workInProgressRootFatalError = null; @@ -1729,6 +1728,7 @@ function handleThrow(root, thrownValue): void { // as suspending the execution of the work loop. workInProgressIsSuspended = true; workInProgressThrownValue = thrownValue; + workInProgressSuspendedThenableState = getThenableStateAfterSuspending(); const erroredWork = workInProgress; if (erroredWork === null) { @@ -2014,7 +2014,7 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { break; } catch (thrownValue) { handleThrow(root, thrownValue); - if (isTrackingSuspendedThenable()) { + if (workInProgressSuspendedThenableState !== null) { // If this fiber just suspended, it's possible the data is already // cached. Yield to the main thread to give it a chance to ping. If // it does, we can retry immediately without unwinding the stack. @@ -2117,13 +2117,14 @@ function resumeSuspendedUnitOfWork( // instead of unwinding the stack. It's a separate function to keep the // additional logic out of the work loop's hot path. - const wasPinged = suspendedThenableDidResolve(); - resetWakeableStateAfterEachAttempt(); + const wasPinged = + workInProgressSuspendedThenableState !== null && + isThenableStateResolved(workInProgressSuspendedThenableState); if (!wasPinged) { // The thenable wasn't pinged. Return to the normal work loop. This will // unwind the stack, and potentially result in showing a fallback. - resetThenableStateOnCompletion(); + workInProgressSuspendedThenableState = null; const returnFiber = unitOfWork.return; if (returnFiber === null || workInProgressRoot === null) { @@ -2188,7 +2189,7 @@ function resumeSuspendedUnitOfWork( // The begin phase finished successfully without suspending. Reset the state // used to track the fiber while it was suspended. Then return to the normal // work loop. - resetThenableStateOnCompletion(); + workInProgressSuspendedThenableState = null; resetCurrentDebugFiberInDEV(); unitOfWork.memoizedProps = unitOfWork.pendingProps; @@ -2202,6 +2203,10 @@ function resumeSuspendedUnitOfWork( ReactCurrentOwner.current = null; } +export function getSuspendedThenableState(): ThenableState | null { + return workInProgressSuspendedThenableState; +} + 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/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index 4c86b9707b452..ae24f0f5af572 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -22,6 +22,7 @@ import type { TransitionAbort, } from './ReactFiberTracingMarkerComponent.old'; import type {OffscreenInstance} from './ReactFiberOffscreenComponent'; +import type {ThenableState} from './ReactFiberThenable.old'; import { warnAboutDeprecatedLifecycles, @@ -265,10 +266,8 @@ import { } from './ReactFiberAct.old'; import {processTransitionCallbacks} from './ReactFiberTracingMarkerComponent.old'; import { - resetWakeableStateAfterEachAttempt, - resetThenableStateOnCompletion, - suspendedThenableDidResolve, - isTrackingSuspendedThenable, + getThenableStateAfterSuspending, + isThenableStateResolved, } from './ReactFiberThenable.old'; import {schedulePostPaintCallback} from './ReactPostPaintCallback'; @@ -315,6 +314,7 @@ let workInProgressRootRenderLanes: Lanes = NoLanes; // immediately instead of unwinding the stack. let workInProgressIsSuspended: boolean = false; let workInProgressThrownValue: mixed = null; +let workInProgressSuspendedThenableState: ThenableState | null = null; // Whether a ping listener was attached during this render. This is slightly // different that whether something suspended, because we don't add multiple @@ -1686,8 +1686,6 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber { ); interruptedWork = interruptedWork.return; } - resetWakeableStateAfterEachAttempt(); - resetThenableStateOnCompletion(); } workInProgressRoot = root; const rootWorkInProgress = createWorkInProgress(root.current, null); @@ -1695,6 +1693,7 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber { workInProgressRootRenderLanes = renderLanes = lanes; workInProgressIsSuspended = false; workInProgressThrownValue = null; + workInProgressSuspendedThenableState = null; workInProgressRootDidAttachPingListener = false; workInProgressRootExitStatus = RootInProgress; workInProgressRootFatalError = null; @@ -1729,6 +1728,7 @@ function handleThrow(root, thrownValue): void { // as suspending the execution of the work loop. workInProgressIsSuspended = true; workInProgressThrownValue = thrownValue; + workInProgressSuspendedThenableState = getThenableStateAfterSuspending(); const erroredWork = workInProgress; if (erroredWork === null) { @@ -2014,7 +2014,7 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { break; } catch (thrownValue) { handleThrow(root, thrownValue); - if (isTrackingSuspendedThenable()) { + if (workInProgressSuspendedThenableState !== null) { // If this fiber just suspended, it's possible the data is already // cached. Yield to the main thread to give it a chance to ping. If // it does, we can retry immediately without unwinding the stack. @@ -2117,13 +2117,14 @@ function resumeSuspendedUnitOfWork( // instead of unwinding the stack. It's a separate function to keep the // additional logic out of the work loop's hot path. - const wasPinged = suspendedThenableDidResolve(); - resetWakeableStateAfterEachAttempt(); + const wasPinged = + workInProgressSuspendedThenableState !== null && + isThenableStateResolved(workInProgressSuspendedThenableState); if (!wasPinged) { // The thenable wasn't pinged. Return to the normal work loop. This will // unwind the stack, and potentially result in showing a fallback. - resetThenableStateOnCompletion(); + workInProgressSuspendedThenableState = null; const returnFiber = unitOfWork.return; if (returnFiber === null || workInProgressRoot === null) { @@ -2188,7 +2189,7 @@ function resumeSuspendedUnitOfWork( // The begin phase finished successfully without suspending. Reset the state // used to track the fiber while it was suspended. Then return to the normal // work loop. - resetThenableStateOnCompletion(); + workInProgressSuspendedThenableState = null; resetCurrentDebugFiberInDEV(); unitOfWork.memoizedProps = unitOfWork.pendingProps; @@ -2202,6 +2203,10 @@ function resumeSuspendedUnitOfWork( ReactCurrentOwner.current = null; } +export function getSuspendedThenableState(): ThenableState | null { + return workInProgressSuspendedThenableState; +} + 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__/ReactThenable-test.js b/packages/react-reconciler/src/__tests__/ReactThenable-test.js index 5062ff71658ca..2ac5c8a21995a 100644 --- a/packages/react-reconciler/src/__tests__/ReactThenable-test.js +++ b/packages/react-reconciler/src/__tests__/ReactThenable-test.js @@ -8,7 +8,7 @@ let use; let Suspense; let startTransition; -describe('ReactWakeable', () => { +describe('ReactThenable', () => { beforeEach(() => { jest.resetModules(); @@ -243,6 +243,76 @@ describe('ReactWakeable', () => { expect(Scheduler).toHaveYielded(['Oops!', 'Oops!']); }); + // @gate enableUseHook + test('use(promise) in multiple components', async () => { + // This tests that the state for tracking promises is reset per component. + const promiseA = Promise.resolve('A'); + const promiseB = Promise.resolve('B'); + const promiseC = Promise.resolve('C'); + const promiseD = Promise.resolve('D'); + + function Child({prefix}) { + return ; + } + + function Parent() { + return ; + } + + function App() { + return ( + }> + + + ); + } + + const root = ReactNoop.createRoot(); + await act(async () => { + startTransition(() => { + root.render(); + }); + }); + expect(Scheduler).toHaveYielded(['ABCD']); + expect(root).toMatchRenderedOutput('ABCD'); + }); + + // @gate enableUseHook + test('use(promise) in multiple sibling components', async () => { + // This tests that the state for tracking promises is reset per component. + + const promiseA = {then: () => {}, status: 'pending', value: null}; + const promiseB = {then: () => {}, status: 'pending', value: null}; + const promiseC = {then: () => {}, status: 'fulfilled', value: 'C'}; + const promiseD = {then: () => {}, status: 'fulfilled', value: 'D'}; + + function Sibling1({prefix}) { + return ; + } + + function Sibling2() { + return ; + } + + function App() { + return ( + }> + + + + ); + } + + const root = ReactNoop.createRoot(); + await act(async () => { + startTransition(() => { + root.render(); + }); + }); + expect(Scheduler).toHaveYielded(['CD', 'Loading...']); + expect(root).toMatchRenderedOutput('Loading...'); + }); + // @gate enableUseHook test('erroring in the same component as an uncached promise does not result in an infinite loop', async () => { class ErrorBoundary extends React.Component { diff --git a/packages/react-server/src/ReactFizzHooks.js b/packages/react-server/src/ReactFizzHooks.js index 5545d8ef8d46e..b8c1e4c92773b 100644 --- a/packages/react-server/src/ReactFizzHooks.js +++ b/packages/react-server/src/ReactFizzHooks.js @@ -25,11 +25,7 @@ import type {ThenableState} from './ReactFizzThenable'; import {readContext as readContextImpl} from './ReactFizzNewContext'; import {getTreeId} from './ReactFizzTreeContext'; -import { - getPreviouslyUsedThenableAtIndex, - createThenableState, - trackUsedThenable, -} from './ReactFizzThenable'; +import {createThenableState, trackUsedThenable} from './ReactFizzThenable'; import {makeId} from './ReactServerFormatConfig'; @@ -593,62 +589,10 @@ function use(usable: Usable): T { const index = thenableIndexCounter; thenableIndexCounter += 1; - // TODO: Unify this switch statement with the one in trackUsedThenable. - switch (thenable.status) { - case 'fulfilled': { - const fulfilledValue: T = thenable.value; - return fulfilledValue; - } - case 'rejected': { - const rejectedError = thenable.reason; - throw rejectedError; - } - default: { - const prevThenableAtIndex: Thenable | null = getPreviouslyUsedThenableAtIndex( - thenableState, - index, - ); - if (prevThenableAtIndex !== null) { - if (thenable !== prevThenableAtIndex) { - // Avoid an unhandled rejection errors for the Promises that we'll - // intentionally ignore. - thenable.then(noop, noop); - } - switch (prevThenableAtIndex.status) { - case 'fulfilled': { - const fulfilledValue: T = prevThenableAtIndex.value; - return fulfilledValue; - } - case 'rejected': { - const rejectedError: mixed = prevThenableAtIndex.reason; - throw rejectedError; - } - default: { - // The thenable still hasn't resolved. Suspend with the same - // thenable as last time to avoid redundant listeners. - throw prevThenableAtIndex; - } - } - } else { - // This is the first time something has been used at this index. - // Stash the thenable at the current index so we can reuse it during - // the next attempt. - if (thenableState === null) { - thenableState = createThenableState(); - } - trackUsedThenable(thenableState, thenable, index); - - // Suspend. - // TODO: Throwing here is an implementation detail that allows us to - // unwind the call stack. But we shouldn't allow it to leak into - // userspace. Throw an opaque placeholder value instead of the - // actual thenable. If it doesn't get captured by the work loop, log - // a warning, because that means something in userspace must have - // caught it. - throw thenable; - } - } + if (thenableState === null) { + thenableState = createThenableState(); } + return trackUsedThenable(thenableState, thenable, index); } else if ( usable.$$typeof === REACT_CONTEXT_TYPE || usable.$$typeof === REACT_SERVER_CONTEXT_TYPE diff --git a/packages/react-server/src/ReactFizzThenable.js b/packages/react-server/src/ReactFizzThenable.js index 52cedc579e3f0..267ee6036041d 100644 --- a/packages/react-server/src/ReactFizzThenable.js +++ b/packages/react-server/src/ReactFizzThenable.js @@ -20,8 +20,7 @@ import type { RejectedThenable, } from 'shared/ReactTypes'; -// TODO: Sparse arrays are bad for performance. -export opaque type ThenableState = Array | void>; +export opaque type ThenableState = Array>; export function createThenableState(): ThenableState { // The ThenableState is created the first time a component suspends. If it @@ -29,12 +28,27 @@ export function createThenableState(): ThenableState { return []; } +function noop(): void {} + export function trackUsedThenable( thenableState: ThenableState, thenable: Thenable, index: number, -) { - thenableState[index] = thenable; +): T { + const previous = thenableState[index]; + if (previous === undefined) { + thenableState.push(thenable); + } else { + if (previous !== thenable) { + // Reuse the previous thenable, and drop the new one. We can assume + // they represent the same value, because components are idempotent. + + // Avoid an unhandled rejection errors for the Promises that we'll + // intentionally ignore. + thenable.then(noop, noop); + thenable = previous; + } + } // We use an expando to track the status and result of a thenable so that we // can synchronously unwrap the value. Think of this as an extension of the @@ -43,53 +57,48 @@ export function trackUsedThenable( // If the thenable doesn't have a status, set it to "pending" and attach // a listener that will update its status and result when it resolves. switch (thenable.status) { - case 'fulfilled': - case 'rejected': - // A thenable that already resolved shouldn't have been thrown, so this is - // unexpected. Suggests a mistake in a userspace data library. Don't track - // this thenable, because if we keep trying it will likely infinite loop - // without ever resolving. - // TODO: Log a warning? - break; + case 'fulfilled': { + const fulfilledValue: T = thenable.value; + return fulfilledValue; + } + case 'rejected': { + const rejectedError = thenable.reason; + throw rejectedError; + } default: { if (typeof thenable.status === 'string') { // Only instrument the thenable if the status if not defined. If // it's defined, but an unknown value, assume it's been instrumented by // some custom userspace implementation. We treat it as "pending". - break; + } else { + const pendingThenable: PendingThenable = (thenable: any); + pendingThenable.status = 'pending'; + pendingThenable.then( + fulfilledValue => { + if (thenable.status === 'pending') { + const fulfilledThenable: FulfilledThenable = (thenable: any); + fulfilledThenable.status = 'fulfilled'; + fulfilledThenable.value = fulfilledValue; + } + }, + (error: mixed) => { + if (thenable.status === 'pending') { + const rejectedThenable: RejectedThenable = (thenable: any); + rejectedThenable.status = 'rejected'; + rejectedThenable.reason = error; + } + }, + ); } - const pendingThenable: PendingThenable = (thenable: any); - pendingThenable.status = 'pending'; - pendingThenable.then( - fulfilledValue => { - if (thenable.status === 'pending') { - const fulfilledThenable: FulfilledThenable = (thenable: any); - fulfilledThenable.status = 'fulfilled'; - fulfilledThenable.value = fulfilledValue; - } - }, - (error: mixed) => { - if (thenable.status === 'pending') { - const rejectedThenable: RejectedThenable = (thenable: any); - rejectedThenable.status = 'rejected'; - rejectedThenable.reason = error; - } - }, - ); - break; - } - } -} -export function getPreviouslyUsedThenableAtIndex( - thenableState: ThenableState | null, - index: number, -): Thenable | null { - if (thenableState !== null) { - const thenable = thenableState[index]; - if (thenable !== undefined) { - return thenable; + // Suspend. + // TODO: Throwing here is an implementation detail that allows us to + // unwind the call stack. But we shouldn't allow it to leak into + // userspace. Throw an opaque placeholder value instead of the + // actual thenable. If it doesn't get captured by the work loop, log + // a warning, because that means something in userspace must have + // caught it. + throw thenable; } } - return null; } diff --git a/packages/react-server/src/ReactFlightHooks.js b/packages/react-server/src/ReactFlightHooks.js index 0f7f5b17e9a68..d1fb683260620 100644 --- a/packages/react-server/src/ReactFlightHooks.js +++ b/packages/react-server/src/ReactFlightHooks.js @@ -17,11 +17,7 @@ import { } from 'shared/ReactSymbols'; import {readContext as readContextImpl} from './ReactFlightNewContext'; import {enableUseHook} from 'shared/ReactFeatureFlags'; -import { - getPreviouslyUsedThenableAtIndex, - createThenableState, - trackUsedThenable, -} from './ReactFlightThenable'; +import {createThenableState, trackUsedThenable} from './ReactFlightThenable'; let currentRequest = null; let thenableIndexCounter = 0; @@ -121,8 +117,6 @@ function useId(): string { return ':' + currentRequest.identifierPrefix + 'S' + id.toString(32) + ':'; } -function noop(): void {} - function use(usable: Usable): T { if (usable !== null && typeof usable === 'object') { // $FlowFixMe[method-unbinding] @@ -134,61 +128,10 @@ function use(usable: Usable): T { const index = thenableIndexCounter; thenableIndexCounter += 1; - switch (thenable.status) { - case 'fulfilled': { - const fulfilledValue: T = thenable.value; - return fulfilledValue; - } - case 'rejected': { - const rejectedError = thenable.reason; - throw rejectedError; - } - default: { - const prevThenableAtIndex: Thenable | null = getPreviouslyUsedThenableAtIndex( - thenableState, - index, - ); - if (prevThenableAtIndex !== null) { - if (thenable !== prevThenableAtIndex) { - // Avoid an unhandled rejection errors for the Promises that we'll - // intentionally ignore. - thenable.then(noop, noop); - } - switch (prevThenableAtIndex.status) { - case 'fulfilled': { - const fulfilledValue: T = prevThenableAtIndex.value; - return fulfilledValue; - } - case 'rejected': { - const rejectedError: mixed = prevThenableAtIndex.reason; - throw rejectedError; - } - default: { - // The thenable still hasn't resolved. Suspend with the same - // thenable as last time to avoid redundant listeners. - throw prevThenableAtIndex; - } - } - } else { - // This is the first time something has been used at this index. - // Stash the thenable at the current index so we can reuse it during - // the next attempt. - if (thenableState === null) { - thenableState = createThenableState(); - } - trackUsedThenable(thenableState, thenable, index); - - // Suspend. - // TODO: Throwing here is an implementation detail that allows us to - // unwind the call stack. But we shouldn't allow it to leak into - // userspace. Throw an opaque placeholder value instead of the - // actual thenable. If it doesn't get captured by the work loop, log - // a warning, because that means something in userspace must have - // caught it. - throw thenable; - } - } + if (thenableState === null) { + thenableState = createThenableState(); } + return trackUsedThenable(thenableState, thenable, index); } else if (usable.$$typeof === REACT_SERVER_CONTEXT_TYPE) { const context: ReactServerContext = (usable: any); return readContext(context); diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index dc82d78d0d40d..b91b8db2a9694 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -22,6 +22,9 @@ import type { ServerContextJSONValue, Wakeable, Thenable, + PendingThenable, + FulfilledThenable, + RejectedThenable, } from 'shared/ReactTypes'; import type {LazyComponent} from 'react/src/ReactLazy'; @@ -66,7 +69,6 @@ import { getActiveContext, rootContextSnapshot, } from './ReactFlightNewContext'; -import {trackSuspendedWakeable} from './ReactFlightThenable'; import { REACT_ELEMENT_TYPE, @@ -224,10 +226,44 @@ function readThenable(thenable: Thenable): T { } function createLazyWrapperAroundWakeable(wakeable: Wakeable) { - trackSuspendedWakeable(wakeable); + // This is a temporary fork of the `use` implementation until we accept + // promises everywhere. + const thenable: Thenable = (wakeable: any); + switch (thenable.status) { + case 'fulfilled': + case 'rejected': + break; + default: { + if (typeof thenable.status === 'string') { + // Only instrument the thenable if the status if not defined. If + // it's defined, but an unknown value, assume it's been instrumented by + // some custom userspace implementation. We treat it as "pending". + break; + } + const pendingThenable: PendingThenable = (thenable: any); + pendingThenable.status = 'pending'; + pendingThenable.then( + fulfilledValue => { + if (thenable.status === 'pending') { + const fulfilledThenable: FulfilledThenable = (thenable: any); + fulfilledThenable.status = 'fulfilled'; + fulfilledThenable.value = fulfilledValue; + } + }, + (error: mixed) => { + if (thenable.status === 'pending') { + const rejectedThenable: RejectedThenable = (thenable: any); + rejectedThenable.status = 'rejected'; + rejectedThenable.reason = error; + } + }, + ); + break; + } + } const lazyType: LazyComponent> = { $$typeof: REACT_LAZY_TYPE, - _payload: (wakeable: any), + _payload: thenable, _init: readThenable, }; return lazyType; @@ -818,11 +854,7 @@ export function resolveModelToJSON( ); const ping = newTask.ping; x.then(ping, ping); - - const wakeable: Wakeable = x; - trackSuspendedWakeable(wakeable); newTask.thenableState = getThenableStateAfterSuspending(); - return serializeByRefID(newTask.id); } else { // Something errored. We'll still send everything we have up until this point. @@ -1146,9 +1178,6 @@ function retryTask(request: Request, task: Task): void { // Something suspended again, let's pick it back up later. const ping = task.ping; x.then(ping, ping); - - const wakeable: Wakeable = x; - trackSuspendedWakeable(wakeable); task.thenableState = getThenableStateAfterSuspending(); return; } else { diff --git a/packages/react-server/src/ReactFlightThenable.js b/packages/react-server/src/ReactFlightThenable.js index ac47b76bc1acb..9a91587b04fd7 100644 --- a/packages/react-server/src/ReactFlightThenable.js +++ b/packages/react-server/src/ReactFlightThenable.js @@ -14,15 +14,13 @@ // instead of "Wakeable". Or some other more appropriate name. import type { - Wakeable, Thenable, PendingThenable, FulfilledThenable, RejectedThenable, } from 'shared/ReactTypes'; -// TODO: Sparse arrays are bad for performance. -export opaque type ThenableState = Array | void>; +export opaque type ThenableState = Array>; export function createThenableState(): ThenableState { // The ThenableState is created the first time a component suspends. If it @@ -30,14 +28,27 @@ export function createThenableState(): ThenableState { return []; } -// TODO: Unify this with trackSuspendedThenable. It needs to support not only -// `use`, but async components, too. -export function trackSuspendedWakeable(wakeable: Wakeable) { - // If this wakeable isn't already a thenable, turn it into one now. Then, - // when we resume the work loop, we can check if its status is - // still pending. - // TODO: Get rid of the Wakeable type? It's superseded by UntrackedThenable. - const thenable: Thenable = (wakeable: any); +function noop(): void {} + +export function trackUsedThenable( + thenableState: ThenableState, + thenable: Thenable, + index: number, +): T { + const previous = thenableState[index]; + if (previous === undefined) { + thenableState.push(thenable); + } else { + if (previous !== thenable) { + // Reuse the previous thenable, and drop the new one. We can assume + // they represent the same value, because components are idempotent. + + // Avoid an unhandled rejection errors for the Promises that we'll + // intentionally ignore. + thenable.then(noop, noop); + thenable = previous; + } + } // We use an expando to track the status and result of a thenable so that we // can synchronously unwrap the value. Think of this as an extension of the @@ -46,67 +57,48 @@ export function trackSuspendedWakeable(wakeable: Wakeable) { // If the thenable doesn't have a status, set it to "pending" and attach // a listener that will update its status and result when it resolves. switch (thenable.status) { - case 'fulfilled': - case 'rejected': - // A thenable that already resolved shouldn't have been thrown, so this is - // unexpected. Suggests a mistake in a userspace data library. Don't track - // this thenable, because if we keep trying it will likely infinite loop - // without ever resolving. - // TODO: Log a warning? - break; + case 'fulfilled': { + const fulfilledValue: T = thenable.value; + return fulfilledValue; + } + case 'rejected': { + const rejectedError = thenable.reason; + throw rejectedError; + } default: { if (typeof thenable.status === 'string') { // Only instrument the thenable if the status if not defined. If // it's defined, but an unknown value, assume it's been instrumented by // some custom userspace implementation. We treat it as "pending". - break; + } else { + const pendingThenable: PendingThenable = (thenable: any); + pendingThenable.status = 'pending'; + pendingThenable.then( + fulfilledValue => { + if (thenable.status === 'pending') { + const fulfilledThenable: FulfilledThenable = (thenable: any); + fulfilledThenable.status = 'fulfilled'; + fulfilledThenable.value = fulfilledValue; + } + }, + (error: mixed) => { + if (thenable.status === 'pending') { + const rejectedThenable: RejectedThenable = (thenable: any); + rejectedThenable.status = 'rejected'; + rejectedThenable.reason = error; + } + }, + ); } - const pendingThenable: PendingThenable = (thenable: any); - pendingThenable.status = 'pending'; - pendingThenable.then( - fulfilledValue => { - if (thenable.status === 'pending') { - const fulfilledThenable: FulfilledThenable = (thenable: any); - fulfilledThenable.status = 'fulfilled'; - fulfilledThenable.value = fulfilledValue; - } - }, - (error: mixed) => { - if (thenable.status === 'pending') { - const rejectedThenable: RejectedThenable = (thenable: any); - rejectedThenable.status = 'rejected'; - rejectedThenable.reason = error; - } - }, - ); - break; - } - } -} - -export function trackUsedThenable( - thenableState: ThenableState, - thenable: Thenable, - index: number, -) { - // This is only a separate function from trackSuspendedWakeable for symmetry - // with Fiber. - // TODO: Disallow throwing a thenable directly. It must go through `use` (or - // some equivalent for internal Suspense implementations). We can't do this in - // Fiber yet because it's a breaking change but we can do it in Server - // Components because Server Components aren't released yet. - thenableState[index] = thenable; -} -export function getPreviouslyUsedThenableAtIndex( - thenableState: ThenableState | null, - index: number, -): Thenable | null { - if (thenableState !== null) { - const thenable = thenableState[index]; - if (thenable !== undefined) { - return thenable; + // Suspend. + // TODO: Throwing here is an implementation detail that allows us to + // unwind the call stack. But we shouldn't allow it to leak into + // userspace. Throw an opaque placeholder value instead of the + // actual thenable. If it doesn't get captured by the work loop, log + // a warning, because that means something in userspace must have + // caught it. + throw thenable; } } - return null; }