Reset lastEffect when resuming SuspenseList (#18412)
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.
Sebastian Markbåge committed
Mar 29, 2020 at 10:13 UTC
689d27586e30f94be1b7a7bb634b82633c2baf42
2 files changed
+186
packages/react-reconciler/src/ReactFiberBeginWork.js
+1
@@ -3153,6 +3153,7 @@ function beginWork(
3153
// update in the past but didn't complete it.
3154
renderState.rendering = null;
3155
renderState.tail = null;
3156
+ renderState.lastEffect = null;
3157
}
3158
pushSuspenseContext(workInProgress, suspenseStackCursor.current);
3159
packages/react-reconciler/src/__tests__/ReactSuspenseList-test.internal.js
+185
@@ -2300,4 +2300,189 @@ describe('ReactSuspenseList', () => {
2300
// remount.
2301
expect(previousInst).toBe(setAsyncB);
2302
});
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
+ ReactNoop.act(() => {
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
+ });
2440
+
2441
+ jest.runAllTimers();
2442
+
2443
+ expect(Scheduler).toHaveYielded(
2444
+ __DEV__
2445
+ ? [
2446
+ // First attempt at high pri.
2447
+ 'Suspend! [A]',
2448
+ 'Loading A',
2449
+ // Re-render at forced.
2450
+ 'Suspend! [A]',
2451
+ 'Loading A',
2452
+ // We auto-commit this on DEV.
2453
+ // Try again on low-pri.
2454
+ 'Suspend! [A]',
2455
+ 'Loading A',
2456
+ ]
2457
+ : [
2458
+ // First attempt at high pri.
2459
+ 'Suspend! [A]',
2460
+ 'Loading A',
2461
+ // Re-render at forced.
2462
+ 'Suspend! [A]',
2463
+ 'Loading A',
2464
+ // We didn't commit so retry at low-pri.
2465
+ 'Suspend! [A]',
2466
+ 'Loading A',
2467
+ // Re-render at forced.
2468
+ 'Suspend! [A]',
2469
+ 'Loading A',
2470
+ ],
2471
+ );
2472
+
2473
+ expect(ReactNoop).toMatchRenderedOutput(<span>Loading A</span>);
2474
+
2475
+ await AsyncA.resolve();
2476
+
2477
+ expect(Scheduler).toFlushAndYield(['A', 'B', 'C', 'D']);
2478
+
2479
+ expect(ReactNoop).toMatchRenderedOutput(
2480
+ <>
2481
+ <span>A</span>
2482
+ <span>B</span>
2483
+ <span>C</span>
2484
+ <span>D</span>
2485
+ </>,
2486
+ );
2487
+ });
2488
});