From 9e20bfdd4c33e0c134d9d2fbb7e0e64ec8d1d239 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Sat, 22 Oct 2022 20:39:19 -0400 Subject: [PATCH] Unify promise switch statements There are two different switch statements that we use to unwrap a `use`-ed promise, but there really only needs to be one. This was a factoring artifact that arose because I implemented the yieldy `status` instrumentation thing before I implemented `use` (for promises that are thrown directly during render, which is the old Suspense pattern that will be superseded by `use`). --- .../src/ReactFiberHooks.new.js | 57 +---------- .../src/ReactFiberHooks.old.js | 57 +---------- .../src/ReactFiberThenable.new.js | 96 ++++++++++-------- .../src/ReactFiberThenable.old.js | 96 ++++++++++-------- packages/react-server/src/ReactFizzHooks.js | 64 +----------- .../react-server/src/ReactFizzThenable.js | 97 ++++++++++--------- packages/react-server/src/ReactFlightHooks.js | 65 +------------ .../react-server/src/ReactFlightThenable.js | 97 ++++++++++--------- 8 files changed, 222 insertions(+), 407 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.new.js b/packages/react-reconciler/src/ReactFiberHooks.new.js index 11c6a61f83483..0b6c540ecc741 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.new.js +++ b/packages/react-reconciler/src/ReactFiberHooks.new.js @@ -137,7 +137,6 @@ import {now} from './Scheduler'; import { prepareThenableState, trackUsedThenable, - getPreviouslyUsedThenableAtIndex, } from './ReactFiberThenable.new'; const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals; @@ -776,8 +775,6 @@ if (enableUseMemoCacheHook) { }; } -function noop(): void {} - function use(usable: Usable): T { if (usable !== null && typeof usable === 'object') { // $FlowFixMe[method-unbinding] @@ -788,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 705e8b659c2ea..cb5eeb4adb459 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.old.js +++ b/packages/react-reconciler/src/ReactFiberHooks.old.js @@ -137,7 +137,6 @@ import {now} from './Scheduler'; import { prepareThenableState, trackUsedThenable, - getPreviouslyUsedThenableAtIndex, } from './ReactFiberThenable.old'; const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals; @@ -776,8 +775,6 @@ if (enableUseMemoCacheHook) { }; } -function noop(): void {} - function use(usable: Usable): T { if (usable !== null && typeof usable === 'object') { // $FlowFixMe[method-unbinding] @@ -788,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 a7b958006a332..c2de159f89ca9 100644 --- a/packages/react-reconciler/src/ReactFiberThenable.new.js +++ b/packages/react-reconciler/src/ReactFiberThenable.new.js @@ -17,8 +17,7 @@ import type { import ReactSharedInternals from 'shared/ReactSharedInternals'; const {ReactCurrentActQueue} = ReactSharedInternals; -// TODO: Sparse arrays are bad for performance. -export opaque type ThenableState = Array | void>; +export opaque type ThenableState = Array>; let thenableState: ThenableState | null = null; @@ -62,7 +61,9 @@ export function isThenableStateResolved(thenables: ThenableState): boolean { 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; } @@ -70,7 +71,20 @@ export function trackUsedThenable(thenable: Thenable, index: number) { if (thenableState === null) { thenableState = [thenable]; } else { - thenableState[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. + + // 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 @@ -80,52 +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? - 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( - 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-reconciler/src/ReactFiberThenable.old.js b/packages/react-reconciler/src/ReactFiberThenable.old.js index a7b958006a332..c2de159f89ca9 100644 --- a/packages/react-reconciler/src/ReactFiberThenable.old.js +++ b/packages/react-reconciler/src/ReactFiberThenable.old.js @@ -17,8 +17,7 @@ import type { import ReactSharedInternals from 'shared/ReactSharedInternals'; const {ReactCurrentActQueue} = ReactSharedInternals; -// TODO: Sparse arrays are bad for performance. -export opaque type ThenableState = Array | void>; +export opaque type ThenableState = Array>; let thenableState: ThenableState | null = null; @@ -62,7 +61,9 @@ export function isThenableStateResolved(thenables: ThenableState): boolean { 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; } @@ -70,7 +71,20 @@ export function trackUsedThenable(thenable: Thenable, index: number) { if (thenableState === null) { thenableState = [thenable]; } else { - thenableState[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. + + // 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 @@ -80,52 +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? - 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( - 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/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/ReactFlightThenable.js b/packages/react-server/src/ReactFlightThenable.js index eaeb635217d68..9a91587b04fd7 100644 --- a/packages/react-server/src/ReactFlightThenable.js +++ b/packages/react-server/src/ReactFlightThenable.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; }