Skip to content

Commit 02007a3

Browse files
acdliteacdlite
committed
Revert yieldy behavior for non-use Suspense
To derisk the rollout of `use`, and simplify the implementation, this reverts the yield-to-microtasks behavior for promises that are thrown directly (as opposed to being unwrapped by `use`). We may add this back later. However, the plan is to deprecate throwing a promise directly and migrate all existing Suspense code to `use`, so the extra code probably isn't worth it.
1 parent d44683b commit 02007a3

11 files changed

Lines changed: 46 additions & 138 deletions

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -783,6 +783,7 @@ function use<T>(usable: Usable<T>): T {
783783
const index = thenableIndexCounter;
784784
thenableIndexCounter += 1;
785785

786+
// TODO: Unify this switch statement with the one in trackUsedThenable.
786787
switch (thenable.status) {
787788
case 'fulfilled': {
788789
const fulfilledValue: T = thenable.value;

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

Lines changed: 10 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
*/
99

1010
import type {
11-
Wakeable,
1211
Thenable,
1312
PendingThenable,
1413
FulfilledThenable,
@@ -18,14 +17,8 @@ import type {
1817
import ReactSharedInternals from 'shared/ReactSharedInternals';
1918
const {ReactCurrentActQueue} = ReactSharedInternals;
2019

21-
let suspendedThenable: Thenable<mixed> | null = null;
22-
let adHocSuspendCount: number = 0;
23-
24-
// TODO: Sparse arrays are bad for performance.
20+
let suspendedThenable: Thenable<any> | null = null;
2521
let usedThenables: Array<Thenable<any> | void> | null = null;
26-
let lastUsedThenable: Thenable<any> | null = null;
27-
28-
const MAX_AD_HOC_SUSPEND_COUNT = 50;
2922

3023
export function isTrackingSuspendedThenable(): boolean {
3124
return suspendedThenable !== null;
@@ -39,22 +32,17 @@ export function suspendedThenableDidResolve(): boolean {
3932
return false;
4033
}
4134

42-
export function trackSuspendedWakeable(wakeable: Wakeable) {
43-
// If this wakeable isn't already a thenable, turn it into one now. Then,
44-
// when we resume the work loop, we can check if its status is
45-
// still pending.
46-
// TODO: Get rid of the Wakeable type? It's superseded by UntrackedThenable.
47-
const thenable: Thenable<mixed> = (wakeable: any);
35+
export function trackUsedThenable<T>(thenable: Thenable<T>, index: number) {
36+
if (__DEV__ && ReactCurrentActQueue.current !== null) {
37+
ReactCurrentActQueue.didUsePromise = true;
38+
}
4839

49-
if (thenable !== lastUsedThenable) {
50-
// If this wakeable was not just `use`-d, it must be an ad hoc wakeable
51-
// that was thrown by an older Suspense implementation. Keep a count of
52-
// these so that we can detect an infinite ping loop.
53-
// TODO: Once `use` throws an opaque signal instead of the actual thenable,
54-
// a better way to count ad hoc suspends is whether an actual thenable
55-
// is caught by the work loop.
56-
adHocSuspendCount++;
40+
if (usedThenables === null) {
41+
usedThenables = [thenable];
42+
} else {
43+
usedThenables[index] = thenable;
5744
}
45+
5846
suspendedThenable = thenable;
5947

6048
// We use an expando to track the status and result of a thenable so that we
@@ -105,34 +93,12 @@ export function trackSuspendedWakeable(wakeable: Wakeable) {
10593

10694
export function resetWakeableStateAfterEachAttempt() {
10795
suspendedThenable = null;
108-
adHocSuspendCount = 0;
109-
lastUsedThenable = null;
11096
}
11197

11298
export function resetThenableStateOnCompletion() {
11399
usedThenables = null;
114100
}
115101

116-
export function throwIfInfinitePingLoopDetected() {
117-
if (adHocSuspendCount > MAX_AD_HOC_SUSPEND_COUNT) {
118-
// TODO: Guard against an infinite loop by throwing an error if the same
119-
// component suspends too many times in a row. This should be thrown from
120-
// the render phase so that it gets the component stack.
121-
}
122-
}
123-
124-
export function trackUsedThenable<T>(thenable: Thenable<T>, index: number) {
125-
if (usedThenables === null) {
126-
usedThenables = [];
127-
}
128-
usedThenables[index] = thenable;
129-
lastUsedThenable = thenable;
130-
131-
if (__DEV__ && ReactCurrentActQueue.current !== null) {
132-
ReactCurrentActQueue.didUsePromise = true;
133-
}
134-
}
135-
136102
export function getPreviouslyUsedThenableAtIndex<T>(
137103
index: number,
138104
): Thenable<T> | null {

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

Lines changed: 10 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
*/
99

1010
import type {
11-
Wakeable,
1211
Thenable,
1312
PendingThenable,
1413
FulfilledThenable,
@@ -18,14 +17,8 @@ import type {
1817
import ReactSharedInternals from 'shared/ReactSharedInternals';
1918
const {ReactCurrentActQueue} = ReactSharedInternals;
2019

21-
let suspendedThenable: Thenable<mixed> | null = null;
22-
let adHocSuspendCount: number = 0;
23-
24-
// TODO: Sparse arrays are bad for performance.
20+
let suspendedThenable: Thenable<any> | null = null;
2521
let usedThenables: Array<Thenable<any> | void> | null = null;
26-
let lastUsedThenable: Thenable<any> | null = null;
27-
28-
const MAX_AD_HOC_SUSPEND_COUNT = 50;
2922

3023
export function isTrackingSuspendedThenable(): boolean {
3124
return suspendedThenable !== null;
@@ -39,22 +32,17 @@ export function suspendedThenableDidResolve(): boolean {
3932
return false;
4033
}
4134

42-
export function trackSuspendedWakeable(wakeable: Wakeable) {
43-
// If this wakeable isn't already a thenable, turn it into one now. Then,
44-
// when we resume the work loop, we can check if its status is
45-
// still pending.
46-
// TODO: Get rid of the Wakeable type? It's superseded by UntrackedThenable.
47-
const thenable: Thenable<mixed> = (wakeable: any);
35+
export function trackUsedThenable<T>(thenable: Thenable<T>, index: number) {
36+
if (__DEV__ && ReactCurrentActQueue.current !== null) {
37+
ReactCurrentActQueue.didUsePromise = true;
38+
}
4839

49-
if (thenable !== lastUsedThenable) {
50-
// If this wakeable was not just `use`-d, it must be an ad hoc wakeable
51-
// that was thrown by an older Suspense implementation. Keep a count of
52-
// these so that we can detect an infinite ping loop.
53-
// TODO: Once `use` throws an opaque signal instead of the actual thenable,
54-
// a better way to count ad hoc suspends is whether an actual thenable
55-
// is caught by the work loop.
56-
adHocSuspendCount++;
40+
if (usedThenables === null) {
41+
usedThenables = [thenable];
42+
} else {
43+
usedThenables[index] = thenable;
5744
}
45+
5846
suspendedThenable = thenable;
5947

6048
// We use an expando to track the status and result of a thenable so that we
@@ -105,34 +93,12 @@ export function trackSuspendedWakeable(wakeable: Wakeable) {
10593

10694
export function resetWakeableStateAfterEachAttempt() {
10795
suspendedThenable = null;
108-
adHocSuspendCount = 0;
109-
lastUsedThenable = null;
11096
}
11197

11298
export function resetThenableStateOnCompletion() {
11399
usedThenables = null;
114100
}
115101

116-
export function throwIfInfinitePingLoopDetected() {
117-
if (adHocSuspendCount > MAX_AD_HOC_SUSPEND_COUNT) {
118-
// TODO: Guard against an infinite loop by throwing an error if the same
119-
// component suspends too many times in a row. This should be thrown from
120-
// the render phase so that it gets the component stack.
121-
}
122-
}
123-
124-
export function trackUsedThenable<T>(thenable: Thenable<T>, index: number) {
125-
if (usedThenables === null) {
126-
usedThenables = [];
127-
}
128-
usedThenables[index] = thenable;
129-
lastUsedThenable = thenable;
130-
131-
if (__DEV__ && ReactCurrentActQueue.current !== null) {
132-
ReactCurrentActQueue.didUsePromise = true;
133-
}
134-
}
135-
136102
export function getPreviouslyUsedThenableAtIndex<T>(
137103
index: number,
138104
): Thenable<T> | null {

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

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,6 @@ import {processTransitionCallbacks} from './ReactFiberTracingMarkerComponent.new
267267
import {
268268
resetWakeableStateAfterEachAttempt,
269269
resetThenableStateOnCompletion,
270-
trackSuspendedWakeable,
271270
suspendedThenableDidResolve,
272271
isTrackingSuspendedThenable,
273272
} from './ReactFiberThenable.new';
@@ -1739,11 +1738,6 @@ function handleThrow(root, thrownValue): void {
17391738
return;
17401739
}
17411740

1742-
const isWakeable =
1743-
thrownValue !== null &&
1744-
typeof thrownValue === 'object' &&
1745-
typeof thrownValue.then === 'function';
1746-
17471741
if (enableProfilerTimer && erroredWork.mode & ProfileMode) {
17481742
// Record the time spent rendering before an error was thrown. This
17491743
// avoids inaccurate Profiler durations in the case of a
@@ -1753,7 +1747,11 @@ function handleThrow(root, thrownValue): void {
17531747

17541748
if (enableSchedulingProfiler) {
17551749
markComponentRenderStopped();
1756-
if (isWakeable) {
1750+
if (
1751+
thrownValue !== null &&
1752+
typeof thrownValue === 'object' &&
1753+
typeof thrownValue.then === 'function'
1754+
) {
17571755
const wakeable: Wakeable = (thrownValue: any);
17581756
markComponentSuspended(
17591757
erroredWork,
@@ -1768,12 +1766,6 @@ function handleThrow(root, thrownValue): void {
17681766
);
17691767
}
17701768
}
1771-
1772-
if (isWakeable) {
1773-
const wakeable: Wakeable = (thrownValue: any);
1774-
1775-
trackSuspendedWakeable(wakeable);
1776-
}
17771769
}
17781770

17791771
function pushDispatcher(container) {

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

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,6 @@ import {processTransitionCallbacks} from './ReactFiberTracingMarkerComponent.old
267267
import {
268268
resetWakeableStateAfterEachAttempt,
269269
resetThenableStateOnCompletion,
270-
trackSuspendedWakeable,
271270
suspendedThenableDidResolve,
272271
isTrackingSuspendedThenable,
273272
} from './ReactFiberThenable.old';
@@ -1739,11 +1738,6 @@ function handleThrow(root, thrownValue): void {
17391738
return;
17401739
}
17411740

1742-
const isWakeable =
1743-
thrownValue !== null &&
1744-
typeof thrownValue === 'object' &&
1745-
typeof thrownValue.then === 'function';
1746-
17471741
if (enableProfilerTimer && erroredWork.mode & ProfileMode) {
17481742
// Record the time spent rendering before an error was thrown. This
17491743
// avoids inaccurate Profiler durations in the case of a
@@ -1753,7 +1747,11 @@ function handleThrow(root, thrownValue): void {
17531747

17541748
if (enableSchedulingProfiler) {
17551749
markComponentRenderStopped();
1756-
if (isWakeable) {
1750+
if (
1751+
thrownValue !== null &&
1752+
typeof thrownValue === 'object' &&
1753+
typeof thrownValue.then === 'function'
1754+
) {
17571755
const wakeable: Wakeable = (thrownValue: any);
17581756
markComponentSuspended(
17591757
erroredWork,
@@ -1768,12 +1766,6 @@ function handleThrow(root, thrownValue): void {
17681766
);
17691767
}
17701768
}
1771-
1772-
if (isWakeable) {
1773-
const wakeable: Wakeable = (thrownValue: any);
1774-
1775-
trackSuspendedWakeable(wakeable);
1776-
}
17771769
}
17781770

17791771
function pushDispatcher(container) {

‎packages/react-reconciler/src/__tests__/ReactThenable-test.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,11 @@ describe('ReactWakeable', () => {
2626
return props.text;
2727
}
2828

29+
// This behavior was intentionally disabled to derisk the rollout of `use`.
30+
// It changes the behavior of old, pre-`use` Suspense implementations. We may
31+
// add this back; however, the plan is to migrate all existing Suspense code
32+
// to `use`, so the extra code probably isn't worth it.
33+
// @gate TODO
2934
test('if suspended fiber is pinged in a microtask, retry immediately without unwinding the stack', async () => {
3035
let resolved = false;
3136
function Async() {

‎packages/react-server/src/ReactFizzHooks.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -593,6 +593,7 @@ function use<T>(usable: Usable<T>): T {
593593
const index = thenableIndexCounter;
594594
thenableIndexCounter += 1;
595595

596+
// TODO: Unify this switch statement with the one in trackUsedThenable.
596597
switch (thenable.status) {
597598
case 'fulfilled': {
598599
const fulfilledValue: T = thenable.value;

‎packages/react-server/src/ReactFizzServer.js‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import type {
1717
ReactContext,
1818
ReactProviderType,
1919
OffscreenMode,
20-
Wakeable,
2120
} from 'shared/ReactTypes';
2221
import type {LazyComponent as LazyComponentType} from 'react/src/ReactLazy';
2322
import type {
@@ -139,7 +138,6 @@ import {
139138
import assign from 'shared/assign';
140139
import getComponentNameFromType from 'shared/getComponentNameFromType';
141140
import isArray from 'shared/isArray';
142-
import {trackSuspendedWakeable} from './ReactFizzThenable';
143141

144142
const ReactCurrentDispatcher = ReactSharedInternals.ReactCurrentDispatcher;
145143
const ReactCurrentCache = ReactSharedInternals.ReactCurrentCache;
@@ -1554,8 +1552,6 @@ function spawnNewSuspendedTask(
15541552
task.treeContext,
15551553
);
15561554

1557-
trackSuspendedWakeable(x);
1558-
15591555
if (__DEV__) {
15601556
if (task.componentStack !== null) {
15611557
// We pop one task off the stack because the node that suspended will be tried again,
@@ -1879,9 +1875,6 @@ function retryTask(request: Request, task: Task): void {
18791875
// Something suspended again, let's pick it back up later.
18801876
const ping = task.ping;
18811877
x.then(ping, ping);
1882-
1883-
const wakeable: Wakeable = x;
1884-
trackSuspendedWakeable(wakeable);
18851878
task.thenableState = getThenableStateAfterSuspending();
18861879
} else {
18871880
task.abortSet.delete(task);

‎packages/react-server/src/ReactFizzThenable.js‎

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
// instead of "Wakeable". Or some other more appropriate name.
1515

1616
import type {
17-
Wakeable,
1817
Thenable,
1918
PendingThenable,
2019
FulfilledThenable,
@@ -30,12 +29,12 @@ export function createThenableState(): ThenableState {
3029
return [];
3130
}
3231

33-
export function trackSuspendedWakeable(wakeable: Wakeable) {
34-
// If this wakeable isn't already a thenable, turn it into one now. Then,
35-
// when we resume the work loop, we can check if its status is
36-
// still pending.
37-
// TODO: Get rid of the Wakeable type? It's superseded by UntrackedThenable.
38-
const thenable: Thenable<mixed> = (wakeable: any);
32+
export function trackUsedThenable<T>(
33+
thenableState: ThenableState,
34+
thenable: Thenable<T>,
35+
index: number,
36+
) {
37+
thenableState[index] = thenable;
3938

4039
// We use an expando to track the status and result of a thenable so that we
4140
// can synchronously unwrap the value. Think of this as an extension of the
@@ -82,16 +81,6 @@ export function trackSuspendedWakeable(wakeable: Wakeable) {
8281
}
8382
}
8483

85-
export function trackUsedThenable<T>(
86-
thenableState: ThenableState,
87-
thenable: Thenable<T>,
88-
index: number,
89-
) {
90-
// This is only a separate function from trackSuspendedWakeable for symmetry
91-
// with Fiber.
92-
thenableState[index] = thenable;
93-
}
94-
9584
export function getPreviouslyUsedThenableAtIndex<T>(
9685
thenableState: ThenableState | null,
9786
index: number,

‎packages/react-server/src/ReactFlightThenable.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ export function createThenableState(): ThenableState {
3030
return [];
3131
}
3232

33+
// TODO: Unify this with trackSuspendedThenable. It needs to support not only
34+
// `use`, but async components, too.
3335
export function trackSuspendedWakeable(wakeable: Wakeable) {
3436
// If this wakeable isn't already a thenable, turn it into one now. Then,
3537
// when we resume the work loop, we can check if its status is

0 commit comments

Comments
 (0)