Skip to content

Commit d742611

Browse files
sebmarkbagesebmarkbage
authored
Replace Implicit Options on SuspenseList with Explicit Options (#33424)
We want to change the defaults for `revealOrder` and `tail` on SuspenseList. This is an intermediate step to allow experimental users to upgrade. To explicitly specify these options I added `revealOrder="independent"` and `tail="visible"`. I then added warnings if `undefined` or `null` is passed. You must now always explicitly specify them. However, semantics are still preserved for now until the next step. We also want to change the rendering order of the `children` prop for `revealOrder="backwards"`. As an intermediate step I first added `revealOrder="unstable_legacy-backwards"` option. This will only be temporary until all users can switch to the new `"backwards"` semantics once we flip it in the next step. I also clarified the types that the directional props requires iterable children but not iterable inside of those. Rows with multiple items can be modeled as explicit fragments.
1 parent 1540081 commit d742611

18 files changed

+397
-72
lines changed

‎.eslintrc.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -611,6 +611,7 @@ module.exports = {
611611
TimeoutID: 'readonly',
612612
WheelEventHandler: 'readonly',
613613
FinalizationRegistry: 'readonly',
614+
Exclude: 'readonly',
614615
Omit: 'readonly',
615616
Keyframe: 'readonly',
616617
PropertyIndexedKeyframes: 'readonly',

‎fixtures/ssr/src/components/LargeContent.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import React, {
66

77
export default function LargeContent() {
88
return (
9-
<SuspenseList revealOrder="forwards">
9+
<SuspenseList revealOrder="forwards" tail="visible">
1010
<Suspense fallback={null}>
1111
<p>
1212
Lorem ipsum dolor sit amet, consectetur adipiscing elit. Mauris

‎packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1289,7 +1289,7 @@ describe('ReactDOMFizzServer', () => {
12891289
function App({showMore}) {
12901290
return (
12911291
<div>
1292-
<SuspenseList revealOrder="forwards">
1292+
<SuspenseList revealOrder="forwards" tail="visible">
12931293
{a}
12941294
{b}
12951295
{showMore ? (

‎packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2254,7 +2254,7 @@ describe('ReactDOMFizzStaticBrowser', () => {
22542254
function App() {
22552255
return (
22562256
<div>
2257-
<SuspenseList revealOrder="forwards">
2257+
<SuspenseList revealOrder="forwards" tail="visible">
22582258
<Suspense fallback="Loading A">
22592259
<ComponentA />
22602260
</Suspense>

‎packages/react-dom/src/__tests__/ReactDOMFizzSuspenseList-test.js

Lines changed: 75 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,70 @@ describe('ReactDOMFizzSuspenseList', () => {
197197
);
198198
});
199199

200+
// @gate enableSuspenseList
201+
it('independently with revealOrder="independent"', async () => {
202+
const A = createAsyncText('A');
203+
const B = createAsyncText('B');
204+
const C = createAsyncText('C');
205+
206+
function Foo() {
207+
return (
208+
<div>
209+
<SuspenseList revealOrder="independent">
210+
<Suspense fallback={<Text text="Loading A" />}>
211+
<A />
212+
</Suspense>
213+
<Suspense fallback={<Text text="Loading B" />}>
214+
<B />
215+
</Suspense>
216+
<Suspense fallback={<Text text="Loading C" />}>
217+
<C />
218+
</Suspense>
219+
</SuspenseList>
220+
</div>
221+
);
222+
}
223+
224+
await A.resolve();
225+
226+
await serverAct(async () => {
227+
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<Foo />);
228+
pipe(writable);
229+
});
230+
231+
assertLog(['A', 'Suspend! [B]', 'Suspend! [C]', 'Loading B', 'Loading C']);
232+
233+
expect(getVisibleChildren(container)).toEqual(
234+
<div>
235+
<span>A</span>
236+
<span>Loading B</span>
237+
<span>Loading C</span>
238+
</div>,
239+
);
240+
241+
await serverAct(() => C.resolve());
242+
assertLog(['C']);
243+
244+
expect(getVisibleChildren(container)).toEqual(
245+
<div>
246+
<span>A</span>
247+
<span>Loading B</span>
248+
<span>C</span>
249+
</div>,
250+
);
251+
252+
await serverAct(() => B.resolve());
253+
assertLog(['B']);
254+
255+
expect(getVisibleChildren(container)).toEqual(
256+
<div>
257+
<span>A</span>
258+
<span>B</span>
259+
<span>C</span>
260+
</div>,
261+
);
262+
});
263+
200264
// @gate enableSuspenseList
201265
it('displays all "together"', async () => {
202266
const A = createAsyncText('A');
@@ -452,7 +516,7 @@ describe('ReactDOMFizzSuspenseList', () => {
452516
});
453517

454518
// @gate enableSuspenseList
455-
it('displays all "together" in nested SuspenseLists where the inner is default', async () => {
519+
it('displays all "together" in nested SuspenseLists where the inner is "independent"', async () => {
456520
const A = createAsyncText('A');
457521
const B = createAsyncText('B');
458522
const C = createAsyncText('C');
@@ -464,7 +528,7 @@ describe('ReactDOMFizzSuspenseList', () => {
464528
<Suspense fallback={<Text text="Loading A" />}>
465529
<A />
466530
</Suspense>
467-
<SuspenseList>
531+
<SuspenseList revealOrder="independent">
468532
<Suspense fallback={<Text text="Loading B" />}>
469533
<B />
470534
</Suspense>
@@ -523,7 +587,7 @@ describe('ReactDOMFizzSuspenseList', () => {
523587
function Foo() {
524588
return (
525589
<div>
526-
<SuspenseList revealOrder="forwards">
590+
<SuspenseList revealOrder="forwards" tail="visible">
527591
<Suspense fallback={<Text text="Loading A" />}>
528592
<A />
529593
</Suspense>
@@ -586,15 +650,15 @@ describe('ReactDOMFizzSuspenseList', () => {
586650
});
587651

588652
// @gate enableSuspenseList
589-
it('displays each items in "backwards" order', async () => {
653+
it('displays each items in "backwards" order in legacy mode', async () => {
590654
const A = createAsyncText('A');
591655
const B = createAsyncText('B');
592656
const C = createAsyncText('C');
593657

594658
function Foo() {
595659
return (
596660
<div>
597-
<SuspenseList revealOrder="backwards">
661+
<SuspenseList revealOrder="unstable_legacy-backwards" tail="visible">
598662
<Suspense fallback={<Text text="Loading A" />}>
599663
<A />
600664
</Suspense>
@@ -665,8 +729,10 @@ describe('ReactDOMFizzSuspenseList', () => {
665729
function Foo() {
666730
return (
667731
<div>
668-
<SuspenseList revealOrder="forwards">
669-
<SuspenseList revealOrder="backwards">
732+
<SuspenseList revealOrder="forwards" tail="visible">
733+
<SuspenseList
734+
revealOrder="unstable_legacy-backwards"
735+
tail="visible">
670736
<Suspense fallback={<Text text="Loading A" />}>
671737
<A />
672738
</Suspense>
@@ -736,7 +802,7 @@ describe('ReactDOMFizzSuspenseList', () => {
736802
function Foo() {
737803
return (
738804
<div>
739-
<SuspenseList revealOrder="forwards">
805+
<SuspenseList revealOrder="forwards" tail="visible">
740806
<Suspense fallback={<Text text="Loading A" />}>
741807
<A />
742808
</Suspense>
@@ -791,7 +857,7 @@ describe('ReactDOMFizzSuspenseList', () => {
791857
function Foo() {
792858
return (
793859
<div>
794-
<SuspenseList revealOrder="forwards">
860+
<SuspenseList revealOrder="forwards" tail="visible">
795861
<Suspense fallback={<Text text="Loading A" />}>
796862
<A />
797863
</Suspense>

‎packages/react-dom/src/__tests__/ReactDOMFloat-test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5755,7 +5755,7 @@ body {
57555755
<html>
57565756
<body>
57575757
<Suspense fallback="loading...">
5758-
<SuspenseList revealOrder="forwards">
5758+
<SuspenseList revealOrder="forwards" tail="visible">
57595759
<Suspense fallback="loading foo...">
57605760
<BlockedOn value="foo">
57615761
<link rel="stylesheet" href="foo" precedence="foo" />

‎packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2362,7 +2362,7 @@ describe('ReactDOMServerPartialHydration', () => {
23622362

23632363
function App({showMore}) {
23642364
return (
2365-
<SuspenseList revealOrder="forwards">
2365+
<SuspenseList revealOrder="forwards" tail="visible">
23662366
{a}
23672367
{b}
23682368
{showMore ? (

‎packages/react-dom/src/__tests__/ReactDOMServerSuspense-test.internal.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ describe('ReactDOMServerSuspense', () => {
123123
// @gate enableSuspenseList
124124
it('server renders a SuspenseList component and its children', async () => {
125125
const example = (
126-
<SuspenseList>
126+
<SuspenseList revealOrder="forwards" tail="visible">
127127
<React.Suspense fallback="Loading A">
128128
<div>A</div>
129129
</React.Suspense>

‎packages/react-dom/src/__tests__/ReactWrongReturnPointer-test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ test('regression (#20932): return pointer is correct before entering deleted tre
172172

173173
function App() {
174174
return (
175-
<SuspenseList revealOrder="forwards">
175+
<SuspenseList revealOrder="forwards" tail="visible">
176176
<Suspense fallback={<Text text="Loading Async..." />}>
177177
<Async />
178178
</Suspense>

‎packages/react-reconciler/src/ReactChildFiber.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2097,7 +2097,9 @@ export function validateSuspenseListChildren(
20972097
) {
20982098
if (__DEV__) {
20992099
if (
2100-
(revealOrder === 'forwards' || revealOrder === 'backwards') &&
2100+
(revealOrder === 'forwards' ||
2101+
revealOrder === 'backwards' ||
2102+
revealOrder === 'unstable_legacy-backwards') &&
21012103
children !== undefined &&
21022104
children !== null &&
21032105
children !== false

‎packages/react-reconciler/src/ReactFiberBeginWork.js

Lines changed: 52 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,7 @@ if (__DEV__) {
337337
didWarnAboutContextTypes = ({}: {[string]: boolean});
338338
didWarnAboutGetDerivedStateOnFunctionComponent = ({}: {[string]: boolean});
339339
didWarnAboutReassigningProps = false;
340-
didWarnAboutRevealOrder = ({}: {[empty]: boolean});
340+
didWarnAboutRevealOrder = ({}: {[string]: boolean});
341341
didWarnAboutTailOptions = ({}: {[string]: boolean});
342342
didWarnAboutDefaultPropsOnFunctionComponent = ({}: {[string]: boolean});
343343
didWarnAboutClassNameOnViewTransition = ({}: {[string]: boolean});
@@ -3225,19 +3225,32 @@ function findLastContentRow(firstChild: null | Fiber): null | Fiber {
32253225

32263226
function validateRevealOrder(revealOrder: SuspenseListRevealOrder) {
32273227
if (__DEV__) {
3228+
const cacheKey = revealOrder == null ? 'null' : revealOrder;
32283229
if (
3229-
revealOrder !== undefined &&
32303230
revealOrder !== 'forwards' &&
3231-
revealOrder !== 'backwards' &&
3231+
revealOrder !== 'unstable_legacy-backwards' &&
32323232
revealOrder !== 'together' &&
3233-
!didWarnAboutRevealOrder[revealOrder]
3233+
revealOrder !== 'independent' &&
3234+
!didWarnAboutRevealOrder[cacheKey]
32343235
) {
3235-
didWarnAboutRevealOrder[revealOrder] = true;
3236-
if (typeof revealOrder === 'string') {
3236+
didWarnAboutRevealOrder[cacheKey] = true;
3237+
if (revealOrder == null) {
3238+
console.error(
3239+
'The default for the <SuspenseList revealOrder="..."> prop is changing. ' +
3240+
'To be future compatible you must explictly specify either ' +
3241+
'"independent" (the current default), "together", "forwards" or "legacy_unstable-backwards".',
3242+
);
3243+
} else if (revealOrder === 'backwards') {
3244+
console.error(
3245+
'The rendering order of <SuspenseList revealOrder="backwards"> is changing. ' +
3246+
'To be future compatible you must specify revealOrder="legacy_unstable-backwards" instead.',
3247+
);
3248+
} else if (typeof revealOrder === 'string') {
32373249
switch (revealOrder.toLowerCase()) {
32383250
case 'together':
32393251
case 'forwards':
3240-
case 'backwards': {
3252+
case 'backwards':
3253+
case 'independent': {
32413254
console.error(
32423255
'"%s" is not a valid value for revealOrder on <SuspenseList />. ' +
32433256
'Use lowercase "%s" instead.',
@@ -3259,15 +3272,15 @@ function validateRevealOrder(revealOrder: SuspenseListRevealOrder) {
32593272
default:
32603273
console.error(
32613274
'"%s" is not a supported revealOrder on <SuspenseList />. ' +
3262-
'Did you mean "together", "forwards" or "backwards"?',
3275+
'Did you mean "independent", "together", "forwards" or "backwards"?',
32633276
revealOrder,
32643277
);
32653278
break;
32663279
}
32673280
} else {
32683281
console.error(
32693282
'%s is not a supported value for revealOrder on <SuspenseList />. ' +
3270-
'Did you mean "together", "forwards" or "backwards"?',
3283+
'Did you mean "independent", "together", "forwards" or "backwards"?',
32713284
revealOrder,
32723285
);
32733286
}
@@ -3280,16 +3293,38 @@ function validateTailOptions(
32803293
revealOrder: SuspenseListRevealOrder,
32813294
) {
32823295
if (__DEV__) {
3283-
if (tailMode !== undefined && !didWarnAboutTailOptions[tailMode]) {
3284-
if (tailMode !== 'collapsed' && tailMode !== 'hidden') {
3285-
didWarnAboutTailOptions[tailMode] = true;
3296+
const cacheKey = tailMode == null ? 'null' : tailMode;
3297+
if (!didWarnAboutTailOptions[cacheKey]) {
3298+
if (tailMode == null) {
3299+
if (
3300+
revealOrder === 'forwards' ||
3301+
revealOrder === 'backwards' ||
3302+
revealOrder === 'unstable_legacy-backwards'
3303+
) {
3304+
didWarnAboutTailOptions[cacheKey] = true;
3305+
console.error(
3306+
'The default for the <SuspenseList tail="..."> prop is changing. ' +
3307+
'To be future compatible you must explictly specify either ' +
3308+
'"visible" (the current default), "collapsed" or "hidden".',
3309+
);
3310+
}
3311+
} else if (
3312+
tailMode !== 'visible' &&
3313+
tailMode !== 'collapsed' &&
3314+
tailMode !== 'hidden'
3315+
) {
3316+
didWarnAboutTailOptions[cacheKey] = true;
32863317
console.error(
32873318
'"%s" is not a supported value for tail on <SuspenseList />. ' +
3288-
'Did you mean "collapsed" or "hidden"?',
3319+
'Did you mean "visible", "collapsed" or "hidden"?',
32893320
tailMode,
32903321
);
3291-
} else if (revealOrder !== 'forwards' && revealOrder !== 'backwards') {
3292-
didWarnAboutTailOptions[tailMode] = true;
3322+
} else if (
3323+
revealOrder !== 'forwards' &&
3324+
revealOrder !== 'backwards' &&
3325+
revealOrder !== 'unstable_legacy-backwards'
3326+
) {
3327+
didWarnAboutTailOptions[cacheKey] = true;
32933328
console.error(
32943329
'<SuspenseList tail="%s" /> is only valid if revealOrder is ' +
32953330
'"forwards" or "backwards". ' +
@@ -3414,7 +3449,8 @@ function updateSuspenseListComponent(
34143449
);
34153450
break;
34163451
}
3417-
case 'backwards': {
3452+
case 'backwards':
3453+
case 'unstable_legacy-backwards': {
34183454
// We're going to find the first row that has existing content.
34193455
// At the same time we're going to reverse the list of everything
34203456
// we pass in the meantime. That's going to be our tail in reverse

‎packages/react-reconciler/src/ReactFiberSuspenseComponent.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,12 @@ export function findFirstSuspended(row: Fiber): null | Fiber {
7575
}
7676
} else if (
7777
node.tag === SuspenseListComponent &&
78-
// revealOrder undefined can't be trusted because it don't
78+
// Independent revealOrder can't be trusted because it doesn't
7979
// keep track of whether it suspended or not.
80-
node.memoizedProps.revealOrder !== undefined
80+
(node.memoizedProps.revealOrder === 'forwards' ||
81+
node.memoizedProps.revealOrder === 'backwards' ||
82+
node.memoizedProps.revealOrder === 'unstable_legacy-backwards' ||
83+
node.memoizedProps.revealOrder === 'together')
8184
) {
8285
const didSuspend = (node.flags & DidCapture) !== NoFlags;
8386
if (didSuspend) {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -677,7 +677,7 @@ describe('ReactLazyContextPropagation', () => {
677677
setContext = setValue;
678678
const children = React.useMemo(
679679
() => (
680-
<SuspenseList revealOrder="forwards">
680+
<SuspenseList revealOrder="forwards" tail="visible">
681681
<Child />
682682
<Child />
683683
</SuspenseList>

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ describe('ReactFragment', () => {
255255
onCaughtError,
256256
}).render(
257257
<CatchingBoundary>
258-
<SuspenseList>
258+
<SuspenseList revealOrder="independent">
259259
<SomethingThatErrors />
260260
</SuspenseList>
261261
</CatchingBoundary>,

0 commit comments

Comments
 (0)