Skip to content

Commit fa77f52

Browse files
acdliteacdlite
committed
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`).
1 parent 7572e49 commit fa77f52

8 files changed

+222
-407
lines changed

‎packages/react-reconciler/src/ReactFiberHooks.new.js

+1-56
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,6 @@ import {now} from './Scheduler';
137137
import {
138138
prepareThenableState,
139139
trackUsedThenable,
140-
getPreviouslyUsedThenableAtIndex,
141140
} from './ReactFiberThenable.new';
142141

143142
const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals;
@@ -776,8 +775,6 @@ if (enableUseMemoCacheHook) {
776775
};
777776
}
778777

779-
function noop(): void {}
780-
781778
function use<T>(usable: Usable<T>): T {
782779
if (usable !== null && typeof usable === 'object') {
783780
// $FlowFixMe[method-unbinding]
@@ -788,59 +785,7 @@ function use<T>(usable: Usable<T>): T {
788785
// Track the position of the thenable within this fiber.
789786
const index = thenableIndexCounter;
790787
thenableIndexCounter += 1;
791-
792-
// TODO: Unify this switch statement with the one in trackUsedThenable.
793-
switch (thenable.status) {
794-
case 'fulfilled': {
795-
const fulfilledValue: T = thenable.value;
796-
return fulfilledValue;
797-
}
798-
case 'rejected': {
799-
const rejectedError = thenable.reason;
800-
throw rejectedError;
801-
}
802-
default: {
803-
const prevThenableAtIndex: Thenable<T> | null = getPreviouslyUsedThenableAtIndex(
804-
index,
805-
);
806-
if (prevThenableAtIndex !== null) {
807-
if (thenable !== prevThenableAtIndex) {
808-
// Avoid an unhandled rejection errors for the Promises that we'll
809-
// intentionally ignore.
810-
thenable.then(noop, noop);
811-
}
812-
switch (prevThenableAtIndex.status) {
813-
case 'fulfilled': {
814-
const fulfilledValue: T = prevThenableAtIndex.value;
815-
return fulfilledValue;
816-
}
817-
case 'rejected': {
818-
const rejectedError: mixed = prevThenableAtIndex.reason;
819-
throw rejectedError;
820-
}
821-
default: {
822-
// The thenable still hasn't resolved. Suspend with the same
823-
// thenable as last time to avoid redundant listeners.
824-
throw prevThenableAtIndex;
825-
}
826-
}
827-
} else {
828-
// This is the first time something has been used at this index.
829-
// Stash the thenable at the current index so we can reuse it during
830-
// the next attempt.
831-
trackUsedThenable(thenable, index);
832-
833-
// Suspend.
834-
// TODO: Throwing here is an implementation detail that allows us to
835-
// unwind the call stack. But we shouldn't allow it to leak into
836-
// userspace. Throw an opaque placeholder value instead of the
837-
// actual thenable. If it doesn't get captured by the work loop, log
838-
// a warning, because that means something in userspace must have
839-
// caught it.
840-
throw thenable;
841-
}
842-
}
843-
}
788+
return trackUsedThenable(thenable, index);
844789
} else if (
845790
usable.$$typeof === REACT_CONTEXT_TYPE ||
846791
usable.$$typeof === REACT_SERVER_CONTEXT_TYPE

‎packages/react-reconciler/src/ReactFiberHooks.old.js

+1-56
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,6 @@ import {now} from './Scheduler';
137137
import {
138138
prepareThenableState,
139139
trackUsedThenable,
140-
getPreviouslyUsedThenableAtIndex,
141140
} from './ReactFiberThenable.old';
142141

143142
const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals;
@@ -776,8 +775,6 @@ if (enableUseMemoCacheHook) {
776775
};
777776
}
778777

779-
function noop(): void {}
780-
781778
function use<T>(usable: Usable<T>): T {
782779
if (usable !== null && typeof usable === 'object') {
783780
// $FlowFixMe[method-unbinding]
@@ -788,59 +785,7 @@ function use<T>(usable: Usable<T>): T {
788785
// Track the position of the thenable within this fiber.
789786
const index = thenableIndexCounter;
790787
thenableIndexCounter += 1;
791-
792-
// TODO: Unify this switch statement with the one in trackUsedThenable.
793-
switch (thenable.status) {
794-
case 'fulfilled': {
795-
const fulfilledValue: T = thenable.value;
796-
return fulfilledValue;
797-
}
798-
case 'rejected': {
799-
const rejectedError = thenable.reason;
800-
throw rejectedError;
801-
}
802-
default: {
803-
const prevThenableAtIndex: Thenable<T> | null = getPreviouslyUsedThenableAtIndex(
804-
index,
805-
);
806-
if (prevThenableAtIndex !== null) {
807-
if (thenable !== prevThenableAtIndex) {
808-
// Avoid an unhandled rejection errors for the Promises that we'll
809-
// intentionally ignore.
810-
thenable.then(noop, noop);
811-
}
812-
switch (prevThenableAtIndex.status) {
813-
case 'fulfilled': {
814-
const fulfilledValue: T = prevThenableAtIndex.value;
815-
return fulfilledValue;
816-
}
817-
case 'rejected': {
818-
const rejectedError: mixed = prevThenableAtIndex.reason;
819-
throw rejectedError;
820-
}
821-
default: {
822-
// The thenable still hasn't resolved. Suspend with the same
823-
// thenable as last time to avoid redundant listeners.
824-
throw prevThenableAtIndex;
825-
}
826-
}
827-
} else {
828-
// This is the first time something has been used at this index.
829-
// Stash the thenable at the current index so we can reuse it during
830-
// the next attempt.
831-
trackUsedThenable(thenable, index);
832-
833-
// Suspend.
834-
// TODO: Throwing here is an implementation detail that allows us to
835-
// unwind the call stack. But we shouldn't allow it to leak into
836-
// userspace. Throw an opaque placeholder value instead of the
837-
// actual thenable. If it doesn't get captured by the work loop, log
838-
// a warning, because that means something in userspace must have
839-
// caught it.
840-
throw thenable;
841-
}
842-
}
843-
}
788+
return trackUsedThenable(thenable, index);
844789
} else if (
845790
usable.$$typeof === REACT_CONTEXT_TYPE ||
846791
usable.$$typeof === REACT_SERVER_CONTEXT_TYPE

‎packages/react-reconciler/src/ReactFiberThenable.new.js

+53-43
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,7 @@ import type {
1717
import ReactSharedInternals from 'shared/ReactSharedInternals';
1818
const {ReactCurrentActQueue} = ReactSharedInternals;
1919

20-
// TODO: Sparse arrays are bad for performance.
21-
export opaque type ThenableState = Array<Thenable<any> | void>;
20+
export opaque type ThenableState = Array<Thenable<any>>;
2221

2322
let thenableState: ThenableState | null = null;
2423

@@ -62,15 +61,30 @@ export function isThenableStateResolved(thenables: ThenableState): boolean {
6261
return true;
6362
}
6463

65-
export function trackUsedThenable<T>(thenable: Thenable<T>, index: number) {
64+
function noop(): void {}
65+
66+
export function trackUsedThenable<T>(thenable: Thenable<T>, index: number): T {
6667
if (__DEV__ && ReactCurrentActQueue.current !== null) {
6768
ReactCurrentActQueue.didUsePromise = true;
6869
}
6970

7071
if (thenableState === null) {
7172
thenableState = [thenable];
7273
} else {
73-
thenableState[index] = thenable;
74+
const previous = thenableState[index];
75+
if (previous === undefined) {
76+
thenableState.push(thenable);
77+
} else {
78+
if (previous !== thenable) {
79+
// Reuse the previous thenable, and drop the new one. We can assume
80+
// they represent the same value, because components are idempotent.
81+
82+
// Avoid an unhandled rejection errors for the Promises that we'll
83+
// intentionally ignore.
84+
thenable.then(noop, noop);
85+
thenable = previous;
86+
}
87+
}
7488
}
7589

7690
// We use an expando to track the status and result of a thenable so that we
@@ -80,52 +94,48 @@ export function trackUsedThenable<T>(thenable: Thenable<T>, index: number) {
8094
// If the thenable doesn't have a status, set it to "pending" and attach
8195
// a listener that will update its status and result when it resolves.
8296
switch (thenable.status) {
83-
case 'fulfilled':
84-
case 'rejected':
85-
// A thenable that already resolved shouldn't have been thrown, so this is
86-
// unexpected. Suggests a mistake in a userspace data library. Don't track
87-
// this thenable, because if we keep trying it will likely infinite loop
88-
// without ever resolving.
89-
// TODO: Log a warning?
90-
break;
97+
case 'fulfilled': {
98+
const fulfilledValue: T = thenable.value;
99+
return fulfilledValue;
100+
}
101+
case 'rejected': {
102+
const rejectedError = thenable.reason;
103+
throw rejectedError;
104+
}
91105
default: {
92106
if (typeof thenable.status === 'string') {
93107
// Only instrument the thenable if the status if not defined. If
94108
// it's defined, but an unknown value, assume it's been instrumented by
95109
// some custom userspace implementation. We treat it as "pending".
96-
break;
110+
} else {
111+
const pendingThenable: PendingThenable<mixed> = (thenable: any);
112+
pendingThenable.status = 'pending';
113+
pendingThenable.then(
114+
fulfilledValue => {
115+
if (thenable.status === 'pending') {
116+
const fulfilledThenable: FulfilledThenable<mixed> = (thenable: any);
117+
fulfilledThenable.status = 'fulfilled';
118+
fulfilledThenable.value = fulfilledValue;
119+
}
120+
},
121+
(error: mixed) => {
122+
if (thenable.status === 'pending') {
123+
const rejectedThenable: RejectedThenable<mixed> = (thenable: any);
124+
rejectedThenable.status = 'rejected';
125+
rejectedThenable.reason = error;
126+
}
127+
},
128+
);
97129
}
98-
const pendingThenable: PendingThenable<mixed> = (thenable: any);
99-
pendingThenable.status = 'pending';
100-
pendingThenable.then(
101-
fulfilledValue => {
102-
if (thenable.status === 'pending') {
103-
const fulfilledThenable: FulfilledThenable<mixed> = (thenable: any);
104-
fulfilledThenable.status = 'fulfilled';
105-
fulfilledThenable.value = fulfilledValue;
106-
}
107-
},
108-
(error: mixed) => {
109-
if (thenable.status === 'pending') {
110-
const rejectedThenable: RejectedThenable<mixed> = (thenable: any);
111-
rejectedThenable.status = 'rejected';
112-
rejectedThenable.reason = error;
113-
}
114-
},
115-
);
116-
break;
117-
}
118-
}
119-
}
120130

121-
export function getPreviouslyUsedThenableAtIndex<T>(
122-
index: number,
123-
): Thenable<T> | null {
124-
if (thenableState !== null) {
125-
const thenable = thenableState[index];
126-
if (thenable !== undefined) {
127-
return thenable;
131+
// Suspend.
132+
// TODO: Throwing here is an implementation detail that allows us to
133+
// unwind the call stack. But we shouldn't allow it to leak into
134+
// userspace. Throw an opaque placeholder value instead of the
135+
// actual thenable. If it doesn't get captured by the work loop, log
136+
// a warning, because that means something in userspace must have
137+
// caught it.
138+
throw thenable;
128139
}
129140
}
130-
return null;
131141
}

0 commit comments

Comments
 (0)