Skip to content

Commit 00c5575

Browse files
committed
Reset lastEffect when resuming SuspenseList
We store an effect pointer so we can backtrack in the effect list in some cases. This is a stateful variable. If we interrupt a render we need to reset it. This field was added after the optimization was added and I didn't remember to reset it here. Otherwise we end up not resetting the firstEffect so it points to a stale list. As a result children don't end up inserted like we think they were. Then we try to remove them it errors. It would be nicer to just get rid of the effect list and use the tree for effects instead. Maybe we still need something for deletions tho.
1 parent 35a2f74 commit 00c5575

File tree

2 files changed

+153
-0
lines changed

2 files changed

+153
-0
lines changed

packages/react-reconciler/src/ReactFiberBeginWork.js

+1
Original file line numberDiff line numberDiff line change
@@ -3153,6 +3153,7 @@ function beginWork(
31533153
// update in the past but didn't complete it.
31543154
renderState.rendering = null;
31553155
renderState.tail = null;
3156+
renderState.lastEffect = null;
31563157
}
31573158
pushSuspenseContext(workInProgress, suspenseStackCursor.current);
31583159

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

+152
Original file line numberDiff line numberDiff line change
@@ -2300,4 +2300,156 @@ describe('ReactSuspenseList', () => {
23002300
// remount.
23012301
expect(previousInst).toBe(setAsyncB);
23022302
});
2303+
2304+
it('is able to re-suspend the last rows during an update with hidden', async () => {
2305+
let AsyncB = createAsyncText('B');
2306+
2307+
let setAsyncB;
2308+
2309+
function B() {
2310+
let [shouldBeAsync, setAsync] = React.useState(false);
2311+
setAsyncB = setAsync;
2312+
2313+
return shouldBeAsync ? (
2314+
<Suspense fallback={<Text text="Loading B" />}>
2315+
<AsyncB />
2316+
</Suspense>
2317+
) : (
2318+
<Text text="Sync B" />
2319+
);
2320+
}
2321+
2322+
function Foo({updateList}) {
2323+
return (
2324+
<SuspenseList revealOrder="forwards" tail="hidden">
2325+
<Suspense key="A" fallback={<Text text="Loading A" />}>
2326+
<Text text="A" />
2327+
</Suspense>
2328+
<B key="B" updateList={updateList} />
2329+
</SuspenseList>
2330+
);
2331+
}
2332+
2333+
ReactNoop.render(<Foo />);
2334+
2335+
expect(Scheduler).toFlushAndYield(['A', 'Sync B']);
2336+
2337+
expect(ReactNoop).toMatchRenderedOutput(
2338+
<>
2339+
<span>A</span>
2340+
<span>Sync B</span>
2341+
</>,
2342+
);
2343+
2344+
let previousInst = setAsyncB;
2345+
2346+
// During an update we suspend on B.
2347+
ReactNoop.act(() => setAsyncB(true));
2348+
2349+
expect(Scheduler).toHaveYielded([
2350+
'Suspend! [B]',
2351+
'Loading B',
2352+
// The second pass is the "force hide" pass
2353+
'Loading B',
2354+
]);
2355+
2356+
expect(ReactNoop).toMatchRenderedOutput(
2357+
<>
2358+
<span>A</span>
2359+
<span>Loading B</span>
2360+
</>,
2361+
);
2362+
2363+
// Before we resolve we'll rerender the whole list.
2364+
// This should leave the tree intact.
2365+
ReactNoop.act(() => ReactNoop.render(<Foo updateList={true} />));
2366+
2367+
expect(Scheduler).toHaveYielded(['A', 'Suspend! [B]', 'Loading B']);
2368+
2369+
expect(ReactNoop).toMatchRenderedOutput(
2370+
<>
2371+
<span>A</span>
2372+
<span>Loading B</span>
2373+
</>,
2374+
);
2375+
2376+
await AsyncB.resolve();
2377+
2378+
expect(Scheduler).toFlushAndYield(['B']);
2379+
2380+
expect(ReactNoop).toMatchRenderedOutput(
2381+
<>
2382+
<span>A</span>
2383+
<span>B</span>
2384+
</>,
2385+
);
2386+
2387+
// This should be the same instance. I.e. it didn't
2388+
// remount.
2389+
expect(previousInst).toBe(setAsyncB);
2390+
});
2391+
2392+
it('is able to interrupt a partially rendered tree and continue later', async () => {
2393+
let AsyncA = createAsyncText('A');
2394+
2395+
let updateLowPri;
2396+
let updateHighPri;
2397+
2398+
function Bar() {
2399+
let [highPriState, setHighPriState] = React.useState(false);
2400+
updateHighPri = setHighPriState;
2401+
return highPriState ? <AsyncA /> : null;
2402+
}
2403+
2404+
function Foo() {
2405+
let [lowPriState, setLowPriState] = React.useState(false);
2406+
updateLowPri = setLowPriState;
2407+
return (
2408+
<SuspenseList revealOrder="forwards" tail="hidden">
2409+
<Suspense key="A" fallback={<Text text="Loading A" />}>
2410+
<Bar />
2411+
</Suspense>
2412+
{lowPriState ? <Text text="B" /> : null}
2413+
{lowPriState ? <Text text="C" /> : null}
2414+
{lowPriState ? <Text text="D" /> : null}
2415+
</SuspenseList>
2416+
);
2417+
}
2418+
2419+
ReactNoop.render(<Foo />);
2420+
2421+
expect(Scheduler).toFlushAndYield([]);
2422+
2423+
expect(ReactNoop).toMatchRenderedOutput(null);
2424+
2425+
await ReactNoop.act(async () => {
2426+
// Add a few items at the end.
2427+
updateLowPri(true);
2428+
2429+
// Flush partially through.
2430+
expect(Scheduler).toFlushAndYieldThrough(['B', 'C']);
2431+
2432+
// Schedule another update at higher priority.
2433+
Scheduler.unstable_runWithPriority(
2434+
Scheduler.unstable_UserBlockingPriority,
2435+
() => updateHighPri(true),
2436+
);
2437+
2438+
// That will intercept the previous render.
2439+
expect(Scheduler).toFlushAndYield([
2440+
'Suspend! [A]',
2441+
'Loading A',
2442+
'Suspend! [A]',
2443+
'Loading A',
2444+
'Suspend! [A]',
2445+
'Loading A',
2446+
]);
2447+
2448+
expect(ReactNoop).toMatchRenderedOutput(<span>Loading A</span>);
2449+
2450+
await AsyncA.resolve();
2451+
2452+
expect(Scheduler).toFlushAndYield(['A', 'B', 'C', 'D']);
2453+
});
2454+
});
23032455
});

0 commit comments

Comments
 (0)