diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index a83f9a6f2505..24420f9ebebb 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -679,9 +679,10 @@ function performConcurrentWorkOnRoot(root, didTimeout) { // We now have a consistent tree. The next step is either to commit it, // or, if something suspended, wait to commit it after a timeout. - const finishedWork: Fiber = ((root.finishedWork = - root.current.alternate): any); + const finishedWork: Fiber = (root.current.alternate: any); + root.finishedWork = finishedWork; root.finishedExpirationTime = expirationTime; + root.nextKnownPendingLevel = getRemainingExpirationTime(finishedWork); finishConcurrentRender(root, finishedWork, exitStatus, expirationTime); } @@ -717,9 +718,6 @@ function finishConcurrentRender( case RootSuspended: { markRootSuspendedAtTime(root, expirationTime); const lastSuspendedTime = root.lastSuspendedTime; - if (expirationTime === lastSuspendedTime) { - root.nextKnownPendingLevel = getRemainingExpirationTime(finishedWork); - } // We have an acceptable loading state. We need to figure out if we // should immediately commit it or wait a bit. @@ -792,9 +790,6 @@ function finishConcurrentRender( case RootSuspendedWithDelay: { markRootSuspendedAtTime(root, expirationTime); const lastSuspendedTime = root.lastSuspendedTime; - if (expirationTime === lastSuspendedTime) { - root.nextKnownPendingLevel = getRemainingExpirationTime(finishedWork); - } if ( // do not delay if we're inside an act() scope @@ -979,9 +974,10 @@ function performSyncWorkOnRoot(root) { // We now have a consistent tree. Because this is a sync render, we // will commit it even if something suspended. - root.finishedWork = (root.current.alternate: any); + const finishedWork: Fiber = (root.current.alternate: any); + root.finishedWork = finishedWork; root.finishedExpirationTime = expirationTime; - + root.nextKnownPendingLevel = getRemainingExpirationTime(finishedWork); commitRoot(root); // Before exiting, make sure there's a callback scheduled for the next @@ -1176,6 +1172,8 @@ function prepareFreshStack(root, expirationTime) { // to it later, in case the render we're about to start gets aborted. // Generally we only reach this path via a ping, but we shouldn't assume // that will always be the case. + // Note: This is defensive coding to prevent a pending commit from + // being dropped without being rescheduled. It shouldn't be necessary. if (lastPingedTime === NoWork || lastPingedTime > lastSuspendedTime) { root.lastPingedTime = lastSuspendedTime; } @@ -1772,7 +1770,6 @@ function commitRootImpl(root, renderPriorityLevel) { root.callbackNode = null; root.callbackExpirationTime = NoWork; root.callbackPriority = NoPriority; - root.nextKnownPendingLevel = NoWork; // Update the first and last pending times on this root. The new first // pending time is whatever is left on the root fiber. diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js index 966fc653ca12..d27b422f46c5 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js @@ -3645,4 +3645,117 @@ describe('ReactSuspenseWithNoopRenderer', () => { , ); }); + + it('regression: ping at high priority causes update to be dropped', async () => { + const {useState, useTransition} = React; + + let setTextA; + function A() { + const [textA, _setTextA] = useState('A'); + setTextA = _setTextA; + return ( + }> + + + ); + } + + let setTextB; + let startTransition; + function B() { + const [textB, _setTextB] = useState('B'); + const [_startTransition] = useTransition({timeoutMs: 10000}); + startTransition = _startTransition; + setTextB = _setTextB; + return ( + }> + + + ); + } + + function App() { + return ( + <> + + + + ); + } + + const root = ReactNoop.createRoot(); + await ReactNoop.act(async () => { + await resolveText('A'); + await resolveText('B'); + root.render(); + }); + expect(Scheduler).toHaveYielded(['A', 'B']); + expect(root).toMatchRenderedOutput( + <> + + + , + ); + + await ReactNoop.act(async () => { + // Triggers suspense at normal pri + setTextA('A1'); + // Triggers in an unrelated tree at a different pri + startTransition(() => { + // Update A again so that it doesn't suspend on A1. That way we can ping + // the A1 update without also pinging this one. This is a workaround + // because there's currently no way to render at a lower priority (B2) + // without including all updates at higher priority (A1). + setTextA('A2'); + setTextB('B2'); + }); + }); + expect(Scheduler).toHaveYielded([ + 'B', + 'Suspend! [A1]', + 'Loading...', + + 'Suspend! [A2]', + 'Loading...', + 'Suspend! [B2]', + 'Loading...', + ]); + expect(root).toMatchRenderedOutput( + <> + + + , + ); + + await ReactNoop.act(async () => { + resolveText('A1'); + }); + expect(Scheduler).toHaveYielded([ + 'Promise resolved [A1]', + 'A1', + 'Suspend! [A2]', + 'Loading...', + 'Suspend! [B2]', + 'Loading...', + ]); + expect(root).toMatchRenderedOutput( + <> + + + , + ); + + // Commit the placeholder + Scheduler.unstable_advanceTime(20000); + await advanceTimers(20000); + + expect(root).toMatchRenderedOutput( + <> +