diff --git a/packages/react-reconciler/src/ReactFiberLane.new.js b/packages/react-reconciler/src/ReactFiberLane.new.js index dbbbc486265b9..878fc433f3c53 100644 --- a/packages/react-reconciler/src/ReactFiberLane.new.js +++ b/packages/react-reconciler/src/ReactFiberLane.new.js @@ -401,7 +401,6 @@ export function markStarvedLanesAsExpired( // expiration time. If so, we'll assume the update is being starved and mark // it as expired to force it to finish. let lanes = pendingLanes; - let expiredLanes = 0; while (lanes > 0) { const index = pickArbitraryLaneIndex(lanes); const lane = 1 << index; @@ -420,15 +419,11 @@ export function markStarvedLanesAsExpired( } } else if (expirationTime <= currentTime) { // This lane expired - expiredLanes |= lane; + root.expiredLanes |= lane; } lanes &= ~lane; } - - if (expiredLanes !== 0) { - markRootExpired(root, expiredLanes); - } } // This returns the highest priority pending lanes regardless of whether they @@ -459,16 +454,22 @@ export function includesOnlyTransitions(lanes: Lanes) { } export function shouldTimeSlice(root: FiberRoot, lanes: Lanes) { - if (!enableSyncDefaultUpdates) { + if ((lanes & root.expiredLanes) !== NoLanes) { + // At least one of these lanes expired. To prevent additional starvation, + // finish rendering without yielding execution. + return false; + } + if (enableSyncDefaultUpdates) { + const SyncDefaultLanes = + InputContinuousHydrationLane | + InputContinuousLane | + DefaultHydrationLane | + DefaultLane; + // TODO: Check for root override, once that lands + return (lanes & SyncDefaultLanes) === NoLanes; + } else { return true; } - const SyncDefaultLanes = - InputContinuousHydrationLane | - InputContinuousLane | - DefaultHydrationLane | - DefaultLane; - // TODO: Check for root override, once that lands - return (lanes & SyncDefaultLanes) === NoLanes; } export function isTransitionLane(lane: Lane) { @@ -613,14 +614,6 @@ export function markRootPinged( root.pingedLanes |= root.suspendedLanes & pingedLanes; } -export function markRootExpired(root: FiberRoot, expiredLanes: Lanes) { - const entanglements = root.entanglements; - const SyncLaneIndex = 0; - entanglements[SyncLaneIndex] |= expiredLanes; - root.entangledLanes |= SyncLane; - root.pendingLanes |= SyncLane; -} - export function markRootMutableRead(root: FiberRoot, updateLane: Lane) { root.mutableReadLanes |= updateLane & root.pendingLanes; } @@ -634,6 +627,7 @@ export function markRootFinished(root: FiberRoot, remainingLanes: Lanes) { root.suspendedLanes = 0; root.pingedLanes = 0; + root.expiredLanes &= remainingLanes; root.mutableReadLanes &= remainingLanes; root.entangledLanes &= remainingLanes; diff --git a/packages/react-reconciler/src/ReactFiberLane.old.js b/packages/react-reconciler/src/ReactFiberLane.old.js index af432cc6fd433..d4010468f0d74 100644 --- a/packages/react-reconciler/src/ReactFiberLane.old.js +++ b/packages/react-reconciler/src/ReactFiberLane.old.js @@ -401,7 +401,6 @@ export function markStarvedLanesAsExpired( // expiration time. If so, we'll assume the update is being starved and mark // it as expired to force it to finish. let lanes = pendingLanes; - let expiredLanes = 0; while (lanes > 0) { const index = pickArbitraryLaneIndex(lanes); const lane = 1 << index; @@ -420,15 +419,11 @@ export function markStarvedLanesAsExpired( } } else if (expirationTime <= currentTime) { // This lane expired - expiredLanes |= lane; + root.expiredLanes |= lane; } lanes &= ~lane; } - - if (expiredLanes !== 0) { - markRootExpired(root, expiredLanes); - } } // This returns the highest priority pending lanes regardless of whether they @@ -459,16 +454,22 @@ export function includesOnlyTransitions(lanes: Lanes) { } export function shouldTimeSlice(root: FiberRoot, lanes: Lanes) { - if (!enableSyncDefaultUpdates) { + if ((lanes & root.expiredLanes) !== NoLanes) { + // At least one of these lanes expired. To prevent additional starvation, + // finish rendering without yielding execution. + return false; + } + if (enableSyncDefaultUpdates) { + const SyncDefaultLanes = + InputContinuousHydrationLane | + InputContinuousLane | + DefaultHydrationLane | + DefaultLane; + // TODO: Check for root override, once that lands + return (lanes & SyncDefaultLanes) === NoLanes; + } else { return true; } - const SyncDefaultLanes = - InputContinuousHydrationLane | - InputContinuousLane | - DefaultHydrationLane | - DefaultLane; - // TODO: Check for root override, once that lands - return (lanes & SyncDefaultLanes) === NoLanes; } export function isTransitionLane(lane: Lane) { @@ -613,14 +614,6 @@ export function markRootPinged( root.pingedLanes |= root.suspendedLanes & pingedLanes; } -export function markRootExpired(root: FiberRoot, expiredLanes: Lanes) { - const entanglements = root.entanglements; - const SyncLaneIndex = 0; - entanglements[SyncLaneIndex] |= expiredLanes; - root.entangledLanes |= SyncLane; - root.pendingLanes |= SyncLane; -} - export function markRootMutableRead(root: FiberRoot, updateLane: Lane) { root.mutableReadLanes |= updateLane & root.pendingLanes; } @@ -634,6 +627,7 @@ export function markRootFinished(root: FiberRoot, remainingLanes: Lanes) { root.suspendedLanes = 0; root.pingedLanes = 0; + root.expiredLanes &= remainingLanes; root.mutableReadLanes &= remainingLanes; root.entangledLanes &= remainingLanes; diff --git a/packages/react-reconciler/src/ReactFiberRoot.new.js b/packages/react-reconciler/src/ReactFiberRoot.new.js index 9201a35980753..728769261cbe9 100644 --- a/packages/react-reconciler/src/ReactFiberRoot.new.js +++ b/packages/react-reconciler/src/ReactFiberRoot.new.js @@ -50,6 +50,7 @@ function FiberRootNode(containerInfo, tag, hydrate) { this.pendingLanes = NoLanes; this.suspendedLanes = NoLanes; this.pingedLanes = NoLanes; + this.expiredLanes = NoLanes; this.mutableReadLanes = NoLanes; this.finishedLanes = NoLanes; diff --git a/packages/react-reconciler/src/ReactFiberRoot.old.js b/packages/react-reconciler/src/ReactFiberRoot.old.js index f2968b314ecae..7f338a4dbb21a 100644 --- a/packages/react-reconciler/src/ReactFiberRoot.old.js +++ b/packages/react-reconciler/src/ReactFiberRoot.old.js @@ -50,6 +50,7 @@ function FiberRootNode(containerInfo, tag, hydrate) { this.pendingLanes = NoLanes; this.suspendedLanes = NoLanes; this.pingedLanes = NoLanes; + this.expiredLanes = NoLanes; this.mutableReadLanes = NoLanes; this.finishedLanes = NoLanes; diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index c688214a100e5..ddf72f428dfd4 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -159,7 +159,7 @@ import { markRootUpdated, markRootSuspended as markRootSuspended_dontCallThisOneDirectly, markRootPinged, - markRootExpired, + markRootEntangled, markRootFinished, getHighestPriorityLane, addFiberToLanesMap, @@ -787,22 +787,17 @@ function performConcurrentWorkOnRoot(root, didTimeout) { return null; } + // We disable time-slicing in some cases: if the work has been CPU-bound + // for too long ("expired" work, to prevent starvation), or we're in + // sync-updates-by-default mode. // TODO: We only check `didTimeout` defensively, to account for a Scheduler // bug we're still investigating. Once the bug in Scheduler is fixed, // we can remove this, since we track expiration ourselves. - if (!disableSchedulerTimeoutInWorkLoop && didTimeout) { - // Something expired. Flush synchronously until there's no expired - // work left. - markRootExpired(root, lanes); - // This will schedule a synchronous callback. - ensureRootIsScheduled(root, now()); - return null; - } - - let exitStatus = shouldTimeSlice(root, lanes) - ? renderRootConcurrent(root, lanes) - : // Time slicing is disabled for default updates in this root. - renderRootSync(root, lanes); + let exitStatus = + shouldTimeSlice(root, lanes) && + (disableSchedulerTimeoutInWorkLoop || !didTimeout) + ? renderRootConcurrent(root, lanes) + : renderRootSync(root, lanes); if (exitStatus !== RootIncomplete) { if (exitStatus === RootErrored) { executionContext |= RetryAfterError; @@ -990,16 +985,7 @@ function performSyncWorkOnRoot(root) { flushPassiveEffects(); let lanes = getNextLanes(root, NoLanes); - if (includesSomeLane(lanes, SyncLane)) { - if ( - root === workInProgressRoot && - includesSomeLane(lanes, workInProgressRootRenderLanes) - ) { - // There's a partial tree, and at least one of its lanes has expired. Finish - // rendering it before rendering the rest of the expired work. - lanes = workInProgressRootRenderLanes; - } - } else { + if (!includesSomeLane(lanes, SyncLane)) { // There's no remaining sync work left. ensureRootIsScheduled(root, now()); return null; @@ -1052,11 +1038,9 @@ function performSyncWorkOnRoot(root) { return null; } -// TODO: Do we still need this API? I think we can delete it. Was only used -// internally. export function flushRoot(root: FiberRoot, lanes: Lanes) { if (lanes !== NoLanes) { - markRootExpired(root, lanes); + markRootEntangled(root, mergeLanes(lanes, SyncLane)); ensureRootIsScheduled(root, now()); if ((executionContext & (RenderContext | CommitContext)) === NoContext) { resetRenderTimer(); diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index fb5ae4c1d7a25..3b0b196e112cd 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -159,7 +159,7 @@ import { markRootUpdated, markRootSuspended as markRootSuspended_dontCallThisOneDirectly, markRootPinged, - markRootExpired, + markRootEntangled, markRootFinished, getHighestPriorityLane, addFiberToLanesMap, @@ -787,22 +787,17 @@ function performConcurrentWorkOnRoot(root, didTimeout) { return null; } + // We disable time-slicing in some cases: if the work has been CPU-bound + // for too long ("expired" work, to prevent starvation), or we're in + // sync-updates-by-default mode. // TODO: We only check `didTimeout` defensively, to account for a Scheduler // bug we're still investigating. Once the bug in Scheduler is fixed, // we can remove this, since we track expiration ourselves. - if (!disableSchedulerTimeoutInWorkLoop && didTimeout) { - // Something expired. Flush synchronously until there's no expired - // work left. - markRootExpired(root, lanes); - // This will schedule a synchronous callback. - ensureRootIsScheduled(root, now()); - return null; - } - - let exitStatus = shouldTimeSlice(root, lanes) - ? renderRootConcurrent(root, lanes) - : // Time slicing is disabled for default updates in this root. - renderRootSync(root, lanes); + let exitStatus = + shouldTimeSlice(root, lanes) && + (disableSchedulerTimeoutInWorkLoop || !didTimeout) + ? renderRootConcurrent(root, lanes) + : renderRootSync(root, lanes); if (exitStatus !== RootIncomplete) { if (exitStatus === RootErrored) { executionContext |= RetryAfterError; @@ -990,16 +985,7 @@ function performSyncWorkOnRoot(root) { flushPassiveEffects(); let lanes = getNextLanes(root, NoLanes); - if (includesSomeLane(lanes, SyncLane)) { - if ( - root === workInProgressRoot && - includesSomeLane(lanes, workInProgressRootRenderLanes) - ) { - // There's a partial tree, and at least one of its lanes has expired. Finish - // rendering it before rendering the rest of the expired work. - lanes = workInProgressRootRenderLanes; - } - } else { + if (!includesSomeLane(lanes, SyncLane)) { // There's no remaining sync work left. ensureRootIsScheduled(root, now()); return null; @@ -1052,11 +1038,9 @@ function performSyncWorkOnRoot(root) { return null; } -// TODO: Do we still need this API? I think we can delete it. Was only used -// internally. export function flushRoot(root: FiberRoot, lanes: Lanes) { if (lanes !== NoLanes) { - markRootExpired(root, lanes); + markRootEntangled(root, mergeLanes(lanes, SyncLane)); ensureRootIsScheduled(root, now()); if ((executionContext & (RenderContext | CommitContext)) === NoContext) { resetRenderTimer(); diff --git a/packages/react-reconciler/src/ReactInternalTypes.js b/packages/react-reconciler/src/ReactInternalTypes.js index 08f5cbc2d0e9a..6d7d38232e347 100644 --- a/packages/react-reconciler/src/ReactInternalTypes.js +++ b/packages/react-reconciler/src/ReactInternalTypes.js @@ -228,6 +228,7 @@ type BaseFiberRootProperties = {| pendingLanes: Lanes, suspendedLanes: Lanes, pingedLanes: Lanes, + expiredLanes: Lanes, mutableReadLanes: Lanes, finishedLanes: Lanes, diff --git a/packages/react-reconciler/src/__tests__/ReactExpiration-test.js b/packages/react-reconciler/src/__tests__/ReactExpiration-test.js index 4c89c341c649f..885ee46e1477d 100644 --- a/packages/react-reconciler/src/__tests__/ReactExpiration-test.js +++ b/packages/react-reconciler/src/__tests__/ReactExpiration-test.js @@ -15,6 +15,8 @@ let Scheduler; let readText; let resolveText; let startTransition; +let useState; +let useEffect; describe('ReactExpiration', () => { beforeEach(() => { @@ -24,6 +26,8 @@ describe('ReactExpiration', () => { ReactNoop = require('react-noop-renderer'); Scheduler = require('scheduler'); startTransition = React.unstable_startTransition; + useState = React.useState; + useEffect = React.useEffect; const textCache = new Map(); @@ -478,9 +482,7 @@ describe('ReactExpiration', () => { }); // @gate experimental || !enableSyncDefaultUpdates - it('prevents starvation by sync updates', async () => { - const {useState} = React; - + it('prevents starvation by sync updates by disabling time slicing if too much time has elapsed', async () => { let updateSyncPri; let updateNormalPri; function App() { @@ -519,15 +521,17 @@ describe('ReactExpiration', () => { } expect(Scheduler).toFlushAndYieldThrough(['Sync pri: 0']); updateSyncPri(); + expect(Scheduler).toHaveYielded(['Sync pri: 1', 'Normal pri: 0']); + + // The remaining work hasn't expired, so the render phase is time sliced. + // In other words, we can flush just the first child without flushing + // the rest. + Scheduler.unstable_flushNumberOfYields(1); + // Yield right after first child. + expect(Scheduler).toHaveYielded(['Sync pri: 1']); + // Now do the rest. + expect(Scheduler).toFlushAndYield(['Normal pri: 1']); }); - expect(Scheduler).toHaveYielded([ - // Interrupt high pri update to render sync update - 'Sync pri: 1', - 'Normal pri: 0', - // Now render normal pri - 'Sync pri: 1', - 'Normal pri: 1', - ]); expect(root).toMatchRenderedOutput('Sync pri: 1, Normal pri: 1'); // Do the same thing, but starve the first update @@ -547,22 +551,18 @@ describe('ReactExpiration', () => { // starvation of normal priority updates.) Scheduler.unstable_advanceTime(10000); - // So when we get a high pri update, we shouldn't interrupt updateSyncPri(); + expect(Scheduler).toHaveYielded(['Sync pri: 2', 'Normal pri: 1']); + + // The remaining work _has_ expired, so the render phase is _not_ time + // sliced. Attempting to flush just the first child also flushes the rest. + Scheduler.unstable_flushNumberOfYields(1); + expect(Scheduler).toHaveYielded(['Sync pri: 2', 'Normal pri: 2']); }); - expect(Scheduler).toHaveYielded([ - // Finish normal pri update - 'Normal pri: 2', - // Then do high pri update - 'Sync pri: 2', - 'Normal pri: 2', - ]); expect(root).toMatchRenderedOutput('Sync pri: 2, Normal pri: 2'); }); it('idle work never expires', async () => { - const {useState} = React; - let updateSyncPri; let updateIdlePri; function App() { @@ -629,23 +629,19 @@ describe('ReactExpiration', () => { }); // @gate experimental - it('a single update can expire without forcing all other updates to expire', async () => { - const {useState} = React; - - let updateHighPri; - let updateNormalPri; + it('when multiple lanes expire, we can finish the in-progress one without including the others', async () => { + let setA; + let setB; function App() { - const [highPri, setHighPri] = useState(0); - const [normalPri, setNormalPri] = useState(0); - updateHighPri = () => ReactNoop.flushSync(() => setHighPri(n => n + 1)); - updateNormalPri = () => setNormalPri(n => n + 1); + const [a, _setA] = useState(0); + const [b, _setB] = useState(0); + setA = _setA; + setB = _setB; return ( <> - - {', '} - - {', '} - + + + ); } @@ -654,184 +650,168 @@ describe('ReactExpiration', () => { await ReactNoop.act(async () => { root.render(); }); - expect(Scheduler).toHaveYielded([ - 'High pri: 0', - 'Normal pri: 0', - 'Sibling', - ]); - expect(root).toMatchRenderedOutput('High pri: 0, Normal pri: 0, Sibling'); + expect(Scheduler).toHaveYielded(['A0', 'B0', 'C']); + expect(root).toMatchRenderedOutput('A0B0C'); await ReactNoop.act(async () => { - // Partially render an update startTransition(() => { - updateNormalPri(); + setA(1); }); - expect(Scheduler).toFlushAndYieldThrough(['High pri: 0']); - - // Some time goes by. Schedule another update. - // This will be placed into a separate batch. - Scheduler.unstable_advanceTime(4000); - + expect(Scheduler).toFlushAndYieldThrough(['A1']); startTransition(() => { - updateNormalPri(); - }); - // Keep rendering the first update - expect(Scheduler).toFlushAndYieldThrough(['Normal pri: 1']); - // More time goes by. Enough to expire the first batch, but not the - // second one. - Scheduler.unstable_advanceTime(1000); - // Attempt to interrupt with a high pri update. - await ReactNoop.act(async () => { - updateHighPri(); + setB(1); }); - - expect(Scheduler).toHaveYielded([ - // The first update expired - 'Sibling', - // Then render the high pri update - 'High pri: 1', - 'Normal pri: 1', - 'Sibling', - // Then the second normal pri update - 'High pri: 1', - 'Normal pri: 2', - 'Sibling', - ]); + // Expire both the transitions + Scheduler.unstable_advanceTime(10000); + // Both transitions have expired, but since they aren't related + // (entangled), we should be able to finish the in-progress transition + // without also including the next one. + Scheduler.unstable_flushNumberOfYields(1); + expect(Scheduler).toHaveYielded(['B0', 'C']); + expect(root).toMatchRenderedOutput('A1B0C'); + + // The next transition also finishes without yielding. + Scheduler.unstable_flushNumberOfYields(1); + expect(Scheduler).toHaveYielded(['A1', 'B1', 'C']); + expect(root).toMatchRenderedOutput('A1B1C'); }); }); // @gate experimental || !enableSyncDefaultUpdates - it('detects starvation in multiple batches', async () => { - const {useState} = React; + it('updates do not expire while they are IO-bound', async () => { + const {Suspense} = React; - let updateHighPri; - let updateNormalPri; - function App() { - const [highPri, setHighPri] = useState(0); - const [normalPri, setNormalPri] = useState(0); - updateHighPri = () => { - ReactNoop.flushSync(() => { - setHighPri(n => n + 1); - }); - }; - updateNormalPri = () => setNormalPri(n => n + 1); + function App({step}) { return ( - <> - - {', '} - - {', '} - - + }> + + + + ); } const root = ReactNoop.createRoot(); await ReactNoop.act(async () => { - root.render(); + await resolveText('A0'); + root.render(); }); - expect(Scheduler).toHaveYielded([ - 'High pri: 0', - 'Normal pri: 0', - 'Sibling', - ]); - expect(root).toMatchRenderedOutput('High pri: 0, Normal pri: 0, Sibling'); + expect(Scheduler).toHaveYielded(['A0', 'B', 'C']); + expect(root).toMatchRenderedOutput('A0BC'); await ReactNoop.act(async () => { - // Partially render an update if (gate(flags => flags.enableSyncDefaultUpdates)) { React.unstable_startTransition(() => { - updateNormalPri(); + root.render(); }); } else { - updateNormalPri(); + root.render(); } - expect(Scheduler).toFlushAndYieldThrough(['High pri: 0']); - // Some time goes by. In an interleaved event, schedule another update. - // This will be placed into a separate batch. - Scheduler.unstable_advanceTime(4000); - updateNormalPri(); - // Keep rendering the first update - expect(Scheduler).toFlushAndYieldThrough(['Normal pri: 1']); - // More time goes by. This expires both of the updates just scheduled. + expect(Scheduler).toFlushAndYield([ + 'Suspend! [A1]', + 'B', + 'C', + 'Loading...', + ]); + + // Lots of time elapses before the promise resolves Scheduler.unstable_advanceTime(10000); - expect(Scheduler).toHaveYielded([]); + await resolveText('A1'); + expect(Scheduler).toHaveYielded(['Promise resolved [A1]']); - // Attempt to interrupt with a high pri update. - updateHighPri(); - - // Both normal pri updates should have expired. - // The sync update and the expired normal pri updates render in a - // single batch. - expect(Scheduler).toHaveYielded([ - 'Sibling', - 'High pri: 1', - 'Normal pri: 2', - 'Sibling', - ]); + // But the update doesn't expire, because it was IO bound. So we can + // partially rendering without finishing. + expect(Scheduler).toFlushAndYieldThrough(['A1']); + expect(root).toMatchRenderedOutput('A0BC'); + + // Lots more time elapses. We're CPU-bound now, so we should treat this + // as starvation. + Scheduler.unstable_advanceTime(10000); + + // The rest of the update finishes without yielding. + Scheduler.unstable_flushNumberOfYields(1); + expect(Scheduler).toHaveYielded(['B', 'C']); }); }); - // @gate experimental || !enableSyncDefaultUpdates - it('updates do not expire while they are IO-bound', async () => { - const {Suspense} = React; - - function App({text}) { + // @gate experimental + it('flushSync should not affect expired work', async () => { + let setA; + let setB; + function App() { + const [a, _setA] = useState(0); + const [b, _setB] = useState(0); + setA = _setA; + setB = _setB; return ( - }> - - {', '} - - + <> + + + ); } const root = ReactNoop.createRoot(); await ReactNoop.act(async () => { - await resolveText('A'); - root.render(); + root.render(); }); - expect(Scheduler).toHaveYielded(['A', 'Sibling']); - expect(root).toMatchRenderedOutput('A, Sibling'); + expect(Scheduler).toHaveYielded(['A0', 'B0']); await ReactNoop.act(async () => { - if (gate(flags => flags.enableSyncDefaultUpdates)) { - React.unstable_startTransition(() => { - root.render(); - }); - } else { - root.render(); - } - expect(Scheduler).toFlushAndYield([ - 'Suspend! [B]', - 'Sibling', - 'Loading...', - ]); + startTransition(() => { + setA(1); + }); + expect(Scheduler).toFlushAndYieldThrough(['A1']); - // Lots of time elapses before the promise resolves + // Expire the in-progress update Scheduler.unstable_advanceTime(10000); - await resolveText('B'); - expect(Scheduler).toHaveYielded(['Promise resolved [B]']); - // But the update doesn't expire, because it was IO bound. So we can - // partially rendering without finishing. - expect(Scheduler).toFlushAndYieldThrough(['B']); - expect(root).toMatchRenderedOutput('A, Sibling'); + ReactNoop.flushSync(() => { + setB(1); + }); + expect(Scheduler).toHaveYielded(['A0', 'B1']); - // Lots more time elapses. We're CPU-bound now, so we should treat this - // as starvation. + // Now flush the original update. Because it expired, it should finish + // without yielding. + Scheduler.unstable_flushNumberOfYields(1); + expect(Scheduler).toHaveYielded(['A1', 'B1']); + }); + }); + + // @gate experimental + it('passive effects of expired update flush after paint', async () => { + function App({step}) { + useEffect(() => { + Scheduler.unstable_yieldValue('Effect: ' + step); + }, [step]); + return ( + <> + + + + + ); + } + + const root = ReactNoop.createRoot(); + await ReactNoop.act(async () => { + root.render(); + }); + expect(Scheduler).toHaveYielded(['A0', 'B0', 'C0', 'Effect: 0']); + expect(root).toMatchRenderedOutput('A0B0C0'); + + await ReactNoop.act(async () => { + startTransition(() => { + root.render(); + }); + // Expire the update Scheduler.unstable_advanceTime(10000); - // Attempt to interrupt with a sync update. - ReactNoop.flushSync(() => root.render()); - expect(Scheduler).toHaveYielded([ - // Because the previous update had already expired, we don't interrupt - // it. Finish rendering it first. - 'Sibling', - // Then do the sync update. - 'A', - 'Sibling', - ]); + // The update finishes without yielding. But it does not flush the effect. + Scheduler.unstable_flushNumberOfYields(1); + expect(Scheduler).toHaveYielded(['A1', 'B1', 'C1']); }); + // The effect flushes after paint. + expect(Scheduler).toHaveYielded(['Effect: 1']); }); }); diff --git a/packages/react-reconciler/src/__tests__/ReactSchedulerIntegration-test.js b/packages/react-reconciler/src/__tests__/ReactSchedulerIntegration-test.js index d0fb1fec858c4..3c09935a5b7cc 100644 --- a/packages/react-reconciler/src/__tests__/ReactSchedulerIntegration-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSchedulerIntegration-test.js @@ -300,10 +300,9 @@ describe( ReactNoop.render(); }); - ReactNoop.flushSync(); - // Because the render expired, React should finish the tree without // consulting `shouldYield` again + Scheduler.unstable_flushNumberOfYields(1); expect(Scheduler).toHaveYielded(['B', 'C']); }); }); diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js index 7861b2904f28d..668299b282b9b 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js @@ -1958,32 +1958,13 @@ describe('ReactSuspenseWithNoopRenderer', () => { await advanceTimers(5000); // Retry with the new content. - if (gate(flags => flags.disableSchedulerTimeoutInWorkLoop)) { - expect(Scheduler).toFlushAndYield([ - 'A', - // B still suspends - 'Suspend! [B]', - 'Loading more...', - ]); - } else { - // In this branch, right as we start rendering, we detect that the work - // has expired (via Scheduler's didTimeout argument) and re-schedule the - // work as synchronous. Since sync work does not flow through Scheduler, - // we need to use `flushSync`. - // - // Usually we would use `act`, which fluses both sync work and Scheduler - // work, but that would also force the fallback to display, and this test - // is specifically about whether we delay or show the fallback. - expect(Scheduler).toFlushAndYield([]); - // This will flush the synchronous callback we just scheduled. - ReactNoop.flushSync(); - expect(Scheduler).toHaveYielded([ - 'A', - // B still suspends - 'Suspend! [B]', - 'Loading more...', - ]); - } + expect(Scheduler).toFlushAndYield([ + 'A', + // B still suspends + 'Suspend! [B]', + 'Loading more...', + ]); + // Because we've already been waiting for so long we've exceeded // our threshold and we show the next level immediately. expect(ReactNoop.getChildren()).toEqual([