diff --git a/packages/react-dom/src/__tests__/ReactDOMSelect-test.js b/packages/react-dom/src/__tests__/ReactDOMSelect-test.js index 72ebfd1bbe67b..e450cf9a8166e 100644 --- a/packages/react-dom/src/__tests__/ReactDOMSelect-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMSelect-test.js @@ -911,9 +911,7 @@ describe('ReactDOMSelect', () => { const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); async function changeView() { - await act(() => { - root.unmount(); - }); + root.unmount(); } const stub = ( diff --git a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js index e9354b0bf259d..9615c34256c3c 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js @@ -3743,6 +3743,10 @@ describe('ReactDOMServerPartialHydration', () => { await waitForPaint(['App']); expect(visibleRef.current).toBe(visibleSpan); + if (gate(flags => flags.enableYieldingBeforePassive)) { + // Passive effects. + await waitForPaint([]); + } // Subsequently, the hidden child is prerendered on the client await waitForPaint(['HiddenChild']); expect(container).toMatchInlineSnapshot(` diff --git a/packages/react-reconciler/src/ReactFiberRootScheduler.js b/packages/react-reconciler/src/ReactFiberRootScheduler.js index 8756a9c89009c..dcaadc5a6ef18 100644 --- a/packages/react-reconciler/src/ReactFiberRootScheduler.js +++ b/packages/react-reconciler/src/ReactFiberRootScheduler.js @@ -20,6 +20,7 @@ import { enableProfilerNestedUpdatePhase, enableComponentPerformanceTrack, enableSiblingPrerendering, + enableYieldingBeforePassive, } from 'shared/ReactFeatureFlags'; import { NoLane, @@ -41,6 +42,8 @@ import { getExecutionContext, getWorkInProgressRoot, getWorkInProgressRootRenderLanes, + getRootWithPendingPassiveEffects, + getPendingPassiveEffectsLanes, isWorkLoopSuspendedOnData, performWorkOnRoot, } from './ReactFiberWorkLoop'; @@ -324,12 +327,21 @@ function scheduleTaskForRootDuringMicrotask( markStarvedLanesAsExpired(root, currentTime); // Determine the next lanes to work on, and their priority. + const rootWithPendingPassiveEffects = getRootWithPendingPassiveEffects(); + const pendingPassiveEffectsLanes = getPendingPassiveEffectsLanes(); const workInProgressRoot = getWorkInProgressRoot(); const workInProgressRootRenderLanes = getWorkInProgressRootRenderLanes(); - const nextLanes = getNextLanes( - root, - root === workInProgressRoot ? workInProgressRootRenderLanes : NoLanes, - ); + const nextLanes = + enableYieldingBeforePassive && root === rootWithPendingPassiveEffects + ? // This will schedule the callback at the priority of the lane but we used to + // always schedule it at NormalPriority. Discrete will flush it sync anyway. + // So the only difference is Idle and it doesn't seem necessarily right for that + // to get upgraded beyond something important just because we're past commit. + pendingPassiveEffectsLanes + : getNextLanes( + root, + root === workInProgressRoot ? workInProgressRootRenderLanes : NoLanes, + ); const existingCallbackNode = root.callbackNode; if ( diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 9a002ce4b6fd9..d524dd8599621 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -40,6 +40,7 @@ import { disableDefaultPropsExceptForClasses, enableSiblingPrerendering, enableComponentPerformanceTrack, + enableYieldingBeforePassive, } from 'shared/ReactFeatureFlags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import is from 'shared/objectIs'; @@ -610,7 +611,6 @@ export function getRenderTargetTime(): number { let legacyErrorBoundariesThatAlreadyFailed: Set | null = null; -let rootDoesHavePassiveEffects: boolean = false; let rootWithPendingPassiveEffects: FiberRoot | null = null; let pendingPassiveEffectsLanes: Lanes = NoLanes; let pendingPassiveEffectsRemainingLanes: Lanes = NoLanes; @@ -638,6 +638,14 @@ export function getWorkInProgressRootRenderLanes(): Lanes { return workInProgressRootRenderLanes; } +export function getRootWithPendingPassiveEffects(): FiberRoot | null { + return rootWithPendingPassiveEffects; +} + +export function getPendingPassiveEffectsLanes(): Lanes { + return pendingPassiveEffectsLanes; +} + export function isWorkLoopSuspendedOnData(): boolean { return ( workInProgressSuspendedReason === SuspendedOnData || @@ -3210,12 +3218,6 @@ function commitRootImpl( ); } - // commitRoot never returns a continuation; it always finishes synchronously. - // So we can clear these now to allow a new callback to be scheduled. - root.callbackNode = null; - root.callbackPriority = NoLane; - root.cancelPendingCommit = null; - // Check which lanes no longer have any work scheduled on them, and mark // those as finished. let remainingLanes = mergeLanes(finishedWork.lanes, finishedWork.childLanes); @@ -3253,6 +3255,7 @@ function commitRootImpl( // might get scheduled in the commit phase. (See #16714.) // TODO: Delete all other places that schedule the passive effect callback // They're redundant. + let rootDoesHavePassiveEffects: boolean = false; if ( // If this subtree rendered with profiling this commit, we need to visit it to log it. (enableProfilerTimer && @@ -3261,17 +3264,25 @@ function commitRootImpl( (finishedWork.subtreeFlags & PassiveMask) !== NoFlags || (finishedWork.flags & PassiveMask) !== NoFlags ) { - if (!rootDoesHavePassiveEffects) { - rootDoesHavePassiveEffects = true; - pendingPassiveEffectsRemainingLanes = remainingLanes; - pendingPassiveEffectsRenderEndTime = completedRenderEndTime; - // workInProgressTransitions might be overwritten, so we want - // to store it in pendingPassiveTransitions until they get processed - // We need to pass this through as an argument to commitRoot - // because workInProgressTransitions might have changed between - // the previous render and commit if we throttle the commit - // with setTimeout - pendingPassiveTransitions = transitions; + rootDoesHavePassiveEffects = true; + pendingPassiveEffectsRemainingLanes = remainingLanes; + pendingPassiveEffectsRenderEndTime = completedRenderEndTime; + // workInProgressTransitions might be overwritten, so we want + // to store it in pendingPassiveTransitions until they get processed + // We need to pass this through as an argument to commitRoot + // because workInProgressTransitions might have changed between + // the previous render and commit if we throttle the commit + // with setTimeout + pendingPassiveTransitions = transitions; + if (enableYieldingBeforePassive) { + // We don't schedule a separate task for flushing passive effects. + // Instead, we just rely on ensureRootIsScheduled below to schedule + // a callback for us to flush the passive effects. + } else { + // So we can clear these now to allow a new callback to be scheduled. + root.callbackNode = null; + root.callbackPriority = NoLane; + root.cancelPendingCommit = null; scheduleCallback(NormalSchedulerPriority, () => { if (enableProfilerTimer && enableComponentPerformanceTrack) { // Track the currently executing event if there is one so we can ignore this @@ -3285,6 +3296,12 @@ function commitRootImpl( return null; }); } + } else { + // If we don't have passive effects, we're not going to need to perform more work + // so we can clear the callback now. + root.callbackNode = null; + root.callbackPriority = NoLane; + root.cancelPendingCommit = null; } if (enableProfilerTimer) { @@ -3441,10 +3458,6 @@ function commitRootImpl( onCommitRootTestSelector(); } - // Always call this before exiting `commitRoot`, to ensure that any - // additional work on this root is scheduled. - ensureRootIsScheduled(root); - if (recoverableErrors !== null) { // There were errors during this render, but recovered from them without // needing to surface it to the UI. We log them here. @@ -3480,6 +3493,10 @@ function commitRootImpl( flushPassiveEffects(); } + // Always call this before exiting `commitRoot`, to ensure that any + // additional work on this root is scheduled. + ensureRootIsScheduled(root); + // Read this again, since a passive effect might have updated it remainingLanes = root.pendingLanes; @@ -3648,6 +3665,13 @@ function flushPassiveEffectsImpl(wasDelayedCommit: void | boolean) { // because it's only used for profiling), but it's a refactor hazard. pendingPassiveEffectsLanes = NoLanes; + if (enableYieldingBeforePassive) { + // We've finished our work for this render pass. + root.callbackNode = null; + root.callbackPriority = NoLane; + root.cancelPendingCommit = null; + } + if ((executionContext & (RenderContext | CommitContext)) !== NoContext) { throw new Error('Cannot flush passive effects while already rendering.'); } @@ -3745,6 +3769,14 @@ function flushPassiveEffectsImpl(wasDelayedCommit: void | boolean) { didScheduleUpdateDuringPassiveEffects = false; } + if (enableYieldingBeforePassive) { + // Next, we reschedule any remaining work in a new task since it's a new + // sequence of work. We wait until the end to do this in case the passive + // effect schedules higher priority work than we had remaining. That way + // we don't schedule an early callback that gets cancelled anyway. + ensureRootIsScheduled(root); + } + // TODO: Move to commitPassiveMountEffects onPostCommitRootDevTools(root); if (enableProfilerTimer && enableProfilerCommitHooks) { diff --git a/packages/react-reconciler/src/__tests__/ReactDeferredValue-test.js b/packages/react-reconciler/src/__tests__/ReactDeferredValue-test.js index 8ca142cb50517..bbd01cd551e86 100644 --- a/packages/react-reconciler/src/__tests__/ReactDeferredValue-test.js +++ b/packages/react-reconciler/src/__tests__/ReactDeferredValue-test.js @@ -753,6 +753,10 @@ describe('ReactDeferredValue', () => { revealContent(); // Because the preview state was already prerendered, we can reveal it // without any addditional work. + if (gate(flags => flags.enableYieldingBeforePassive)) { + // Passive effects. + await waitForPaint([]); + } await waitForPaint([]); expect(root).toMatchRenderedOutput(
Preview [B]
); }); diff --git a/packages/react-reconciler/src/__tests__/ReactExpiration-test.js b/packages/react-reconciler/src/__tests__/ReactExpiration-test.js index aef06288667db..5b09d7d0b8821 100644 --- a/packages/react-reconciler/src/__tests__/ReactExpiration-test.js +++ b/packages/react-reconciler/src/__tests__/ReactExpiration-test.js @@ -755,10 +755,16 @@ describe('ReactExpiration', () => { // The update finishes without yielding. But it does not flush the effect. await waitFor(['B1'], { - additionalLogsAfterAttemptingToYield: ['C1'], + additionalLogsAfterAttemptingToYield: gate( + flags => flags.enableYieldingBeforePassive, + ) + ? ['C1', 'Effect: 1'] + : ['C1'], }); }); - // The effect flushes after paint. - assertLog(['Effect: 1']); + if (!gate(flags => flags.enableYieldingBeforePassive)) { + // The effect flushes after paint. + assertLog(['Effect: 1']); + } }); }); diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js index a43b6c124df7d..dcb362669928e 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js @@ -2695,13 +2695,23 @@ describe('ReactHooksWithNoopRenderer', () => { React.startTransition(() => { ReactNoop.render(); }); - await waitForPaint([ - 'Create passive [current: 0]', - 'Destroy insertion [current: 0]', - 'Create insertion [current: 0]', - 'Destroy layout [current: 1]', - 'Create layout [current: 1]', - ]); + if (gate(flags => flags.enableYieldingBeforePassive)) { + await waitForPaint(['Create passive [current: 0]']); + await waitForPaint([ + 'Destroy insertion [current: 0]', + 'Create insertion [current: 0]', + 'Destroy layout [current: 1]', + 'Create layout [current: 1]', + ]); + } else { + await waitForPaint([ + 'Create passive [current: 0]', + 'Destroy insertion [current: 0]', + 'Create insertion [current: 0]', + 'Destroy layout [current: 1]', + 'Create layout [current: 1]', + ]); + } expect(committedText).toEqual('1'); }); assertLog([ diff --git a/packages/react-reconciler/src/__tests__/ReactSiblingPrerendering-test.js b/packages/react-reconciler/src/__tests__/ReactSiblingPrerendering-test.js index 235e8df2201de..ceb32160976e0 100644 --- a/packages/react-reconciler/src/__tests__/ReactSiblingPrerendering-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSiblingPrerendering-test.js @@ -200,6 +200,10 @@ describe('ReactSiblingPrerendering', () => { await waitForPaint(['A']); expect(root).toMatchRenderedOutput('A'); + if (gate(flags => flags.enableYieldingBeforePassive)) { + // Passive effects. + await waitForPaint([]); + } // The second render is a prerender of the hidden content. await waitForPaint([ 'Suspend! [B]', @@ -237,6 +241,10 @@ describe('ReactSiblingPrerendering', () => { // Immediately after the fallback commits, retry the boundary again. This // time we include B, since we're not blocking the fallback from showing. if (gate('enableSiblingPrerendering')) { + if (gate(flags => flags.enableYieldingBeforePassive)) { + // Passive effects. + await waitForPaint([]); + } await waitForPaint(['Suspend! [A]', 'Suspend! [B]']); } }); @@ -452,6 +460,10 @@ describe('ReactSiblingPrerendering', () => { , ); + if (gate(flags => flags.enableYieldingBeforePassive)) { + // Passive effects. + await waitForPaint([]); + } // Immediately after the fallback commits, retry the boundary again. // Because the promise for A resolved, this is a normal render, _not_ // a prerender. So when we proceed to B, and B suspends, we unwind again @@ -471,6 +483,10 @@ describe('ReactSiblingPrerendering', () => { , ); + if (gate(flags => flags.enableYieldingBeforePassive)) { + // Passive effects. + await waitForPaint([]); + } // Now we can proceed to prerendering C. if (gate('enableSiblingPrerendering')) { await waitForPaint(['Suspend! [B]', 'Suspend! [C]']); diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js index 39cfebec3487e..0dcf0eb4705ee 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js @@ -1901,6 +1901,10 @@ describe('ReactSuspenseWithNoopRenderer', () => { // be throttled because the fallback would have appeared too recently. Scheduler.unstable_advanceTime(10000); jest.advanceTimersByTime(10000); + if (gate(flags => flags.enableYieldingBeforePassive)) { + // Passive effects. + await waitForPaint([]); + } await waitForPaint(['A']); expect(ReactNoop).toMatchRenderedOutput( <> diff --git a/packages/react-reconciler/src/__tests__/ReactUpdatePriority-test.js b/packages/react-reconciler/src/__tests__/ReactUpdatePriority-test.js index aa146558917c1..b2eebfa51611e 100644 --- a/packages/react-reconciler/src/__tests__/ReactUpdatePriority-test.js +++ b/packages/react-reconciler/src/__tests__/ReactUpdatePriority-test.js @@ -81,14 +81,26 @@ describe('ReactUpdatePriority', () => { // Schedule another update at default priority setDefaultState(2); - // The default update flushes first, because - await waitForPaint([ - // Idle update is scheduled - 'Idle update', - - // The default update flushes first, without including the idle update - 'Idle: 1, Default: 2', - ]); + if (gate(flags => flags.enableYieldingBeforePassive)) { + // The default update flushes first, because + await waitForPaint([ + // Idle update is scheduled + 'Idle update', + ]); + await waitForPaint([ + // The default update flushes first, without including the idle update + 'Idle: 1, Default: 2', + ]); + } else { + // The default update flushes first, because + await waitForPaint([ + // Idle update is scheduled + 'Idle update', + + // The default update flushes first, without including the idle update + 'Idle: 1, Default: 2', + ]); + } }); // Now the idle update has flushed assertLog(['Idle: 2, Default: 2']); diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index fff1dcf4d9ddb..8540d6e14721a 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -77,6 +77,9 @@ export const enableLegacyFBSupport = false; // likely to include in an upcoming release. // ----------------------------------------------------------------------------- +// Yield to the browser event loop and not just the scheduler event loop before passive effects. +export const enableYieldingBeforePassive = __EXPERIMENTAL__; + export const enableLegacyCache = __EXPERIMENTAL__; export const enableAsyncIterableChildren = __EXPERIMENTAL__; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 2e37d9aeee5f1..45807e9d876af 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -82,6 +82,7 @@ export const syncLaneExpirationMs = 250; export const transitionLaneExpirationMs = 5000; export const useModernStrictMode = true; export const enableHydrationLaneScheduling = true; +export const enableYieldingBeforePassive = false; // Flow magic to verify the exports of this file match the original version. ((((null: any): ExportsType): FeatureFlagsType): ExportsType); diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 6c86d9f60d405..c29da18a42055 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -76,6 +76,8 @@ export const enableUseResourceEffectHook = false; export const enableHydrationLaneScheduling = true; +export const enableYieldingBeforePassive = false; + // Profiling Only export const enableProfilerTimer = __PROFILE__; export const enableProfilerCommitHooks = __PROFILE__; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 6ba73ae150413..df58c53eda9a4 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -72,6 +72,8 @@ export const enableSiblingPrerendering = true; export const enableUseResourceEffectHook = false; +export const enableYieldingBeforePassive = true; + // TODO: This must be in sync with the main ReactFeatureFlags file because // the Test Renderer's value must be the same as the one used by the // react package. diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js index 14e351fb2f078..a4dc5cb76d109 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js @@ -70,6 +70,7 @@ export const enableFabricCompleteRootInCommitPhase = false; export const enableSiblingPrerendering = true; export const enableUseResourceEffectHook = true; export const enableHydrationLaneScheduling = true; +export const enableYieldingBeforePassive = true; // Flow magic to verify the exports of this file match the original version. ((((null: any): ExportsType): FeatureFlagsType): ExportsType); diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index 91eab54c310a4..e7c92602a0abf 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -84,5 +84,7 @@ export const enableUseResourceEffectHook = false; export const enableHydrationLaneScheduling = true; +export const enableYieldingBeforePassive = true; + // Flow magic to verify the exports of this file match the original version. ((((null: any): ExportsType): FeatureFlagsType): ExportsType); diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index a0efa66cb348b..da62754a13359 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -57,6 +57,8 @@ export const enableMoveBefore = false; export const disableInputAttributeSyncing = false; export const enableLegacyFBSupport = true; +export const enableYieldingBeforePassive = false; + export const enableHydrationLaneScheduling = true; export const enableComponentPerformanceTrack = false; diff --git a/packages/use-subscription/src/__tests__/useSubscription-test.js b/packages/use-subscription/src/__tests__/useSubscription-test.js index 4ecc405789f91..6642ab35afb1f 100644 --- a/packages/use-subscription/src/__tests__/useSubscription-test.js +++ b/packages/use-subscription/src/__tests__/useSubscription-test.js @@ -19,6 +19,7 @@ let ReplaySubject; let assertLog; let waitForAll; let waitFor; +let waitForPaint; describe('useSubscription', () => { beforeEach(() => { @@ -37,6 +38,7 @@ describe('useSubscription', () => { const InternalTestUtils = require('internal-test-utils'); waitForAll = InternalTestUtils.waitForAll; + waitForPaint = InternalTestUtils.waitForPaint; assertLog = InternalTestUtils.assertLog; waitFor = InternalTestUtils.waitFor; }); @@ -595,7 +597,7 @@ describe('useSubscription', () => { React.startTransition(() => { mutate('C'); }); - await waitFor(['render:first:C', 'render:second:C']); + await waitForPaint(['render:first:C', 'render:second:C']); React.startTransition(() => { mutate('D'); });