Don't treat the last row in hidden as deleted if already mounted (#17206)
Already mounted rows that resuspend may be considered as part of a tail if they're at the end. However, for purposes of the tail="..." option they don't get deleted. We deal with that in cutOffTailIfNeeded. However, if they're also the first to suspend in the "hidden" case, we have a special case that deletes the actual rendered row. This needs to consider if that row was already mounted or things go wrong.
Sebastian Markbåge committed
Oct 29, 2019 at 13:38 UTC
6cd365cac69682f778e00238d8aa5c69730c13af
2 files changed
+92
-3
packages/react-reconciler/src/ReactFiberCompleteWork.js
+3
-2
@@ -1098,10 +1098,11 @@ function completeWork(
1098
// This might have been modified.
1099
if (
1100
renderState.tail === null &&
1101
- renderState.tailMode === 'hidden'
1101
+ renderState.tailMode === 'hidden' &&
1102
+ !renderedTail.alternate
1103
) {
1104
// We need to delete the row we just rendered.
1104
- // Reset the effect list to what it w as before we rendered this
1105
+ // Reset the effect list to what it was before we rendered this
1106
// child. The nested children have already appended themselves.
1107
let lastEffect = (workInProgress.lastEffect =
1108
renderState.lastEffect);
packages/react-reconciler/src/__tests__/ReactSuspenseList-test.internal.js
+89
-1
@@ -2023,7 +2023,7 @@ describe('ReactSuspenseList', () => {
2023
);
2024
});
2025
2026
- it('eventually resolves two nested forwards suspense list with a hidden tail', async () => {
2026
+ it('eventually resolves two nested forwards suspense lists with a hidden tail', async () => {
2027
let B = createAsyncText('B');
2028
2029
function Foo({showB}) {
@@ -2135,4 +2135,92 @@ describe('ReactSuspenseList', () => {
2135
</div>,
2136
);
2137
});
2138
+
2139
+ it('is able to re-suspend the last rows during an update with hidden', async () => {
2140
+ let AsyncB = createAsyncText('B');
2141
+
2142
+ let setAsyncB;
2143
+
2144
+ function B() {
2145
+ let [shouldBeAsync, setAsync] = React.useState(false);
2146
+ setAsyncB = setAsync;
2147
+
2148
+ return shouldBeAsync ? (
2149
+ <Suspense fallback={<Text text="Loading B" />}>
2150
+ <AsyncB />
2151
+ </Suspense>
2152
+ ) : (
2153
+ <Text text="Sync B" />
2154
+ );
2155
+ }
2156
+
2157
+ function Foo({updateList}) {
2158
+ return (
2159
+ <SuspenseList revealOrder="forwards" tail="hidden">
2160
+ <Suspense key="A" fallback={<Text text="Loading A" />}>
2161
+ <Text text="A" />
2162
+ </Suspense>
2163
+ <B key="B" updateList={updateList} />
2164
+ </SuspenseList>
2165
+ );
2166
+ }
2167
+
2168
+ ReactNoop.render(<Foo />);
2169
+
2170
+ expect(Scheduler).toFlushAndYield(['A', 'Sync B']);
2171
+
2172
+ expect(ReactNoop).toMatchRenderedOutput(
2173
+ <>
2174
+ <span>A</span>
2175
+ <span>Sync B</span>
2176
+ </>,
2177
+ );
2178
+
2179
+ let previousInst = setAsyncB;
2180
+
2181
+ // During an update we suspend on B.
2182
+ ReactNoop.act(() => setAsyncB(true));
2183
+
2184
+ expect(Scheduler).toHaveYielded([
2185
+ 'Suspend! [B]',
2186
+ 'Loading B',
2187
+ // The second pass is the "force hide" pass
2188
+ 'Loading B',
2189
+ ]);
2190
+
2191
+ expect(ReactNoop).toMatchRenderedOutput(
2192
+ <>
2193
+ <span>A</span>
2194
+ <span>Loading B</span>
2195
+ </>,
2196
+ );
2197
+
2198
+ // Before we resolve we'll rerender the whole list.
2199
+ // This should leave the tree intact.
2200
+ ReactNoop.act(() => ReactNoop.render(<Foo updateList={true} />));
2201
+
2202
+ expect(Scheduler).toHaveYielded(['A', 'Suspend! [B]', 'Loading B']);
2203
+
2204
+ expect(ReactNoop).toMatchRenderedOutput(
2205
+ <>
2206
+ <span>A</span>
2207
+ <span>Loading B</span>
2208
+ </>,
2209
+ );
2210
+
2211
+ await AsyncB.resolve();
2212
+
2213
+ expect(Scheduler).toFlushAndYield(['B']);
2214
+
2215
+ expect(ReactNoop).toMatchRenderedOutput(
2216
+ <>
2217
+ <span>A</span>
2218
+ <span>B</span>
2219
+ </>,
2220
+ );
2221
+
2222
+ // This should be the same instance. I.e. it didn't
2223
+ // remount.
2224
+ expect(previousInst).toBe(setAsyncB);
2225
+ });
2226
});