Skip to content

Commit

Permalink
Unify promise switch statements
Browse files Browse the repository at this point in the history 
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`).
  • Loading branch information
@acdlite
acdlite committed Oct 23, 2022
1 parent 56820b4 commit 9e20bfd
Show file tree
Hide file tree
Showing 8 changed files with 222 additions and 407 deletions.
57 changes: 1 addition & 56 deletions packages/react-reconciler/src/ReactFiberHooks.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ import {now} from './Scheduler';
import {
prepareThenableState,
trackUsedThenable,
getPreviouslyUsedThenableAtIndex,
} from './ReactFiberThenable.new';

const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals;
Expand Down Expand Up @@ -776,8 +775,6 @@ if (enableUseMemoCacheHook) {
};
}

function noop(): void {}

function use<T>(usable: Usable<T>): T {
if (usable !== null && typeof usable === 'object') {
// $FlowFixMe[method-unbinding]
Expand All @@ -788,59 +785,7 @@ function use<T>(usable: Usable<T>): 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<T> | 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
Expand Down
57 changes: 1 addition & 56 deletions packages/react-reconciler/src/ReactFiberHooks.old.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ import {now} from './Scheduler';
import {
prepareThenableState,
trackUsedThenable,
getPreviouslyUsedThenableAtIndex,
} from './ReactFiberThenable.old';

const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals;
Expand Down Expand Up @@ -776,8 +775,6 @@ if (enableUseMemoCacheHook) {
};
}

function noop(): void {}

function use<T>(usable: Usable<T>): T {
if (usable !== null && typeof usable === 'object') {
// $FlowFixMe[method-unbinding]
Expand All @@ -788,59 +785,7 @@ function use<T>(usable: Usable<T>): 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<T> | 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
Expand Down
96 changes: 53 additions & 43 deletions packages/react-reconciler/src/ReactFiberThenable.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<Thenable<any> | void>;
export opaque type ThenableState = Array<Thenable<any>>;

let thenableState: ThenableState | null = null;

Expand Down Expand Up @@ -62,15 +61,30 @@ export function isThenableStateResolved(thenables: ThenableState): boolean {
return true;
}

export function trackUsedThenable<T>(thenable: Thenable<T>, index: number) {
function noop(): void {}

export function trackUsedThenable<T>(thenable: Thenable<T>, index: number): T {
if (__DEV__ && ReactCurrentActQueue.current !== null) {
ReactCurrentActQueue.didUsePromise = true;
}

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
Expand All @@ -80,52 +94,48 @@ export function trackUsedThenable<T>(thenable: Thenable<T>, 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<mixed> = (thenable: any);
pendingThenable.status = 'pending';
pendingThenable.then(
fulfilledValue => {
if (thenable.status === 'pending') {
const fulfilledThenable: FulfilledThenable<mixed> = (thenable: any);
fulfilledThenable.status = 'fulfilled';
fulfilledThenable.value = fulfilledValue;
}
},
(error: mixed) => {
if (thenable.status === 'pending') {
const rejectedThenable: RejectedThenable<mixed> = (thenable: any);
rejectedThenable.status = 'rejected';
rejectedThenable.reason = error;
}
},
);
}
const pendingThenable: PendingThenable<mixed> = (thenable: any);
pendingThenable.status = 'pending';
pendingThenable.then(
fulfilledValue => {
if (thenable.status === 'pending') {
const fulfilledThenable: FulfilledThenable<mixed> = (thenable: any);
fulfilledThenable.status = 'fulfilled';
fulfilledThenable.value = fulfilledValue;
}
},
(error: mixed) => {
if (thenable.status === 'pending') {
const rejectedThenable: RejectedThenable<mixed> = (thenable: any);
rejectedThenable.status = 'rejected';
rejectedThenable.reason = error;
}
},
);
break;
}
}
}

export function getPreviouslyUsedThenableAtIndex<T>(
index: number,
): Thenable<T> | 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;
}
Loading

0 comments on commit 9e20bfd

Please sign in to comment.