Skip to content

Commit 488d88b

Browse files
sebmarkbagesebmarkbage
authored
Render children passed to "backwards" SuspenseList in reverse mount order (facebook#35021)
Stacked on facebook#35018. This mounts the children of SuspenseList backwards. Meaning the first child is mounted last in the DOM (and effect list). It's like calling reverse() on the children. This is meant to set us up for allowing AsyncIterable children where the unknown number of children streams in at the end (which is the beginning in a backwards SuspenseList). For consistency we do that with other children too. `unstable_legacy-backwards` still exists for the old mode but is meant to be deprecated. <img width="100" alt="image" src="https://github.com/user-attachments/assets/5c2a95d7-34c4-4a4e-b602-3646a834d779" />
1 parent 26cf280 commit 488d88b

5 files changed

Lines changed: 140 additions & 33 deletions

File tree

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

Lines changed: 75 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -656,6 +656,77 @@ describe('ReactDOMFizzSuspenseList', () => {
656656
);
657657
});
658658

659+
// @gate enableSuspenseList
660+
it('displays each items in "backwards" mount order', async () => {
661+
const A = createAsyncText('A');
662+
const B = createAsyncText('B');
663+
const C = createAsyncText('C');
664+
665+
function Foo() {
666+
return (
667+
<div>
668+
<SuspenseList revealOrder="backwards" tail="visible">
669+
<Suspense fallback={<Text text="Loading C" />}>
670+
<C />
671+
</Suspense>
672+
<Suspense fallback={<Text text="Loading B" />}>
673+
<B />
674+
</Suspense>
675+
<Suspense fallback={<Text text="Loading A" />}>
676+
<A />
677+
</Suspense>
678+
</SuspenseList>
679+
</div>
680+
);
681+
}
682+
683+
await A.resolve();
684+
685+
await serverAct(async () => {
686+
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<Foo />);
687+
pipe(writable);
688+
});
689+
690+
assertLog([
691+
'Suspend! [C]',
692+
'Suspend! [B]', // TODO: Defer rendering the content after fallback if previous suspended,
693+
'A',
694+
'Loading C',
695+
'Loading B',
696+
'Loading A',
697+
]);
698+
699+
expect(getVisibleChildren(container)).toEqual(
700+
<div>
701+
<span>Loading A</span>
702+
<span>Loading B</span>
703+
<span>Loading C</span>
704+
</div>,
705+
);
706+
707+
await serverAct(() => C.resolve());
708+
assertLog(['C']);
709+
710+
expect(getVisibleChildren(container)).toEqual(
711+
<div>
712+
<span>Loading A</span>
713+
<span>Loading B</span>
714+
<span>C</span>
715+
</div>,
716+
);
717+
718+
await serverAct(() => B.resolve());
719+
assertLog(['B']);
720+
721+
expect(getVisibleChildren(container)).toEqual(
722+
<div>
723+
<span>A</span>
724+
<span>B</span>
725+
<span>C</span>
726+
</div>,
727+
);
728+
});
729+
659730
// @gate enableSuspenseList
660731
it('displays each items in "backwards" order in legacy mode', async () => {
661732
const A = createAsyncText('A');
@@ -737,15 +808,13 @@ describe('ReactDOMFizzSuspenseList', () => {
737808
return (
738809
<div>
739810
<SuspenseList revealOrder="forwards" tail="visible">
740-
<SuspenseList
741-
revealOrder="unstable_legacy-backwards"
742-
tail="visible">
743-
<Suspense fallback={<Text text="Loading A" />}>
744-
<A />
745-
</Suspense>
811+
<SuspenseList revealOrder="backwards" tail="visible">
746812
<Suspense fallback={<Text text="Loading B" />}>
747813
<B />
748814
</Suspense>
815+
<Suspense fallback={<Text text="Loading A" />}>
816+
<A />
817+
</Suspense>
749818
</SuspenseList>
750819
<Suspense fallback={<Text text="Loading C" />}>
751820
<C />

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

Lines changed: 54 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3247,18 +3247,14 @@ function validateRevealOrder(revealOrder: SuspenseListRevealOrder) {
32473247
if (
32483248
revealOrder != null &&
32493249
revealOrder !== 'forwards' &&
3250+
revealOrder !== 'backwards' &&
32503251
revealOrder !== 'unstable_legacy-backwards' &&
32513252
revealOrder !== 'together' &&
32523253
revealOrder !== 'independent' &&
32533254
!didWarnAboutRevealOrder[cacheKey]
32543255
) {
32553256
didWarnAboutRevealOrder[cacheKey] = true;
3256-
if (revealOrder === 'backwards') {
3257-
console.error(
3258-
'The rendering order of <SuspenseList revealOrder="backwards"> is changing. ' +
3259-
'To be future compatible you must specify revealOrder="legacy_unstable-backwards" instead.',
3260-
);
3261-
} else if (typeof revealOrder === 'string') {
3257+
if (typeof revealOrder === 'string') {
32623258
switch (revealOrder.toLowerCase()) {
32633259
case 'together':
32643260
case 'forwards':
@@ -3371,6 +3367,17 @@ function initSuspenseListRenderState(
33713367
}
33723368
}
33733369

3370+
function reverseChildren(fiber: Fiber): void {
3371+
let row = fiber.child;
3372+
fiber.child = null;
3373+
while (row !== null) {
3374+
const nextRow = row.sibling;
3375+
row.sibling = fiber.child;
3376+
fiber.child = row;
3377+
row = nextRow;
3378+
}
3379+
}
3380+
33743381
// This can end up rendering this component multiple passes.
33753382
// The first pass splits the children fibers into two sets. A head and tail.
33763383
// We first render the head. If anything is in fallback state, we do another
@@ -3409,7 +3416,16 @@ function updateSuspenseListComponent(
34093416
validateTailOptions(tailMode, revealOrder);
34103417
validateSuspenseListChildren(newChildren, revealOrder);
34113418

3412-
reconcileChildren(current, workInProgress, newChildren, renderLanes);
3419+
if (revealOrder === 'backwards' && current !== null) {
3420+
// For backwards the current mounted set will be backwards. Reconciling against it
3421+
// will lead to mismatches and reorders. We need to swap the original set first
3422+
// and then restore it afterwards.
3423+
reverseChildren(current);
3424+
reconcileChildren(current, workInProgress, newChildren, renderLanes);
3425+
reverseChildren(current);
3426+
} else {
3427+
reconcileChildren(current, workInProgress, newChildren, renderLanes);
3428+
}
34133429
// Read how many children forks this set pushed so we can push it every time we retry.
34143430
const treeForkCount = getIsHydrating() ? getForksAtLevel(workInProgress) : 0;
34153431

@@ -3434,7 +3450,37 @@ function updateSuspenseListComponent(
34343450
workInProgress.memoizedState = null;
34353451
} else {
34363452
switch (revealOrder) {
3437-
case 'backwards':
3453+
case 'backwards': {
3454+
// We're going to find the first row that has existing content.
3455+
// We are also going to reverse the order of anything in the existing content
3456+
// since we want to actually render them backwards from the reconciled set.
3457+
// The tail is left in order, because it'll be added to the front as we
3458+
// complete each item.
3459+
const lastContentRow = findLastContentRow(workInProgress.child);
3460+
let tail;
3461+
if (lastContentRow === null) {
3462+
// The whole list is part of the tail.
3463+
tail = workInProgress.child;
3464+
workInProgress.child = null;
3465+
} else {
3466+
// Disconnect the tail rows after the content row.
3467+
// We're going to render them separately later in reverse order.
3468+
tail = lastContentRow.sibling;
3469+
lastContentRow.sibling = null;
3470+
// We have to now reverse the main content so it renders backwards too.
3471+
reverseChildren(workInProgress);
3472+
}
3473+
// TODO: If workInProgress.child is null, we can continue on the tail immediately.
3474+
initSuspenseListRenderState(
3475+
workInProgress,
3476+
true, // isBackwards
3477+
tail,
3478+
null, // last
3479+
tailMode,
3480+
treeForkCount,
3481+
);
3482+
break;
3483+
}
34383484
case 'unstable_legacy-backwards': {
34393485
// We're going to find the first row that has existing content.
34403486
// At the same time we're going to reverse the list of everything

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1838,10 +1838,6 @@ function completeWork(
18381838
}
18391839
}
18401840
if (renderState.isBackwards) {
1841-
// The effect list of the backwards tail will have been added
1842-
// to the end. This breaks the guarantee that life-cycles fire in
1843-
// sibling order but that isn't a strong guarantee promised by React.
1844-
// Especially since these might also just pop in during future commits.
18451841
// Append to the beginning of the list.
18461842
renderedTail.sibling = workInProgress.child;
18471843
workInProgress.child = renderedTail;

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

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1022,22 +1022,22 @@ describe('ReactSuspenseList', () => {
10221022
});
10231023

10241024
// @gate enableSuspenseList
1025-
it('warns if revealOrder="backwards" is specified', async () => {
1025+
it('displays each items in "backwards" order', async () => {
10261026
const A = createAsyncText('A');
10271027
const B = createAsyncText('B');
10281028
const C = createAsyncText('C');
10291029

10301030
function Foo() {
10311031
return (
10321032
<SuspenseList revealOrder="backwards" tail="visible">
1033-
<Suspense fallback={<Text text="Loading A" />}>
1034-
<A />
1033+
<Suspense fallback={<Text text="Loading C" />}>
1034+
<C />
10351035
</Suspense>
10361036
<Suspense fallback={<Text text="Loading B" />}>
10371037
<B />
10381038
</Suspense>
1039-
<Suspense fallback={<Text text="Loading C" />}>
1040-
<C />
1039+
<Suspense fallback={<Text text="Loading A" />}>
1040+
<A />
10411041
</Suspense>
10421042
</SuspenseList>
10431043
);
@@ -1056,14 +1056,6 @@ describe('ReactSuspenseList', () => {
10561056
'Suspend! [C]',
10571057
]);
10581058

1059-
assertConsoleErrorDev([
1060-
'The rendering order of <SuspenseList revealOrder="backwards"> is changing. ' +
1061-
'To be future compatible you must specify ' +
1062-
'revealOrder="legacy_unstable-backwards" instead.' +
1063-
'\n in SuspenseList (at **)' +
1064-
'\n in Foo (at **)',
1065-
]);
1066-
10671059
expect(ReactNoop).toMatchRenderedOutput(
10681060
<>
10691061
<span>Loading A</span>
@@ -1101,7 +1093,7 @@ describe('ReactSuspenseList', () => {
11011093
});
11021094

11031095
// @gate enableSuspenseList
1104-
it('displays each items in "backwards" order', async () => {
1096+
it('displays each items in "backwards" order (legacy)', async () => {
11051097
const A = createAsyncText('A');
11061098
const B = createAsyncText('B');
11071099
const C = createAsyncText('C');

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2017,7 +2017,11 @@ function renderSuspenseListRows(
20172017
const parentSegment = task.blockedSegment;
20182018
const childIndex = parentSegment.children.length;
20192019
const insertionIndex = parentSegment.chunks.length;
2020-
for (let i = totalChildren - 1; i >= 0; i--) {
2020+
for (let n = 0; n < totalChildren; n++) {
2021+
const i =
2022+
revealOrder === 'unstable_legacy-backwards'
2023+
? totalChildren - 1 - n
2024+
: n;
20212025
const node = rows[i];
20222026
task.row = previousSuspenseListRow = createSuspenseListRow(
20232027
previousSuspenseListRow,

0 commit comments

Comments
 (0)