diff --git a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js index d089b25ffa..5bdca5d43f 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js @@ -3216,7 +3216,7 @@ body { ); }); - it('can interrupt a suspended commit with a new transition', async () => { + it('can start a new suspended commit after a previous one finishes', async () => { function App({children}) { return ( @@ -3225,66 +3225,81 @@ body { ); } const root = ReactDOMClient.createRoot(document); - root.render((empty)); - - // Start a transition to "A" + root.render(); React.startTransition(() => { root.render( - A - + hello + , ); }); await waitForAll([]); - - // "A" hasn't loaded yet, so we remain on the initial UI. Its preload - // has been inserted into the head, though. expect(getMeaningfulChildren(document)).toEqual( - + - (empty) + , ); - // Interrupt the "A" transition with a new one, "B" React.startTransition(() => { root.render( - B - + hello2 + {null} + , ); }); await waitForAll([]); - - // Still on the initial UI because "B" hasn't loaded, but its preload - // is now in the head, too. expect(getMeaningfulChildren(document)).toEqual( - - + - (empty) + , ); - // Finish loading loadPreloads(); loadStylesheets(); - assertLog(['load preload: A', 'load preload: B', 'load stylesheet: B']); - // The "B" transition has finished. + assertLog(['load preload: foo', 'load stylesheet: foo']); expect(getMeaningfulChildren(document)).toEqual( - - - + + - B + hello + , + ); + + // The second update should process now + await waitForAll([]); + expect(getMeaningfulChildren(document)).toEqual( + + + + + + + hello + , + ); + loadPreloads(); + loadStylesheets(); + assertLog(['load preload: bar', 'load stylesheet: bar']); + expect(getMeaningfulChildren(document)).toEqual( + + + + + + + + hello2 , ); }); diff --git a/packages/react-reconciler/src/ReactFiberRootScheduler.js b/packages/react-reconciler/src/ReactFiberRootScheduler.js index be14c46950..5068194aa2 100644 --- a/packages/react-reconciler/src/ReactFiberRootScheduler.js +++ b/packages/react-reconciler/src/ReactFiberRootScheduler.js @@ -18,6 +18,7 @@ import { SyncLane, getHighestPriorityLane, getNextLanes, + includesOnlyNonUrgentLanes, includesSyncLane, markStarvedLanesAsExpired, } from './ReactFiberLane'; @@ -291,16 +292,14 @@ function scheduleTaskForRootDuringMicrotask( const existingCallbackNode = root.callbackNode; if ( - // Check if there's nothing to work on nextLanes === NoLanes || // If this root is currently suspended and waiting for data to resolve, don't // schedule a task to render it. We'll either wait for a ping, or wait to // receive an update. - // - // Suspended render phase - (root === workInProgressRoot && isWorkLoopSuspendedOnData()) || - // Suspended commit phase - root.cancelPendingCommit !== null + (isWorkLoopSuspendedOnData() && root === workInProgressRoot) || + // We should only interrupt a pending commit if the new update + // is urgent. + (root.cancelPendingCommit !== null && includesOnlyNonUrgentLanes(nextLanes)) ) { // Fast path: There's nothing to work on. if (existingCallbackNode !== null) { diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 71e6f4feab..55c30ae81a 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -725,11 +725,8 @@ export function scheduleUpdateOnFiber( // Check if the work loop is currently suspended and waiting for data to // finish loading. if ( - // Suspended render phase - (root === workInProgressRoot && - workInProgressSuspendedReason === SuspendedOnData) || - // Suspended commit phase - root.cancelPendingCommit !== null + workInProgressSuspendedReason === SuspendedOnData && + root === workInProgressRoot ) { // The incoming update might unblock the current render. Interrupt the // current attempt and restart from the top. diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseyCommitPhase-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseyCommitPhase-test.js index 2a558ba93a..bbd2ae8c2e 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseyCommitPhase-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseyCommitPhase-test.js @@ -137,7 +137,7 @@ describe('ReactSuspenseyCommitPhase', () => { // Nothing showing yet. expect(root).toMatchRenderedOutput(null); - // If there's an update, it should interrupt the suspended commit. + // If there's an urgent update, it should interrupt the suspended commit. await act(() => { root.render(); }); @@ -145,7 +145,7 @@ describe('ReactSuspenseyCommitPhase', () => { expect(root).toMatchRenderedOutput('Something else'); }); - test('a transition update interrupts a suspended commit', async () => { + test('a non-urgent update does not interrupt a suspended commit', async () => { const root = ReactNoop.createRoot(); // Mount an image. This transition will suspend because it's not inside a @@ -159,12 +159,26 @@ describe('ReactSuspenseyCommitPhase', () => { // Nothing showing yet. expect(root).toMatchRenderedOutput(null); - // If there's an update, it should interrupt the suspended commit. + // If there's another transition update, it should not interrupt the + // suspended commit. await act(() => { startTransition(() => { root.render(); }); }); + // Still suspended. + expect(root).toMatchRenderedOutput(null); + + await act(() => { + // Resolving the image should result in an immediate, synchronous commit. + resolveSuspenseyThing('A'); + expect(root).toMatchRenderedOutput(); + }); + // Then the second transition is unblocked. + // TODO: Right now the only way to unsuspend a commit early is to proceed + // with the commit even if everything isn't ready. Maybe there should also + // be a way to abort a commit so that it can be interrupted by + // another transition. assertLog(['Something else']); expect(root).toMatchRenderedOutput('Something else'); });