diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index ce94ab3a3fa3f..4358952c3523f 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -181,6 +181,7 @@ import { scheduleUpdateOnFiber, renderDidSuspendDelayIfPossible, markUnprocessedUpdateTime, + getWorkInProgressRoot, } from './ReactFiberWorkLoop'; const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner; @@ -1605,23 +1606,16 @@ function getRemainingWorkInPrimaryTree( if ((workInProgress.mode & BlockingMode) !== NoMode) { // The highest priority remaining work is part of this render. Since we only // keep track of the highest level, we don't know if there's a lower - // priority level scheduled. As a compromise, we'll render at a really low - // priority that includes all the updates. + // priority level scheduled. As a compromise, we'll render at the lowest + // known level in the entire tree, since that will include everything. // TODO: If expirationTime were a bitmask where each bit represents a // separate task thread, this would be: currentChildBits & ~renderBits - // TODO: We used to track the lowest pending level for the whole root, but - // we removed it to simplify the implementation. It might be worth adding - // it back for this use case, to avoid redundant passes. - if (renderExpirationTime > ContinuousHydration) { - // First we try ContinuousHydration, so that we don't get grouped - // with Idle. - return ContinuousHydration; - } else if (renderExpirationTime > Idle) { - // Then we'll try Idle. - return Idle; - } else { - // Already at lowest possible update level. - return NoWork; + const root = getWorkInProgressRoot(); + if (root !== null) { + const lastPendingTime = root.lastPendingTime; + if (lastPendingTime < renderExpirationTime) { + return lastPendingTime; + } } } // In legacy mode, there's no work left. diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js index 5cecd89b00a83..e24008eb54d2d 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js @@ -734,22 +734,12 @@ describe('ReactHooksWithNoopRenderer', () => { root.render(); setLabel('B'); }); - expect(Scheduler).toHaveYielded([ - 'Suspend!', - // TODO: This second attempt is because React isn't sure if there's - // another update at a lower priority. Ideally we wouldn't need it. - 'Suspend!', - ]); + expect(Scheduler).toHaveYielded(['Suspend!']); expect(root).toMatchRenderedOutput(); // Rendering again should suspend again. root.render(); - expect(Scheduler).toFlushAndYield([ - 'Suspend!', - // TODO: This second attempt is because React isn't sure if there's - // another update at a lower priority. Ideally we wouldn't need it. - 'Suspend!', - ]); + expect(Scheduler).toFlushAndYield(['Suspend!']); // Flip the signal back to "cancel" the update. However, the update to // label should still proceed. It shouldn't have been dropped. diff --git a/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js index 9029a911cc493..0a1d209d41f52 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js @@ -956,14 +956,7 @@ describe('ReactSuspense', () => { // Update that suspends instance.setState({step: 2}); - expect(Scheduler).toFlushAndYield([ - 'Suspend! [Step: 2]', - 'Loading...', - // TODO: This second attempt is because React isn't sure if there's - // another update at a lower priority. Ideally we wouldn't need it. - 'Suspend! [Step: 2]', - 'Loading...', - ]); + expect(Scheduler).toFlushAndYield(['Suspend! [Step: 2]', 'Loading...']); jest.advanceTimersByTime(500); expect(root).toMatchRenderedOutput('Loading...'); diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js index c5e1eb7a12411..1fd71e3cfc5ef 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js @@ -3091,15 +3091,6 @@ describe('ReactSuspenseWithNoopRenderer', () => { // First we attempt the high pri update. It suspends. 'Suspend! [B]', 'Loading...', - - // Then retry at a lower priority level. - // NOTE: The current implementation doesn't know which level to attempt, - // so it picks ContinuousHydration, which is one level higher than Idle. - // Since it doesn't include the Idle update, it will suspend again and - // reschedule to try at Idle. If we refactor expiration time to be a - // bitmask, we shouldn't need this heuristic. - 'Suspend! [B]', - 'Loading...', ]); // Commit the placeholder to unblock the Idle update. @@ -3166,20 +3157,13 @@ describe('ReactSuspenseWithNoopRenderer', () => { await ReactNoop.act(async () => { setText('C'); }); - expect(Scheduler).toHaveYielded([ - 'Suspend! [C]', - 'Loading...', - - 'Suspend! [C]', - 'Loading...', - ]); + expect(Scheduler).toHaveYielded(['Suspend! [C]', 'Loading...']); // Commit. This will insert a fragment fiber to wrap around the component // that triggered the update. await ReactNoop.act(async () => { await advanceTimers(250); }); - expect(Scheduler).toHaveYielded(['Suspend! [C]']); // The fragment fiber is part of the current tree, but the setState return // path still follows the alternate path. That means the fragment fiber is // not part of the return path.