From b66384152c75ebdc4fb7e05c740989154c87425b Mon Sep 17 00:00:00 2001 From: Tianyu Yao Date: Tue, 14 Nov 2023 15:59:49 -0800 Subject: [PATCH 1/3] Add a regression test for infinite suspense --- .../ReactSuspenseWithNoopRenderer-test.js | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js index d93c8bcf606ab..131f37da0b47c 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js @@ -4008,4 +4008,76 @@ describe('ReactSuspenseWithNoopRenderer', () => { ); }, ); + + // @gate enableLegacyCache + it('recurring updates in siblings should not block expensive content in suspense boundary from committing', async () => { + const {useState} = React; + + let setText; + function UpdatingText() { + const [text, _setText] = useState('1'); + setText = _setText; + return ; + } + + function ExpensiveText({text, ms}) { + Scheduler.log(text); + Scheduler.unstable_advanceTime(ms); + return ; + } + + function App() { + return ( + <> + + }> + + + + + + + ); + } + + const root = ReactNoop.createRoot(); + root.render(); + await waitForAll(['1', 'Suspend! [Async]', 'Loading...']); + expect(root).toMatchRenderedOutput( + <> + + + , + ); + + await resolveText('Async'); + expect(root).toMatchRenderedOutput( + <> + + + , + ); + + await waitFor(['Async', 'A', 'B']); + ReactNoop.expire(10000000); + await advanceTimers(10000000); + setText('2'); + await waitForPaint(['2']); + + await waitFor(['Async', 'A', 'B']); + ReactNoop.expire(10000000); + await advanceTimers(10000000); + setText('3'); + + // TODO: At this point we want "C" to commit + await waitForPaint(['3']); + await waitFor(['Async', 'A', 'B']); + + expect(root).toMatchRenderedOutput( + <> + + + , + ); + }); }); From 23e50e848f691f911c7568e0d426e198af2504ca Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Thu, 7 Dec 2023 14:46:38 -0500 Subject: [PATCH 2/3] Fix: enableRetryLaneExpiration This is another place we special-case Retry lanes to opt them out of expiration. The reason is we rely on time slicing to unwrap uncached promises (i.e. async functions during render). Since that ability is still experimental, and enableRetryLaneExpiration is Meta-only, we can remove the special case when enableRetryLaneExpiration is on, for now. --- packages/react-reconciler/src/ReactFiberLane.js | 4 +++- .../ReactSuspenseWithNoopRenderer-test.js | 17 +++++++++-------- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberLane.js b/packages/react-reconciler/src/ReactFiberLane.js index 905eda36a93e8..c110b29c250ef 100644 --- a/packages/react-reconciler/src/ReactFiberLane.js +++ b/packages/react-reconciler/src/ReactFiberLane.js @@ -423,7 +423,9 @@ export function markStarvedLanesAsExpired( // We exclude retry lanes because those must always be time sliced, in order // to unwrap uncached promises. // TODO: Write a test for this - let lanes = pendingLanes & ~RetryLanes; + let lanes = enableRetryLaneExpiration + ? pendingLanes + : pendingLanes & ~RetryLanes; while (lanes > 0) { const index = pickArbitraryLaneIndex(lanes); const lane = 1 << index; diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js index 131f37da0b47c..3c4167a099ee0 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js @@ -5,6 +5,7 @@ let Scheduler; let act; let waitFor; let waitForAll; +let unstable_waitForExpired; let assertLog; let waitForPaint; let Suspense; @@ -29,6 +30,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { waitFor = InternalTestUtils.waitFor; waitForAll = InternalTestUtils.waitForAll; waitForPaint = InternalTestUtils.waitForPaint; + unstable_waitForExpired = InternalTestUtils.unstable_waitForExpired; assertLog = InternalTestUtils.assertLog; getCacheForType = React.unstable_getCacheForType; @@ -4010,6 +4012,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { ); // @gate enableLegacyCache + // @gate enableRetryLaneExpiration it('recurring updates in siblings should not block expensive content in suspense boundary from committing', async () => { const {useState} = React; @@ -4064,19 +4067,17 @@ describe('ReactSuspenseWithNoopRenderer', () => { setText('2'); await waitForPaint(['2']); - await waitFor(['Async', 'A', 'B']); ReactNoop.expire(10000000); await advanceTimers(10000000); - setText('3'); - - // TODO: At this point we want "C" to commit - await waitForPaint(['3']); - await waitFor(['Async', 'A', 'B']); + await unstable_waitForExpired(['Async', 'A', 'B', 'C']); expect(root).toMatchRenderedOutput( <> - - + + + + + , ); }); From f04d17950b83fa45d2ade839d5773cbddc70aee1 Mon Sep 17 00:00:00 2001 From: Tianyu Yao Date: Thu, 7 Dec 2023 16:51:15 -0800 Subject: [PATCH 3/3] Make regression test fail on base --- .../ReactSuspenseWithNoopRenderer-test.js | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js index 3c4167a099ee0..77aae6ee53d9b 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js @@ -5,7 +5,7 @@ let Scheduler; let act; let waitFor; let waitForAll; -let unstable_waitForExpired; +let waitForMicrotasks; let assertLog; let waitForPaint; let Suspense; @@ -30,7 +30,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { waitFor = InternalTestUtils.waitFor; waitForAll = InternalTestUtils.waitForAll; waitForPaint = InternalTestUtils.waitForPaint; - unstable_waitForExpired = InternalTestUtils.unstable_waitForExpired; + waitForMicrotasks = InternalTestUtils.waitForMicrotasks; assertLog = InternalTestUtils.assertLog; getCacheForType = React.unstable_getCacheForType; @@ -4011,8 +4011,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { }, ); - // @gate enableLegacyCache - // @gate enableRetryLaneExpiration + // @gate enableLegacyCache && enableRetryLaneExpiration it('recurring updates in siblings should not block expensive content in suspense boundary from committing', async () => { const {useState} = React; @@ -4062,14 +4061,14 @@ describe('ReactSuspenseWithNoopRenderer', () => { ); await waitFor(['Async', 'A', 'B']); - ReactNoop.expire(10000000); - await advanceTimers(10000000); + ReactNoop.expire(100000); + await advanceTimers(100000); setText('2'); await waitForPaint(['2']); - ReactNoop.expire(10000000); - await advanceTimers(10000000); - await unstable_waitForExpired(['Async', 'A', 'B', 'C']); + await waitForMicrotasks(); + Scheduler.unstable_flushNumberOfYields(1); + assertLog(['Async', 'A', 'B', 'C']); expect(root).toMatchRenderedOutput( <>