[SuspenseList] Store lastEffect before rendering (#17131)
* Add a failing test for SuspenseList bug * Store lastEffect before rendering We can't reset the effect list to null because we don't rereconcile the children so we drop deletion effects if we do that. Instead we store the last effect as it was before we started rendering so we can go back to where it was when we reset it. We actually already do something like this when we delete the last row for the tail="hidden" mode so we had a field available for it already.
Sebastian Markbåge committed
Oct 17, 2019 at 15:57 UTC
4eeee358e12c1408a4b40830bb7bb6956cf26b47
3 files changed
+96
-3
packages/react-reconciler/src/ReactFiberBeginWork.js
+8
-2
@@ -2355,18 +2355,20 @@ function initSuspenseListRenderState(
2355
tail: null | Fiber,
2356
lastContentRow: null | Fiber,
2357
tailMode: SuspenseListTailMode,
2358
+ lastEffectBeforeRendering: null | Fiber,
2359
): void {
2360
let renderState: null | SuspenseListRenderState =
2361
workInProgress.memoizedState;
2362
if (renderState === null) {
2362
- workInProgress.memoizedState = {
2363
+ workInProgress.memoizedState = ({
2364
isBackwards: isBackwards,
2365
rendering: null,
2366
last: lastContentRow,
2367
tail: tail,
2368
tailExpiration: 0,
2369
tailMode: tailMode,
2369
- };
2370
+ lastEffect: lastEffectBeforeRendering,
2371
+ }: SuspenseListRenderState);
2372
} else {
2373
// We can reuse the existing object from previous renders.
2374
renderState.isBackwards = isBackwards;
@@ -2375,6 +2377,7 @@ function initSuspenseListRenderState(
2377
renderState.tail = tail;
2378
renderState.tailExpiration = 0;
2379
renderState.tailMode = tailMode;
2380
+ renderState.lastEffect = lastEffectBeforeRendering;
2381
}
2382
}
2383
@@ -2456,6 +2459,7 @@ function updateSuspenseListComponent(
2459
tail,
2460
lastContentRow,
2461
tailMode,
2462
+ workInProgress.lastEffect,
2463
);
2464
break;
2465
}
@@ -2487,6 +2491,7 @@ function updateSuspenseListComponent(
2491
tail,
2492
null, // last
2493
tailMode,
2494
+ workInProgress.lastEffect,
2495
);
2496
break;
2497
}
@@ -2497,6 +2502,7 @@ function updateSuspenseListComponent(
2502
null, // tail
2503
null, // last
2504
undefined,
2505
+ workInProgress.lastEffect,
2506
);
2507
break;
2508
}
packages/react-reconciler/src/ReactFiberCompleteWork.js
+4
-1
@@ -1053,7 +1053,10 @@ function completeWork(
1053
// Rerender the whole list, but this time, we'll force fallbacks
1054
// to stay in place.
1055
// Reset the effect list before doing the second pass since that's now invalid.
1056
- workInProgress.firstEffect = workInProgress.lastEffect = null;
1056
+ if (renderState.lastEffect === null) {
1057
+ workInProgress.firstEffect = null;
1058
+ }
1059
+ workInProgress.lastEffect = renderState.lastEffect;
1060
// Reset the child fibers to their original state.
1061
resetChildFibers(workInProgress, renderExpirationTime);
1062
packages/react-reconciler/src/__tests__/ReactSuspenseList-test.internal.js
+84
@@ -585,6 +585,90 @@ describe('ReactSuspenseList', () => {
585
);
586
});
587
588
+ it('displays all "together" during an update', async () => {
589
+ let A = createAsyncText('A');
590
+ let B = createAsyncText('B');
591
+ let C = createAsyncText('C');
592
+ let D = createAsyncText('D');
593
+
594
+ function Foo({step}) {
595
+ return (
596
+ <SuspenseList revealOrder="together">
597
+ {step === 0 && (
598
+ <Suspense fallback={<Text text="Loading A" />}>
599
+ <A />
600
+ </Suspense>
601
+ )}
602
+ {step === 0 && (
603
+ <Suspense fallback={<Text text="Loading B" />}>
604
+ <B />
605
+ </Suspense>
606
+ )}
607
+ {step === 1 && (
608
+ <Suspense fallback={<Text text="Loading C" />}>
609
+ <C />
610
+ </Suspense>
611
+ )}
612
+ {step === 1 && (
613
+ <Suspense fallback={<Text text="Loading D" />}>
614
+ <D />
615
+ </Suspense>
616
+ )}
617
+ </SuspenseList>
618
+ );
619
+ }
620
+
621
+ // Mount
622
+ await A.resolve();
623
+ ReactNoop.render(<Foo step={0} />);
624
+ expect(Scheduler).toFlushAndYield([
625
+ 'A',
626
+ 'Suspend! [B]',
627
+ 'Loading B',
628
+ 'Loading A',
629
+ 'Loading B',
630
+ ]);
631
+ expect(ReactNoop).toMatchRenderedOutput(
632
+ <>
633
+ <span>Loading A</span>
634
+ <span>Loading B</span>
635
+ </>,
636
+ );
637
+ await B.resolve();
638
+ expect(Scheduler).toFlushAndYield(['A', 'B']);
639
+ expect(ReactNoop).toMatchRenderedOutput(
640
+ <>
641
+ <span>A</span>
642
+ <span>B</span>
643
+ </>,
644
+ );
645
+
646
+ // Update
647
+ await C.resolve();
648
+ ReactNoop.render(<Foo step={1} />);
649
+ expect(Scheduler).toFlushAndYield([
650
+ 'C',
651
+ 'Suspend! [D]',
652
+ 'Loading D',
653
+ 'Loading C',
654
+ 'Loading D',
655
+ ]);
656
+ expect(ReactNoop).toMatchRenderedOutput(
657
+ <>
658
+ <span>Loading C</span>
659
+ <span>Loading D</span>
660
+ </>,
661
+ );
662
+ await D.resolve();
663
+ expect(Scheduler).toFlushAndYield(['C', 'D']);
664
+ expect(ReactNoop).toMatchRenderedOutput(
665
+ <>
666
+ <span>C</span>
667
+ <span>D</span>
668
+ </>,
669
+ );
670
+ });
671
+
672
it('avoided boundaries can be coordinate with SuspenseList', async () => {
673
let A = createAsyncText('A');
674
let B = createAsyncText('B');