From b7a0583b8f875e7723f7395468be87455aa600ac Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 17 Apr 2020 15:44:38 -0700 Subject: [PATCH 1/2] Failing test for #18657 --- .../ReactSuspenseWithNoopRenderer-test.js | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js index cfe3d244b5103..5f5469bcd9045 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js @@ -3869,4 +3869,53 @@ describe('ReactSuspenseWithNoopRenderer', () => { expect(root).toMatchRenderedOutput(); }); }); + + it('regression: #18657', async () => { + const {useState} = React; + + let setText; + function App() { + const [text, _setText] = useState('A'); + setText = _setText; + return ; + } + + const root = ReactNoop.createRoot(); + await ReactNoop.act(async () => { + await resolveText('A'); + root.render( + }> + + , + ); + }); + expect(Scheduler).toHaveYielded(['A']); + expect(root).toMatchRenderedOutput(); + + await ReactNoop.act(async () => { + setText('B'); + Scheduler.unstable_runWithPriority( + Scheduler.unstable_IdlePriority, + () => { + setText('B'); + }, + ); + // Suspend the first update. The second update doesn't run because it has + // Idle priority. + expect(Scheduler).toFlushAndYield(['Suspend! [B]', 'Loading...']); + + // Commit the fallback. Now we'll try working on Idle. + jest.runAllTimers(); + + // It also suspends. + expect(Scheduler).toFlushAndYield(['Suspend! [B]']); + }); + + await ReactNoop.act(async () => { + setText('B'); + await resolveText('B'); + }); + expect(Scheduler).toHaveYielded(['Promise resolved [B]', 'B']); + expect(root).toMatchRenderedOutput(); + }); }); From c673395cf7407b1d05527891c9f5eb885abee3e5 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 17 Apr 2020 15:52:01 -0700 Subject: [PATCH 2/2] Remove incorrect priority check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I think this was just poor factoring on my part in #18411. Honestly it doesn't make much sense to me, but my best guess is that I must have thought that when `baseTime > currentChildExpirationTime`, the function would fall through to the `currentChildExpirationTime < renderExpirationTime` branch below. Really I think just made an oopsie. Regardless, this logic is galaxy brainéd. A goal of the Lanes refactor I'm working on is to make these types of checks -- is there remaining work in this tree? -- a lot easier to think about. Hopefully. --- packages/react-reconciler/src/ReactFiberBeginWork.new.js | 6 +----- packages/react-reconciler/src/ReactFiberBeginWork.old.js | 6 +----- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index 338191f23666c..b88f17c1cb168 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -1681,11 +1681,7 @@ function getRemainingWorkInPrimaryTree( // This boundary already timed out. Check if this render includes the level // that previously suspended. const baseTime = currentSuspenseState.baseTime; - if ( - baseTime !== NoWork && - baseTime < renderExpirationTime && - baseTime > currentChildExpirationTime - ) { + if (baseTime !== NoWork && baseTime < renderExpirationTime) { // There's pending work at a lower level that might now be unblocked. return baseTime; } diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.old.js b/packages/react-reconciler/src/ReactFiberBeginWork.old.js index 4fde99d80c715..6fbfd5cd97077 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.old.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.old.js @@ -1681,11 +1681,7 @@ function getRemainingWorkInPrimaryTree( // This boundary already timed out. Check if this render includes the level // that previously suspended. const baseTime = currentSuspenseState.baseTime; - if ( - baseTime !== NoWork && - baseTime < renderExpirationTime && - baseTime > currentChildExpirationTime - ) { + if (baseTime !== NoWork && baseTime < renderExpirationTime) { // There's pending work at a lower level that might now be unblocked. return baseTime; }