Add tail="hidden" option to SuspenseList (#16024)
* Move misaligned comment * Add tail="hidden" option * isShowingAnyFallbacks -> findFirstSuspended * We can't reset Placement tags or we'll forget to insert them * Delete hasSuspendedChildrenAndNewContent optimization
Sebastian Markbåge committed
Jul 12, 2019 at 15:55 UTC
fcff9c57bc41f58e8802016b4dbc0a7b72cc63ad
5 files changed
+232
-134
packages/react-reconciler/src/ReactFiber.js
+4
-3
@@ -27,7 +27,7 @@ import type {ReactEventComponentInstance} from 'shared/ReactTypes';
27
import invariant from 'shared/invariant';
28
import warningWithoutStack from 'shared/warningWithoutStack';
29
import {enableProfilerTimer, enableFlareAPI} from 'shared/ReactFeatureFlags';
30
-import {NoEffect} from 'shared/ReactSideEffectTags';
30
+import {NoEffect, Placement} from 'shared/ReactSideEffectTags';
31
import {ConcurrentRoot, BatchedRoot} from 'shared/ReactRootTags';
32
import {
33
IndeterminateComponent,
@@ -494,8 +494,9 @@ export function resetWorkInProgress(
494
// We assume pendingProps, index, key, ref, return are still untouched to
495
// avoid doing another reconciliation.
496
497
- // Reset the effect tag.
498
- workInProgress.effectTag = NoEffect;
497
+ // Reset the effect tag but keep any Placement tags, since that's something
498
+ // that child fiber is setting, not the reconciliation.
499
+ workInProgress.effectTag &= Placement;
500
501
// The effect list is no longer valid.
502
workInProgress.nextEffect = null;
packages/react-reconciler/src/ReactFiberBeginWork.js
+7
-7
@@ -125,7 +125,7 @@ import {
125
addSubtreeSuspenseContext,
126
setShallowSuspenseContext,
127
} from './ReactFiberSuspenseContext';
128
-import {isShowingAnyFallbacks} from './ReactFiberSuspenseComponent';
128
+import {findFirstSuspended} from './ReactFiberSuspenseComponent';
129
import {
130
pushProvider,
131
propagateContextChange,
@@ -2000,7 +2000,7 @@ function findLastContentRow(firstChild: null | Fiber): null | Fiber {
2000
while (row !== null) {
2001
let currentRow = row.alternate;
2002
// New rows can't be content rows.
2003
- if (currentRow !== null && !isShowingAnyFallbacks(currentRow)) {
2003
+ if (currentRow !== null && findFirstSuspended(currentRow) === null) {
2004
lastContentRow = row;
2005
}
2006
row = row.sibling;
@@ -2072,12 +2072,12 @@ function validateTailOptions(
2072
) {
2073
if (__DEV__) {
2074
if (tailMode !== undefined && !didWarnAboutTailOptions[tailMode]) {
2075
- if (tailMode !== 'collapsed') {
2075
+ if (tailMode !== 'collapsed' && tailMode !== 'hidden') {
2076
didWarnAboutTailOptions[tailMode] = true;
2077
warning(
2078
false,
2079
'"%s" is not a supported value for tail on <SuspenseList />. ' +
2080
- 'Did you mean "collapsed"?',
2080
+ 'Did you mean "collapsed" or "hidden"?',
2081
tailMode,
2082
);
2083
} else if (revealOrder !== 'forwards' && revealOrder !== 'backwards') {
@@ -2242,10 +2242,10 @@ function updateSuspenseListComponent(
2242
pushSuspenseContext(workInProgress, suspenseContext);
2243
2244
if ((workInProgress.mode & BatchedMode) === NoMode) {
2245
- workInProgress.memoizedState = null;
2246
- } else {
2245
// Outside of batched mode, SuspenseList doesn't work so we just
2246
// use make it a noop by treating it as the default revealOrder.
2247
+ workInProgress.memoizedState = null;
2248
+ } else {
2249
switch (revealOrder) {
2250
case 'forwards': {
2251
let lastContentRow = findLastContentRow(workInProgress.child);
@@ -2281,7 +2281,7 @@ function updateSuspenseListComponent(
2281
while (row !== null) {
2282
let currentRow = row.alternate;
2283
// New rows can't be content rows.
2284
- if (currentRow !== null && !isShowingAnyFallbacks(currentRow)) {
2284
+ if (currentRow !== null && findFirstSuspended(currentRow) === null) {
2285
// This is the beginning of the main content.
2286
workInProgress.child = row;
2287
break;
packages/react-reconciler/src/ReactFiberCompleteWork.js
+102
-112
@@ -92,7 +92,7 @@ import {
92
ForceSuspenseFallback,
93
setDefaultShallowSuspenseContext,
94
} from './ReactFiberSuspenseContext';
95
-import {isShowingAnyFallbacks} from './ReactFiberSuspenseComponent';
95
+import {findFirstSuspended} from './ReactFiberSuspenseComponent';
96
import {
97
isContextProvider as isLegacyContextProvider,
98
popContext as popLegacyContext,
@@ -537,6 +537,32 @@ function cutOffTailIfNeeded(
537
hasRenderedATailFallback: boolean,
538
) {
539
switch (renderState.tailMode) {
540
+ case 'hidden': {
541
+ // Any insertions at the end of the tail list after this point
542
+ // should be invisible. If there are already mounted boundaries
543
+ // anything before them are not considered for collapsing.
544
+ // Therefore we need to go through the whole tail to find if
545
+ // there are any.
546
+ let tailNode = renderState.tail;
547
+ let lastTailNode = null;
548
+ while (tailNode !== null) {
549
+ if (tailNode.alternate !== null) {
550
+ lastTailNode = tailNode;
551
+ }
552
+ tailNode = tailNode.sibling;
553
+ }
554
+ // Next we're simply going to delete all insertions after the
555
+ // last rendered item.
556
+ if (lastTailNode === null) {
557
+ // All remaining items in the tail are insertions.
558
+ renderState.tail = null;
559
+ } else {
560
+ // Detach the insertion after the last node that was already
561
+ // inserted.
562
+ lastTailNode.sibling = null;
563
+ }
564
+ break;
565
+ }
566
case 'collapsed': {
567
// Any insertions at the end of the tail list after this point
568
// should be invisible. If there are already mounted boundaries
@@ -572,89 +598,6 @@ function cutOffTailIfNeeded(
598
}
599
}
600
575
-// Note this, might mutate the workInProgress passed in.
576
-function hasSuspendedChildrenAndNewContent(
577
- workInProgress: Fiber,
578
- firstChild: null | Fiber,
579
-): boolean {
580
- // Traversal to see if any of the immediately nested Suspense boundaries
581
- // are in their fallback states. I.e. something suspended in them.
582
- // And if some of them have new content that wasn't already visible.
583
- let hasSuspendedBoundaries = false;
584
- let hasNewContent = false;
585
-
586
- let node = firstChild;
587
- while (node !== null) {
588
- // TODO: Hidden subtrees should not be considered.
589
- if (node.tag === SuspenseComponent) {
590
- const state: SuspenseState | null = node.memoizedState;
591
- const isShowingFallback = state !== null;
592
- if (isShowingFallback) {
593
- // Tag the parent fiber as having suspended boundaries.
594
- if (!hasSuspendedBoundaries) {
595
- workInProgress.effectTag |= DidCapture;
596
- }
597
-
598
- hasSuspendedBoundaries = true;
599
-
600
- if (node.updateQueue !== null) {
601
- // If this is a newly suspended tree, it might not get committed as
602
- // part of the second pass. In that case nothing will subscribe to
603
- // its thennables. Instead, we'll transfer its thennables to the
604
- // SuspenseList so that it can retry if they resolve.
605
- // There might be multiple of these in the list but since we're
606
- // going to wait for all of them anyway, it doesn't really matter
607
- // which ones gets to ping. In theory we could get clever and keep
608
- // track of how many dependencies remain but it gets tricky because
609
- // in the meantime, we can add/remove/change items and dependencies.
610
- // We might bail out of the loop before finding any but that
611
- // doesn't matter since that means that the other boundaries that
612
- // we did find already has their listeners attached.
613
- workInProgress.updateQueue = node.updateQueue;
614
- workInProgress.effectTag |= Update;
615
- }
616
- } else {
617
- const current = node.alternate;
618
- const wasNotShowingContent =
619
- current === null || current.memoizedState !== null;
620
- if (wasNotShowingContent) {
621
- hasNewContent = true;
622
- }
623
- }
624
- if (hasSuspendedBoundaries && hasNewContent) {
625
- return true;
626
- }
627
- } else {
628
- // TODO: We can probably just use the information from the list and not
629
- // drill into its children just like if it was a Suspense boundary.
630
- if (node.tag === SuspenseListComponent && node.updateQueue !== null) {
631
- // If there's a nested SuspenseList, we might have transferred
632
- // the thennables set to it already so we must get it from there.
633
- workInProgress.updateQueue = node.updateQueue;
634
- workInProgress.effectTag |= Update;
635
- }
636
-
637
- if (node.child !== null) {
638
- node.child.return = node;
639
- node = node.child;
640
- continue;
641
- }
642
- }
643
- if (node === workInProgress) {
644
- return false;
645
- }
646
- while (node.sibling === null) {
647
- if (node.return === null || node.return === workInProgress) {
648
- return false;
649
- }
650
- node = node.return;
651
- }
652
- node.sibling.return = node.return;
653
- node = node.sibling;
654
- }
655
- return false;
656
-}
657
-
601
function completeWork(
602
current: Fiber | null,
603
workInProgress: Fiber,
@@ -988,7 +931,7 @@ function completeWork(
931
if (!didSuspendAlready) {
932
// This is the first pass. We need to figure out if anything is still
933
// suspended in the rendered set.
991
- const renderedChildren = workInProgress.child;
934
+
935
// If new content unsuspended, but there's still some content that
936
// didn't. Then we need to do a second pass that forces everything
937
// to keep showing their fallbacks.
@@ -996,47 +939,93 @@ function completeWork(
939
// We might be suspended if something in this render pass suspended, or
940
// something in the previous committed pass suspended. Otherwise,
941
// there's no chance so we can skip the expensive call to
999
- // hasSuspendedChildrenAndNewContent.
942
+ // findFirstSuspended.
943
let cannotBeSuspended =
944
renderHasNotSuspendedYet() &&
945
(current === null || (current.effectTag & DidCapture) === NoEffect);
1003
- let needsRerender =
1004
- !cannotBeSuspended &&
1005
- hasSuspendedChildrenAndNewContent(workInProgress, renderedChildren);
1006
- if (needsRerender) {
1007
- // Rerender the whole list, but this time, we'll force fallbacks
1008
- // to stay in place.
1009
- // Reset the effect list before doing the second pass since that's now invalid.
1010
- workInProgress.firstEffect = workInProgress.lastEffect = null;
1011
- // Reset the child fibers to their original state.
1012
- resetChildFibers(workInProgress, renderExpirationTime);
946
+ if (!cannotBeSuspended) {
947
+ let row = workInProgress.child;
948
+ while (row !== null) {
949
+ let suspended = findFirstSuspended(row);
950
+ if (suspended !== null) {
951
+ didSuspendAlready = true;
952
+ workInProgress.effectTag |= DidCapture;
953
+ cutOffTailIfNeeded(renderState, false);
954
1014
- // Set up the Suspense Context to force suspense and immediately
1015
- // rerender the children.
1016
- pushSuspenseContext(
1017
- workInProgress,
1018
- setShallowSuspenseContext(
1019
- suspenseStackCursor.current,
1020
- ForceSuspenseFallback,
1021
- ),
1022
- );
1023
- return workInProgress.child;
955
+ // If this is a newly suspended tree, it might not get committed as
956
+ // part of the second pass. In that case nothing will subscribe to
957
+ // its thennables. Instead, we'll transfer its thennables to the
958
+ // SuspenseList so that it can retry if they resolve.
959
+ // There might be multiple of these in the list but since we're
960
+ // going to wait for all of them anyway, it doesn't really matter
961
+ // which ones gets to ping. In theory we could get clever and keep
962
+ // track of how many dependencies remain but it gets tricky because
963
+ // in the meantime, we can add/remove/change items and dependencies.
964
+ // We might bail out of the loop before finding any but that
965
+ // doesn't matter since that means that the other boundaries that
966
+ // we did find already has their listeners attached.
967
+ let newThennables = suspended.updateQueue;
968
+ if (newThennables !== null) {
969
+ workInProgress.updateQueue = newThennables;
970
+ workInProgress.effectTag |= Update;
971
+ }
972
+
973
+ // Rerender the whole list, but this time, we'll force fallbacks
974
+ // to stay in place.
975
+ // Reset the effect list before doing the second pass since that's now invalid.
976
+ workInProgress.firstEffect = workInProgress.lastEffect = null;
977
+ // Reset the child fibers to their original state.
978
+ resetChildFibers(workInProgress, renderExpirationTime);
979
+
980
+ // Set up the Suspense Context to force suspense and immediately
981
+ // rerender the children.
982
+ pushSuspenseContext(
983
+ workInProgress,
984
+ setShallowSuspenseContext(
985
+ suspenseStackCursor.current,
986
+ ForceSuspenseFallback,
987
+ ),
988
+ );
989
+ return workInProgress.child;
990
+ }
991
+ row = row.sibling;
992
+ }
993
}
1025
- // hasSuspendedChildrenAndNewContent could've set didSuspendAlready
1026
- didSuspendAlready =
1027
- (workInProgress.effectTag & DidCapture) !== NoEffect;
1028
- }
1029
- if (didSuspendAlready) {
994
+ } else {
995
cutOffTailIfNeeded(renderState, false);
996
}
997
// Next we're going to render the tail.
998
} else {
999
// Append the rendered row to the child list.
1000
if (!didSuspendAlready) {
1036
- if (isShowingAnyFallbacks(renderedTail)) {
1001
+ let suspended = findFirstSuspended(renderedTail);
1002
+ if (suspended !== null) {
1003
workInProgress.effectTag |= DidCapture;
1004
didSuspendAlready = true;
1005
cutOffTailIfNeeded(renderState, true);
1006
+ // This might have been modified.
1007
+ if (
1008
+ renderState.tail === null &&
1009
+ renderState.tailMode === 'hidden'
1010
+ ) {
1011
+ // We need to delete the row we just rendered.
1012
+ // Ensure we transfer the update queue to the parent.
1013
+ let newThennables = suspended.updateQueue;
1014
+ if (newThennables !== null) {
1015
+ workInProgress.updateQueue = newThennables;
1016
+ workInProgress.effectTag |= Update;
1017
+ }
1018
+ // Reset the effect list to what it w as before we rendered this
1019
+ // child. The nested children have already appended themselves.
1020
+ let lastEffect = (workInProgress.lastEffect =
1021
+ renderState.lastEffect);
1022
+ // Remove any effects that were appended after this point.
1023
+ if (lastEffect !== null) {
1024
+ lastEffect.nextEffect = null;
1025
+ }
1026
+ // We're done.
1027
+ return null;
1028
+ }
1029
} else if (
1030
now() > renderState.tailExpiration &&
1031
renderExpirationTime > Never
@@ -1093,6 +1082,7 @@ function completeWork(
1082
let next = renderState.tail;
1083
renderState.rendering = next;
1084
renderState.tail = next.sibling;
1085
+ renderState.lastEffect = workInProgress.lastEffect;
1086
next.sibling = null;
1087
1088
// Restore the context.
packages/react-reconciler/src/ReactFiberSuspenseComponent.js
+21
-7
@@ -8,13 +8,14 @@
8
*/
9
10
import type {Fiber} from './ReactFiber';
11
-import {SuspenseComponent} from 'shared/ReactWorkTags';
11
+import {SuspenseComponent, SuspenseListComponent} from 'shared/ReactWorkTags';
12
+import {NoEffect, DidCapture} from 'shared/ReactSideEffectTags';
13
14
// TODO: This is now an empty object. Should we switch this to a boolean?
15
// Alternatively we can make this use an effect tag similar to SuspenseList.
16
export type SuspenseState = {||};
17
17
-export type SuspenseListTailMode = 'collapsed' | void;
18
+export type SuspenseListTailMode = 'collapsed' | 'hidden' | void;
19
20
export type SuspenseListRenderState = {|
21
isBackwards: boolean,
@@ -28,6 +29,9 @@ export type SuspenseListRenderState = {|
29
tailExpiration: number,
30
// Tail insertions setting.
31
tailMode: SuspenseListTailMode,
32
+ // Last Effect before we rendered the "rendering" item.
33
+ // Used to remove new effects added by the rendered item.
34
+ lastEffect: null | Fiber,
35
|};
36
37
export function shouldCaptureSuspense(
@@ -58,13 +62,23 @@ export function shouldCaptureSuspense(
62
return true;
63
}
64
61
-export function isShowingAnyFallbacks(row: Fiber): boolean {
65
+export function findFirstSuspended(row: Fiber): null | Fiber {
66
let node = row;
67
while (node !== null) {
68
if (node.tag === SuspenseComponent) {
69
const state: SuspenseState | null = node.memoizedState;
70
if (state !== null) {
67
- return true;
71
+ return node;
72
+ }
73
+ } else if (
74
+ node.tag === SuspenseListComponent &&
75
+ // revealOrder undefined can't be trusted because it don't
76
+ // keep track of whether it suspended or not.
77
+ node.memoizedProps.revealOrder !== undefined
78
+ ) {
79
+ let didSuspend = (node.effectTag & DidCapture) !== NoEffect;
80
+ if (didSuspend) {
81
+ return node;
82
}
83
} else if (node.child !== null) {
84
node.child.return = node;
@@ -72,16 +86,16 @@ export function isShowingAnyFallbacks(row: Fiber): boolean {
86
continue;
87
}
88
if (node === row) {
75
- return false;
89
+ return null;
90
}
91
while (node.sibling === null) {
92
if (node.return === null || node.return === row) {
79
- return false;
93
+ return null;
94
}
95
node = node.return;
96
}
97
node.sibling.return = node.return;
98
node = node.sibling;
99
}
86
- return false;
100
+ return null;
101
}
packages/react-reconciler/src/__tests__/ReactSuspenseList-test.internal.js
+98
-5
@@ -640,6 +640,9 @@ describe('ReactSuspenseList', () => {
640
'Loading B',
641
'Suspend! [C]',
642
'Loading C',
643
+ 'A',
644
+ 'Loading B',
645
+ 'Loading C',
646
]);
647
648
// This will suspend, since the boundaries are avoided. Give them
@@ -859,6 +862,10 @@ describe('ReactSuspenseList', () => {
862
'Suspend! [C]',
863
'Loading C',
864
'D',
865
+ 'Loading A',
866
+ 'B',
867
+ 'Loading C',
868
+ 'D',
869
'Loading E',
870
'Loading F',
871
]);
@@ -1013,6 +1020,16 @@ describe('ReactSuspenseList', () => {
1020
'E',
1021
'Suspend! [F]',
1022
'Loading F',
1023
+ 'Suspend! [A]',
1024
+ 'Loading A',
1025
+ 'Suspend! [B]',
1026
+ 'Loading B',
1027
+ 'C',
1028
+ 'Suspend! [D]',
1029
+ 'Loading D',
1030
+ 'E',
1031
+ 'Suspend! [F]',
1032
+ 'Loading F',
1033
]);
1034
1035
// This will suspend, since the boundaries are avoided. Give them
@@ -1251,7 +1268,7 @@ describe('ReactSuspenseList', () => {
1268
1269
expect(() => Scheduler.unstable_flushAll()).toWarnDev([
1270
'Warning: "collapse" is not a supported value for tail on ' +
1254
- '<SuspenseList />. Did you mean "collapsed"?' +
1271
+ '<SuspenseList />. Did you mean "collapsed" or "hidden"?' +
1272
'\n in SuspenseList (at **)' +
1273
'\n in Foo (at **)',
1274
]);
@@ -1392,6 +1409,10 @@ describe('ReactSuspenseList', () => {
1409
'Suspend! [C]',
1410
'Loading C',
1411
'D',
1412
+ 'A',
1413
+ 'Loading B',
1414
+ 'Loading C',
1415
+ 'D',
1416
'Loading E',
1417
]);
1418
@@ -1516,6 +1537,10 @@ describe('ReactSuspenseList', () => {
1537
'Suspend! [E]',
1538
'Loading E',
1539
'F',
1540
+ 'C',
1541
+ 'Loading D',
1542
+ 'Loading E',
1543
+ 'F',
1544
'Loading B',
1545
]);
1546
@@ -1530,15 +1555,15 @@ describe('ReactSuspenseList', () => {
1555
</Fragment>,
1556
);
1557
1533
- await E.resolve();
1558
+ await D.resolve();
1559
1535
- expect(Scheduler).toFlushAndYield(['Suspend! [D]', 'E']);
1560
+ expect(Scheduler).toFlushAndYield(['D', 'Suspend! [E]']);
1561
1562
// Incremental loading is suspended.
1563
jest.advanceTimersByTime(500);
1564
1540
- // Even though E is unsuspended, it's still in loading state because
1541
- // it is blocked by D.
1565
+ // Even though D is unsuspended, it's still in loading state because
1566
+ // it is blocked by E.
1567
expect(ReactNoop).toMatchRenderedOutput(
1568
<Fragment>
1569
<span>Loading B</span>
@@ -1650,6 +1675,11 @@ describe('ReactSuspenseList', () => {
1675
'Loading C',
1676
'Suspend! [D]',
1677
'Loading D',
1678
+ 'A',
1679
+ 'Loading B',
1680
+ 'Loading C',
1681
+ 'Suspend! [D]',
1682
+ 'Loading D',
1683
'Loading E',
1684
]);
1685
@@ -1733,4 +1763,67 @@ describe('ReactSuspenseList', () => {
1763
</Fragment>,
1764
);
1765
});
1766
+
1767
+ it('only shows no initial loading state "hidden" tail insertions', async () => {
1768
+ let A = createAsyncText('A');
1769
+ let B = createAsyncText('B');
1770
+ let C = createAsyncText('C');
1771
+
1772
+ function Foo() {
1773
+ return (
1774
+ <SuspenseList revealOrder="forwards" tail="hidden">
1775
+ <Suspense fallback={<Text text="Loading A" />}>
1776
+ <A />
1777
+ </Suspense>
1778
+ <Suspense fallback={<Text text="Loading B" />}>
1779
+ <B />
1780
+ </Suspense>
1781
+ <Suspense fallback={<Text text="Loading C" />}>
1782
+ <C />
1783
+ </Suspense>
1784
+ </SuspenseList>
1785
+ );
1786
+ }
1787
+
1788
+ ReactNoop.render(<Foo />);
1789
+
1790
+ expect(Scheduler).toFlushAndYield(['Suspend! [A]', 'Loading A']);
1791
+
1792
+ expect(ReactNoop).toMatchRenderedOutput(null);
1793
+
1794
+ await A.resolve();
1795
+
1796
+ expect(Scheduler).toFlushAndYield(['A', 'Suspend! [B]', 'Loading B']);
1797
+
1798
+ // Incremental loading is suspended.
1799
+ jest.advanceTimersByTime(500);
1800
+
1801
+ expect(ReactNoop).toMatchRenderedOutput(<span>A</span>);
1802
+
1803
+ await B.resolve();
1804
+
1805
+ expect(Scheduler).toFlushAndYield(['B', 'Suspend! [C]', 'Loading C']);
1806
+
1807
+ // Incremental loading is suspended.
1808
+ jest.advanceTimersByTime(500);
1809
+
1810
+ expect(ReactNoop).toMatchRenderedOutput(
1811
+ <Fragment>
1812
+ <span>A</span>
1813
+ <span>B</span>
1814
+ </Fragment>,
1815
+ );
1816
+
1817
+ await C.resolve();
1818
+
1819
+ expect(Scheduler).toFlushAndYield(['C']);
1820
+
1821
+ expect(ReactNoop).toMatchRenderedOutput(
1822
+ <Fragment>
1823
+ <span>A</span>
1824
+ <span>B</span>
1825
+ <span>C</span>
1826
+ </Fragment>,
1827
+ );
1828
+ });
1829
});