Revert yieldy behavior for non-use Suspense (#25537)
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.
Andrew Clark committed
Oct 22, 2022 at 17:52 UTC
dd5c2082572a6bf21530b5eecd138b9a455558fe
16 files changed
+75
-175
packages/react-reconciler/src/ReactFiberHooks.new.js
+2
-1
@@ -136,7 +136,7 @@ import {now} from './Scheduler';
136
import {
137
trackUsedThenable,
138
getPreviouslyUsedThenableAtIndex,
139
-} from './ReactFiberWakeable.new';
139
+} from './ReactFiberThenable.new';
140
141
const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals;
142
@@ -783,6 +783,7 @@ function use<T>(usable: Usable<T>): T {
783
const index = thenableIndexCounter;
784
thenableIndexCounter += 1;
785
786
+ // TODO: Unify this switch statement with the one in trackUsedThenable.
787
switch (thenable.status) {
788
case 'fulfilled': {
789
const fulfilledValue: T = thenable.value;
packages/react-reconciler/src/ReactFiberHooks.old.js
+2
-1
@@ -136,7 +136,7 @@ import {now} from './Scheduler';
136
import {
137
trackUsedThenable,
138
getPreviouslyUsedThenableAtIndex,
139
-} from './ReactFiberWakeable.old';
139
+} from './ReactFiberThenable.old';
140
141
const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals;
142
@@ -783,6 +783,7 @@ function use<T>(usable: Usable<T>): T {
783
const index = thenableIndexCounter;
784
thenableIndexCounter += 1;
785
786
+ // TODO: Unify this switch statement with the one in trackUsedThenable.
787
switch (thenable.status) {
788
case 'fulfilled': {
789
const fulfilledValue: T = thenable.value;
packages/react-reconciler/src/ReactFiberThenable.new.js
renamed
+10
-44
@@ -8,7 +8,6 @@
8
*/
9
10
import type {
11
- Wakeable,
11
Thenable,
12
PendingThenable,
13
FulfilledThenable,
@@ -18,14 +17,8 @@ import type {
17
import ReactSharedInternals from 'shared/ReactSharedInternals';
18
const {ReactCurrentActQueue} = ReactSharedInternals;
19
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;
21
let usedThenables: Array<Thenable<any> | void> | null = null;
26
-let lastUsedThenable: Thenable<any> | null = null;
27
-
28
-const MAX_AD_HOC_SUSPEND_COUNT = 50;
22
23
export function isTrackingSuspendedThenable(): boolean {
24
return suspendedThenable !== null;
@@ -39,22 +32,17 @@ export function suspendedThenableDidResolve(): boolean {
32
return false;
33
}
34
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
+ }
39
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;
44
}
45
+
46
suspendedThenable = thenable;
47
48
// 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) {
93
94
export function resetWakeableStateAfterEachAttempt() {
95
suspendedThenable = null;
108
- adHocSuspendCount = 0;
109
- lastUsedThenable = null;
96
}
97
98
export function resetThenableStateOnCompletion() {
99
usedThenables = null;
100
}
101
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
-
102
export function getPreviouslyUsedThenableAtIndex<T>(
103
index: number,
104
): Thenable<T> | null {
packages/react-reconciler/src/ReactFiberThenable.old.js
renamed
+10
-44
@@ -8,7 +8,6 @@
8
*/
9
10
import type {
11
- Wakeable,
11
Thenable,
12
PendingThenable,
13
FulfilledThenable,
@@ -18,14 +17,8 @@ import type {
17
import ReactSharedInternals from 'shared/ReactSharedInternals';
18
const {ReactCurrentActQueue} = ReactSharedInternals;
19
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;
21
let usedThenables: Array<Thenable<any> | void> | null = null;
26
-let lastUsedThenable: Thenable<any> | null = null;
27
-
28
-const MAX_AD_HOC_SUSPEND_COUNT = 50;
22
23
export function isTrackingSuspendedThenable(): boolean {
24
return suspendedThenable !== null;
@@ -39,22 +32,17 @@ export function suspendedThenableDidResolve(): boolean {
32
return false;
33
}
34
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
+ }
39
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;
44
}
45
+
46
suspendedThenable = thenable;
47
48
// 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) {
93
94
export function resetWakeableStateAfterEachAttempt() {
95
suspendedThenable = null;
108
- adHocSuspendCount = 0;
109
- lastUsedThenable = null;
96
}
97
98
export function resetThenableStateOnCompletion() {
99
usedThenables = null;
100
}
101
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
-
102
export function getPreviouslyUsedThenableAtIndex<T>(
103
index: number,
104
): Thenable<T> | null {
packages/react-reconciler/src/ReactFiberWorkLoop.new.js
+6
-14
@@ -267,10 +267,9 @@ import {processTransitionCallbacks} from './ReactFiberTracingMarkerComponent.new
267
import {
268
resetWakeableStateAfterEachAttempt,
269
resetThenableStateOnCompletion,
270
- trackSuspendedWakeable,
270
suspendedThenableDidResolve,
271
isTrackingSuspendedThenable,
273
-} from './ReactFiberWakeable.new';
272
+} from './ReactFiberThenable.new';
273
import {schedulePostPaintCallback} from './ReactPostPaintCallback';
274
275
const ceil = Math.ceil;
@@ -1739,11 +1738,6 @@ function handleThrow(root, thrownValue): void {
1738
return;
1739
}
1740
1742
- const isWakeable =
1743
- thrownValue !== null &&
1744
- typeof thrownValue === 'object' &&
1745
- typeof thrownValue.then === 'function';
1746
-
1741
if (enableProfilerTimer && erroredWork.mode & ProfileMode) {
1742
// Record the time spent rendering before an error was thrown. This
1743
// avoids inaccurate Profiler durations in the case of a
@@ -1753,7 +1747,11 @@ function handleThrow(root, thrownValue): void {
1747
1748
if (enableSchedulingProfiler) {
1749
markComponentRenderStopped();
1756
- if (isWakeable) {
1750
+ if (
1751
+ thrownValue !== null &&
1752
+ typeof thrownValue === 'object' &&
1753
+ typeof thrownValue.then === 'function'
1754
+ ) {
1755
const wakeable: Wakeable = (thrownValue: any);
1756
markComponentSuspended(
1757
erroredWork,
@@ -1768,12 +1766,6 @@ function handleThrow(root, thrownValue): void {
1766
);
1767
}
1768
}
1771
-
1772
- if (isWakeable) {
1773
- const wakeable: Wakeable = (thrownValue: any);
1774
-
1775
- trackSuspendedWakeable(wakeable);
1776
- }
1769
}
1770
1771
function pushDispatcher(container) {
packages/react-reconciler/src/ReactFiberWorkLoop.old.js
+6
-14
@@ -267,10 +267,9 @@ import {processTransitionCallbacks} from './ReactFiberTracingMarkerComponent.old
267
import {
268
resetWakeableStateAfterEachAttempt,
269
resetThenableStateOnCompletion,
270
- trackSuspendedWakeable,
270
suspendedThenableDidResolve,
271
isTrackingSuspendedThenable,
273
-} from './ReactFiberWakeable.old';
272
+} from './ReactFiberThenable.old';
273
import {schedulePostPaintCallback} from './ReactPostPaintCallback';
274
275
const ceil = Math.ceil;
@@ -1739,11 +1738,6 @@ function handleThrow(root, thrownValue): void {
1738
return;
1739
}
1740
1742
- const isWakeable =
1743
- thrownValue !== null &&
1744
- typeof thrownValue === 'object' &&
1745
- typeof thrownValue.then === 'function';
1746
-
1741
if (enableProfilerTimer && erroredWork.mode & ProfileMode) {
1742
// Record the time spent rendering before an error was thrown. This
1743
// avoids inaccurate Profiler durations in the case of a
@@ -1753,7 +1747,11 @@ function handleThrow(root, thrownValue): void {
1747
1748
if (enableSchedulingProfiler) {
1749
markComponentRenderStopped();
1756
- if (isWakeable) {
1750
+ if (
1751
+ thrownValue !== null &&
1752
+ typeof thrownValue === 'object' &&
1753
+ typeof thrownValue.then === 'function'
1754
+ ) {
1755
const wakeable: Wakeable = (thrownValue: any);
1756
markComponentSuspended(
1757
erroredWork,
@@ -1768,12 +1766,6 @@ function handleThrow(root, thrownValue): void {
1766
);
1767
}
1768
}
1771
-
1772
- if (isWakeable) {
1773
- const wakeable: Wakeable = (thrownValue: any);
1774
-
1775
- trackSuspendedWakeable(wakeable);
1776
- }
1769
}
1770
1771
function pushDispatcher(container) {
packages/react-reconciler/src/__tests__/ReactOffscreenSuspense-test.js
+16
-26
@@ -485,32 +485,22 @@ describe('ReactOffscreen', () => {
485
// In the same render, also hide the offscreen tree.
486
root.render(<App show={false} />);
487
488
- if (gate(flags => flags.enableSyncDefaultUpdates)) {
489
- expect(Scheduler).toFlushUntilNextPaint([
490
- // The outer update will commit, but the inner update is deferred until
491
- // a later render.
492
- 'Outer: 1',
493
-
494
- // Something suspended. This means we won't commit immediately; there
495
- // will be an async gap between render and commit. In this test, we will
496
- // use this property to schedule a concurrent update. The fact that
497
- // we're using Suspense to schedule a concurrent update is not directly
498
- // relevant to the test — we could also use time slicing, but I've
499
- // chosen to use Suspense the because implementation details of time
500
- // slicing are more volatile.
501
- 'Suspend! [Async: 1]',
502
-
503
- 'Loading...',
504
- ]);
505
- } else {
506
- // When default updates are time sliced, React yields before preparing
507
- // the fallback.
508
- expect(Scheduler).toFlushUntilNextPaint([
509
- 'Outer: 1',
510
- 'Suspend! [Async: 1]',
511
- ]);
512
- expect(Scheduler).toFlushUntilNextPaint(['Loading...']);
513
- }
488
+ expect(Scheduler).toFlushUntilNextPaint([
489
+ // The outer update will commit, but the inner update is deferred until
490
+ // a later render.
491
+ 'Outer: 1',
492
+
493
+ // Something suspended. This means we won't commit immediately; there
494
+ // will be an async gap between render and commit. In this test, we will
495
+ // use this property to schedule a concurrent update. The fact that
496
+ // we're using Suspense to schedule a concurrent update is not directly
497
+ // relevant to the test — we could also use time slicing, but I've
498
+ // chosen to use Suspense the because implementation details of time
499
+ // slicing are more volatile.
500
+ 'Suspend! [Async: 1]',
501
+
502
+ 'Loading...',
503
+ ]);
504
505
// Assert that we haven't committed quite yet
506
expect(root).toMatchRenderedOutput(
packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js
+1
@@ -3874,6 +3874,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
3874
'Suspend! [A2]',
3875
'Loading...',
3876
'Suspend! [B2]',
3877
+ 'Loading...',
3878
]);
3879
expect(root).toMatchRenderedOutput(
3880
<>
packages/react-reconciler/src/__tests__/ReactThenable-test.js
renamed
+5
@@ -26,6 +26,11 @@ describe('ReactWakeable', () => {
26
return props.text;
27
}
28
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
34
test('if suspended fiber is pinged in a microtask, retry immediately without unwinding the stack', async () => {
35
let resolved = false;
36
function Async() {
packages/react-server/src/ReactFizzHooks.js
+3
-2
@@ -21,7 +21,7 @@ import type {
21
22
import type {ResponseState} from './ReactServerFormatConfig';
23
import type {Task} from './ReactFizzServer';
24
-import type {ThenableState} from './ReactFizzWakeable';
24
+import type {ThenableState} from './ReactFizzThenable';
25
26
import {readContext as readContextImpl} from './ReactFizzNewContext';
27
import {getTreeId} from './ReactFizzTreeContext';
@@ -29,7 +29,7 @@ import {
29
getPreviouslyUsedThenableAtIndex,
30
createThenableState,
31
trackUsedThenable,
32
-} from './ReactFizzWakeable';
32
+} from './ReactFizzThenable';
33
34
import {makeId} from './ReactServerFormatConfig';
35
@@ -593,6 +593,7 @@ function use<T>(usable: Usable<T>): T {
593
const index = thenableIndexCounter;
594
thenableIndexCounter += 1;
595
596
+ // TODO: Unify this switch statement with the one in trackUsedThenable.
597
switch (thenable.status) {
598
case 'fulfilled': {
599
const fulfilledValue: T = thenable.value;
packages/react-server/src/ReactFizzServer.js
+1
-8
@@ -17,7 +17,6 @@ import type {
17
ReactContext,
18
ReactProviderType,
19
OffscreenMode,
20
- Wakeable,
20
} from 'shared/ReactTypes';
21
import type {LazyComponent as LazyComponentType} from 'react/src/ReactLazy';
22
import type {
@@ -30,7 +29,7 @@ import type {
29
import type {ContextSnapshot} from './ReactFizzNewContext';
30
import type {ComponentStackNode} from './ReactFizzComponentStack';
31
import type {TreeContext} from './ReactFizzTreeContext';
33
-import type {ThenableState} from './ReactFizzWakeable';
32
+import type {ThenableState} from './ReactFizzThenable';
33
34
import {
35
scheduleWork,
@@ -139,7 +138,6 @@ import {
138
import assign from 'shared/assign';
139
import getComponentNameFromType from 'shared/getComponentNameFromType';
140
import isArray from 'shared/isArray';
142
-import {trackSuspendedWakeable} from './ReactFizzWakeable';
141
142
const ReactCurrentDispatcher = ReactSharedInternals.ReactCurrentDispatcher;
143
const ReactCurrentCache = ReactSharedInternals.ReactCurrentCache;
@@ -1554,8 +1552,6 @@ function spawnNewSuspendedTask(
1552
task.treeContext,
1553
);
1554
1557
- trackSuspendedWakeable(x);
1558
-
1555
if (__DEV__) {
1556
if (task.componentStack !== null) {
1557
// 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 {
1875
// Something suspended again, let's pick it back up later.
1876
const ping = task.ping;
1877
x.then(ping, ping);
1882
-
1883
- const wakeable: Wakeable = x;
1884
- trackSuspendedWakeable(wakeable);
1878
task.thenableState = getThenableStateAfterSuspending();
1879
} else {
1880
task.abortSet.delete(task);
packages/react-server/src/ReactFizzThenable.js
renamed
+6
-17
@@ -14,7 +14,6 @@
14
// instead of "Wakeable". Or some other more appropriate name.
15
16
import type {
17
- Wakeable,
17
Thenable,
18
PendingThenable,
19
FulfilledThenable,
@@ -30,12 +29,12 @@ export function createThenableState(): ThenableState {
29
return [];
30
}
31
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;
38
39
// We use an expando to track the status and result of a thenable so that we
40
// can synchronously unwrap the value. Think of this as an extension of the
@@ -82,16 +81,6 @@ export function trackSuspendedWakeable(wakeable: Wakeable) {
81
}
82
}
83
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
-
84
export function getPreviouslyUsedThenableAtIndex<T>(
85
thenableState: ThenableState | null,
86
index: number,
packages/react-server/src/ReactFlightHooks.js
+2
-2
@@ -10,7 +10,7 @@
10
import type {Dispatcher} from 'react-reconciler/src/ReactInternalTypes';
11
import type {Request} from './ReactFlightServer';
12
import type {ReactServerContext, Thenable, Usable} from 'shared/ReactTypes';
13
-import type {ThenableState} from './ReactFlightWakeable';
13
+import type {ThenableState} from './ReactFlightThenable';
14
import {
15
REACT_SERVER_CONTEXT_TYPE,
16
REACT_MEMO_CACHE_SENTINEL,
@@ -21,7 +21,7 @@ import {
21
getPreviouslyUsedThenableAtIndex,
22
createThenableState,
23
trackUsedThenable,
24
-} from './ReactFlightWakeable';
24
+} from './ReactFlightThenable';
25
26
let currentRequest = null;
27
let thenableIndexCounter = 0;
packages/react-server/src/ReactFlightServer.js
+2
-2
@@ -16,7 +16,7 @@ import type {
16
ModuleKey,
17
} from './ReactFlightServerConfig';
18
import type {ContextSnapshot} from './ReactFlightNewContext';
19
-import type {ThenableState} from './ReactFlightWakeable';
19
+import type {ThenableState} from './ReactFlightThenable';
20
import type {
21
ReactProviderType,
22
ServerContextJSONValue,
@@ -64,7 +64,7 @@ import {
64
getActiveContext,
65
rootContextSnapshot,
66
} from './ReactFlightNewContext';
67
-import {trackSuspendedWakeable} from './ReactFlightWakeable';
67
+import {trackSuspendedWakeable} from './ReactFlightThenable';
68
69
import {
70
REACT_ELEMENT_TYPE,
packages/react-server/src/ReactFlightThenable.js
renamed
+2
@@ -30,6 +30,8 @@ export function createThenableState(): ThenableState {
30
return [];
31
}
32
33
+// TODO: Unify this with trackSuspendedThenable. It needs to support not only
34
+// `use`, but async components, too.
35
export function trackSuspendedWakeable(wakeable: Wakeable) {
36
// If this wakeable isn't already a thenable, turn it into one now. Then,
37
// when we resume the work loop, we can check if its status is
scripts/jest/TestFlags.js
+1
@@ -48,6 +48,7 @@ const environmentFlags = {
48
49
// Use this for tests that are known to be broken.
50
FIXME: false,
51
+ TODO: false,
52
53
// Turn these flags back on (or delete) once the effect list is removed in
54
// favor of a depth-first traversal using `subtreeTags`.