Skip to content

Commit 096b356

Browse files
sebmarkbagesebmarkbage
committed
Render children passed to "backwards" in reverse mount order
1 parent 26cf280 commit 096b356

File tree

5 files changed

+140
-33
lines changed

5 files changed

+140
-33
lines changed

‎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)