Skip to content

Commit 6cd365c

Browse files
authored
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.
1 parent 048879e commit 6cd365c

File tree

2 files changed

+92
-3
lines changed

2 files changed

+92
-3
lines changed

packages/react-reconciler/src/ReactFiberCompleteWork.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1098,10 +1098,11 @@ function completeWork(
10981098
// This might have been modified.
10991099
if (
11001100
renderState.tail === null &&
1101-
renderState.tailMode === 'hidden'
1101+
renderState.tailMode === 'hidden' &&
1102+
!renderedTail.alternate
11021103
) {
11031104
// 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
11051106
// child. The nested children have already appended themselves.
11061107
let lastEffect = (workInProgress.lastEffect =
11071108
renderState.lastEffect);

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

Lines changed: 89 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2023,7 +2023,7 @@ describe('ReactSuspenseList', () => {
20232023
);
20242024
});
20252025

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 () => {
20272027
let B = createAsyncText('B');
20282028

20292029
function Foo({showB}) {
@@ -2135,4 +2135,92 @@ describe('ReactSuspenseList', () => {
21352135
</div>,
21362136
);
21372137
});
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+
});
21382226
});

0 commit comments

Comments
 (0)