@samitouri / QOS-React-1 / commits / facec3ee71

[Fiber] Schedule passive effects using the regular ensureRootIsScheduled flow (#31785)

This treats workInProgressRoot work and rootWithPendingPassiveEffects the same way. Basically as long as there's some work on the root, yield the current task. Including passive effects. This means that passive effects are now a continuation instead of a separate callback. This can mean they're earlier or later than before. Later for Idle in case there's other non-React work. Earlier for same Default if there's other Default priority work. This makes sense since increasing priority of the passive effects beyond Idle doesn't really make sense for an Idle render. However, for any given render at same priority it's more important to complete this work than start something new. Since we special case continuations to always yield to the browser, this has the same effect as #31784 without implementing `requestPaint`. At least assuming nothing else calls `requestPaint`. <img width="587" alt="Screenshot 2024-12-14 at 5 37 37 PM" src="https://github.com/user-attachments/assets/8641b172-8842-4191-8bf0-50cbe263a30c" />

Sebastian Markbåge committed Dec 17, 2024 at 17:01 UTC facec3ee71fff8b23f1e91005fce730cc96e4021

18 files changed +161 -48

packages/react-dom/src/__tests__/ReactDOMSelect-test.js

+1 -3

@@ -911,9 +911,7 @@ describe('ReactDOMSelect', () => {

911 const container = document.createElement('div');

912 const root = ReactDOMClient.createRoot(container);

913 async function changeView() {

914 - await act(() => {

915 - root.unmount();

916 - });

914 + root.unmount();

915 }

916

917 const stub = (

packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js

@@ -3743,6 +3743,10 @@ describe('ReactDOMServerPartialHydration', () => {

3743 await waitForPaint(['App']);

3744 expect(visibleRef.current).toBe(visibleSpan);

3745

3746 + if (gate(flags => flags.enableYieldingBeforePassive)) {

3747 + // Passive effects.

3748 + await waitForPaint([]);

3749 + }

3750 // Subsequently, the hidden child is prerendered on the client

3751 await waitForPaint(['HiddenChild']);

3752 expect(container).toMatchInlineSnapshot(`

packages/react-reconciler/src/ReactFiberRootScheduler.js

+16 -4

@@ -20,6 +20,7 @@ import {

20 enableProfilerNestedUpdatePhase,

21 enableComponentPerformanceTrack,

22 enableSiblingPrerendering,

23 + enableYieldingBeforePassive,

24 } from 'shared/ReactFeatureFlags';

25 import {

26 NoLane,

@@ -41,6 +42,8 @@ import {

42 getExecutionContext,

43 getWorkInProgressRoot,

44 getWorkInProgressRootRenderLanes,

45 + getRootWithPendingPassiveEffects,

46 + getPendingPassiveEffectsLanes,

47 isWorkLoopSuspendedOnData,

48 performWorkOnRoot,

49 } from './ReactFiberWorkLoop';

@@ -324,12 +327,21 @@ function scheduleTaskForRootDuringMicrotask(

327 markStarvedLanesAsExpired(root, currentTime);

328

329 // Determine the next lanes to work on, and their priority.

330 + const rootWithPendingPassiveEffects = getRootWithPendingPassiveEffects();

331 + const pendingPassiveEffectsLanes = getPendingPassiveEffectsLanes();

332 const workInProgressRoot = getWorkInProgressRoot();

333 const workInProgressRootRenderLanes = getWorkInProgressRootRenderLanes();

329 - const nextLanes = getNextLanes(

330 - root,

331 - root === workInProgressRoot ? workInProgressRootRenderLanes : NoLanes,

332 - );

334 + const nextLanes =

335 + enableYieldingBeforePassive && root === rootWithPendingPassiveEffects

336 + ? // This will schedule the callback at the priority of the lane but we used to

337 + // always schedule it at NormalPriority. Discrete will flush it sync anyway.

338 + // So the only difference is Idle and it doesn't seem necessarily right for that

339 + // to get upgraded beyond something important just because we're past commit.

340 + pendingPassiveEffectsLanes

341 + : getNextLanes(

342 + root,

343 + root === workInProgressRoot ? workInProgressRootRenderLanes : NoLanes,

344 + );

345

346 const existingCallbackNode = root.callbackNode;

347 if (

packages/react-reconciler/src/ReactFiberWorkLoop.js

+54 -22

@@ -40,6 +40,7 @@ import {

40 disableDefaultPropsExceptForClasses,

41 enableSiblingPrerendering,

42 enableComponentPerformanceTrack,

43 + enableYieldingBeforePassive,

44 } from 'shared/ReactFeatureFlags';

45 import ReactSharedInternals from 'shared/ReactSharedInternals';

46 import is from 'shared/objectIs';

@@ -610,7 +611,6 @@ export function getRenderTargetTime(): number {

611

612 let legacyErrorBoundariesThatAlreadyFailed: Set<mixed> | null = null;

613

613 -let rootDoesHavePassiveEffects: boolean = false;

614 let rootWithPendingPassiveEffects: FiberRoot | null = null;

615 let pendingPassiveEffectsLanes: Lanes = NoLanes;

616 let pendingPassiveEffectsRemainingLanes: Lanes = NoLanes;

@@ -638,6 +638,14 @@ export function getWorkInProgressRootRenderLanes(): Lanes {

638 return workInProgressRootRenderLanes;

639 }

640

641 +export function getRootWithPendingPassiveEffects(): FiberRoot | null {

642 + return rootWithPendingPassiveEffects;

643 +}

644 +

645 +export function getPendingPassiveEffectsLanes(): Lanes {

646 + return pendingPassiveEffectsLanes;

647 +}

648 +

649 export function isWorkLoopSuspendedOnData(): boolean {

650 return (

651 workInProgressSuspendedReason === SuspendedOnData ||

@@ -3210,12 +3218,6 @@ function commitRootImpl(

3218 );

3219 }

3220

3213 - // commitRoot never returns a continuation; it always finishes synchronously.

3214 - // So we can clear these now to allow a new callback to be scheduled.

3215 - root.callbackNode = null;

3216 - root.callbackPriority = NoLane;

3217 - root.cancelPendingCommit = null;

3218 -

3221 // Check which lanes no longer have any work scheduled on them, and mark

3222 // those as finished.

3223 let remainingLanes = mergeLanes(finishedWork.lanes, finishedWork.childLanes);

@@ -3253,6 +3255,7 @@ function commitRootImpl(

3255 // might get scheduled in the commit phase. (See #16714.)

3256 // TODO: Delete all other places that schedule the passive effect callback

3257 // They're redundant.

3258 + let rootDoesHavePassiveEffects: boolean = false;

3259 if (

3260 // If this subtree rendered with profiling this commit, we need to visit it to log it.

3261 (enableProfilerTimer &&

@@ -3261,17 +3264,25 @@ function commitRootImpl(

3264 (finishedWork.subtreeFlags & PassiveMask) !== NoFlags ||

3265 (finishedWork.flags & PassiveMask) !== NoFlags

3266 ) {

3264 - if (!rootDoesHavePassiveEffects) {

3265 - rootDoesHavePassiveEffects = true;

3266 - pendingPassiveEffectsRemainingLanes = remainingLanes;

3267 - pendingPassiveEffectsRenderEndTime = completedRenderEndTime;

3268 - // workInProgressTransitions might be overwritten, so we want

3269 - // to store it in pendingPassiveTransitions until they get processed

3270 - // We need to pass this through as an argument to commitRoot

3271 - // because workInProgressTransitions might have changed between

3272 - // the previous render and commit if we throttle the commit

3273 - // with setTimeout

3274 - pendingPassiveTransitions = transitions;

3267 + rootDoesHavePassiveEffects = true;

3268 + pendingPassiveEffectsRemainingLanes = remainingLanes;

3269 + pendingPassiveEffectsRenderEndTime = completedRenderEndTime;

3270 + // workInProgressTransitions might be overwritten, so we want

3271 + // to store it in pendingPassiveTransitions until they get processed

3272 + // We need to pass this through as an argument to commitRoot

3273 + // because workInProgressTransitions might have changed between

3274 + // the previous render and commit if we throttle the commit

3275 + // with setTimeout

3276 + pendingPassiveTransitions = transitions;

3277 + if (enableYieldingBeforePassive) {

3278 + // We don't schedule a separate task for flushing passive effects.

3279 + // Instead, we just rely on ensureRootIsScheduled below to schedule

3280 + // a callback for us to flush the passive effects.

3281 + } else {

3282 + // So we can clear these now to allow a new callback to be scheduled.

3283 + root.callbackNode = null;

3284 + root.callbackPriority = NoLane;

3285 + root.cancelPendingCommit = null;

3286 scheduleCallback(NormalSchedulerPriority, () => {

3287 if (enableProfilerTimer && enableComponentPerformanceTrack) {

3288 // Track the currently executing event if there is one so we can ignore this

@@ -3285,6 +3296,12 @@ function commitRootImpl(

3296 return null;

3297 });

3298 }

3299 + } else {

3300 + // If we don't have passive effects, we're not going to need to perform more work

3301 + // so we can clear the callback now.

3302 + root.callbackNode = null;

3303 + root.callbackPriority = NoLane;

3304 + root.cancelPendingCommit = null;

3305 }

3306

3307 if (enableProfilerTimer) {

@@ -3441,10 +3458,6 @@ function commitRootImpl(

3458 onCommitRootTestSelector();

3459 }

3460

3444 - // Always call this before exiting `commitRoot`, to ensure that any

3445 - // additional work on this root is scheduled.

3446 - ensureRootIsScheduled(root);

3447 -

3461 if (recoverableErrors !== null) {

3462 // There were errors during this render, but recovered from them without

3463 // needing to surface it to the UI. We log them here.

@@ -3480,6 +3493,10 @@ function commitRootImpl(

3493 flushPassiveEffects();

3494 }

3495

3496 + // Always call this before exiting `commitRoot`, to ensure that any

3497 + // additional work on this root is scheduled.

3498 + ensureRootIsScheduled(root);

3499 +

3500 // Read this again, since a passive effect might have updated it

3501 remainingLanes = root.pendingLanes;

3502

@@ -3648,6 +3665,13 @@ function flushPassiveEffectsImpl(wasDelayedCommit: void | boolean) {

3665 // because it's only used for profiling), but it's a refactor hazard.

3666 pendingPassiveEffectsLanes = NoLanes;

3667

3668 + if (enableYieldingBeforePassive) {

3669 + // We've finished our work for this render pass.

3670 + root.callbackNode = null;

3671 + root.callbackPriority = NoLane;

3672 + root.cancelPendingCommit = null;

3673 + }

3674 +

3675 if ((executionContext & (RenderContext | CommitContext)) !== NoContext) {

3676 throw new Error('Cannot flush passive effects while already rendering.');

3677 }

@@ -3745,6 +3769,14 @@ function flushPassiveEffectsImpl(wasDelayedCommit: void | boolean) {

3769 didScheduleUpdateDuringPassiveEffects = false;

3770 }

3771

3772 + if (enableYieldingBeforePassive) {

3773 + // Next, we reschedule any remaining work in a new task since it's a new

3774 + // sequence of work. We wait until the end to do this in case the passive

3775 + // effect schedules higher priority work than we had remaining. That way

3776 + // we don't schedule an early callback that gets cancelled anyway.

3777 + ensureRootIsScheduled(root);

3778 + }

3779 +

3780 // TODO: Move to commitPassiveMountEffects

3781 onPostCommitRootDevTools(root);

3782 if (enableProfilerTimer && enableProfilerCommitHooks) {

packages/react-reconciler/src/__tests__/ReactDeferredValue-test.js

@@ -753,6 +753,10 @@ describe('ReactDeferredValue', () => {

753 revealContent();

754 // Because the preview state was already prerendered, we can reveal it

755 // without any addditional work.

756 + if (gate(flags => flags.enableYieldingBeforePassive)) {

757 + // Passive effects.

758 + await waitForPaint([]);

759 + }

760 await waitForPaint([]);

761 expect(root).toMatchRenderedOutput(<div>Preview [B]</div>);

762 });

packages/react-reconciler/src/__tests__/ReactExpiration-test.js

+9 -3

@@ -755,10 +755,16 @@ describe('ReactExpiration', () => {

755

756 // The update finishes without yielding. But it does not flush the effect.

757 await waitFor(['B1'], {

758 - additionalLogsAfterAttemptingToYield: ['C1'],

758 + additionalLogsAfterAttemptingToYield: gate(

759 + flags => flags.enableYieldingBeforePassive,

760 + )

761 + ? ['C1', 'Effect: 1']

762 + : ['C1'],

763 });

764 });

761 - // The effect flushes after paint.

762 - assertLog(['Effect: 1']);

765 + if (!gate(flags => flags.enableYieldingBeforePassive)) {

766 + // The effect flushes after paint.

767 + assertLog(['Effect: 1']);

768 + }

769 });

770 });

packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js

+17 -7

@@ -2695,13 +2695,23 @@ describe('ReactHooksWithNoopRenderer', () => {

2695 React.startTransition(() => {

2696 ReactNoop.render(<Counter count={1} />);

2697 });

2698 - await waitForPaint([

2699 - 'Create passive [current: 0]',

2700 - 'Destroy insertion [current: 0]',

2701 - 'Create insertion [current: 0]',

2702 - 'Destroy layout [current: 1]',

2703 - 'Create layout [current: 1]',

2704 - ]);

2698 + if (gate(flags => flags.enableYieldingBeforePassive)) {

2699 + await waitForPaint(['Create passive [current: 0]']);

2700 + await waitForPaint([

2701 + 'Destroy insertion [current: 0]',

2702 + 'Create insertion [current: 0]',

2703 + 'Destroy layout [current: 1]',

2704 + 'Create layout [current: 1]',

2705 + ]);

2706 + } else {

2707 + await waitForPaint([

2708 + 'Create passive [current: 0]',

2709 + 'Destroy insertion [current: 0]',

2710 + 'Create insertion [current: 0]',

2711 + 'Destroy layout [current: 1]',

2712 + 'Create layout [current: 1]',

2713 + ]);

2714 + }

2715 expect(committedText).toEqual('1');

2716 });

2717 assertLog([

packages/react-reconciler/src/__tests__/ReactSiblingPrerendering-test.js

+16

@@ -200,6 +200,10 @@ describe('ReactSiblingPrerendering', () => {

200 await waitForPaint(['A']);

201 expect(root).toMatchRenderedOutput('A');

202

203 + if (gate(flags => flags.enableYieldingBeforePassive)) {

204 + // Passive effects.

205 + await waitForPaint([]);

206 + }

207 // The second render is a prerender of the hidden content.

208 await waitForPaint([

209 'Suspend! [B]',

@@ -237,6 +241,10 @@ describe('ReactSiblingPrerendering', () => {

241 // Immediately after the fallback commits, retry the boundary again. This

242 // time we include B, since we're not blocking the fallback from showing.

243 if (gate('enableSiblingPrerendering')) {

244 + if (gate(flags => flags.enableYieldingBeforePassive)) {

245 + // Passive effects.

246 + await waitForPaint([]);

247 + }

248 await waitForPaint(['Suspend! [A]', 'Suspend! [B]']);

249 }

250 });

@@ -452,6 +460,10 @@ describe('ReactSiblingPrerendering', () => {

460 </>,

461 );

462

463 + if (gate(flags => flags.enableYieldingBeforePassive)) {

464 + // Passive effects.

465 + await waitForPaint([]);

466 + }

467 // Immediately after the fallback commits, retry the boundary again.

468 // Because the promise for A resolved, this is a normal render, _not_

469 // a prerender. So when we proceed to B, and B suspends, we unwind again

@@ -471,6 +483,10 @@ describe('ReactSiblingPrerendering', () => {

483 </>,

484 );

485

486 + if (gate(flags => flags.enableYieldingBeforePassive)) {

487 + // Passive effects.

488 + await waitForPaint([]);

489 + }

490 // Now we can proceed to prerendering C.

491 if (gate('enableSiblingPrerendering')) {

492 await waitForPaint(['Suspend! [B]', 'Suspend! [C]']);

packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js

@@ -1901,6 +1901,10 @@ describe('ReactSuspenseWithNoopRenderer', () => {

1901 // be throttled because the fallback would have appeared too recently.

1902 Scheduler.unstable_advanceTime(10000);

1903 jest.advanceTimersByTime(10000);

1904 + if (gate(flags => flags.enableYieldingBeforePassive)) {

1905 + // Passive effects.

1906 + await waitForPaint([]);

1907 + }

1908 await waitForPaint(['A']);

1909 expect(ReactNoop).toMatchRenderedOutput(

1910 <>

packages/react-reconciler/src/__tests__/ReactUpdatePriority-test.js

+20 -8

@@ -81,14 +81,26 @@ describe('ReactUpdatePriority', () => {

81 // Schedule another update at default priority

82 setDefaultState(2);

84 - // The default update flushes first, because

85 - await waitForPaint([

86 - // Idle update is scheduled

87 - 'Idle update',

88 -

89 - // The default update flushes first, without including the idle update

90 - 'Idle: 1, Default: 2',

91 - ]);

84 + if (gate(flags => flags.enableYieldingBeforePassive)) {

85 + // The default update flushes first, because

86 + await waitForPaint([

87 + // Idle update is scheduled

88 + 'Idle update',

89 + ]);

90 + await waitForPaint([

91 + // The default update flushes first, without including the idle update

92 + 'Idle: 1, Default: 2',

93 + ]);

94 + } else {

95 + // The default update flushes first, because

96 + await waitForPaint([

97 + // Idle update is scheduled

98 + 'Idle update',

99 +

100 + // The default update flushes first, without including the idle update

101 + 'Idle: 1, Default: 2',

102 + ]);

103 + }

104 });

105 // Now the idle update has flushed

106 assertLog(['Idle: 2, Default: 2']);

packages/shared/ReactFeatureFlags.js

@@ -77,6 +77,9 @@ export const enableLegacyFBSupport = false;

77 // likely to include in an upcoming release.

78 // -----------------------------------------------------------------------------

80 +// Yield to the browser event loop and not just the scheduler event loop before passive effects.

81 +export const enableYieldingBeforePassive = __EXPERIMENTAL__;

82 +

83 export const enableLegacyCache = __EXPERIMENTAL__;

85 export const enableAsyncIterableChildren = __EXPERIMENTAL__;

packages/shared/forks/ReactFeatureFlags.native-fb.js

@@ -82,6 +82,7 @@ export const syncLaneExpirationMs = 250;

82 export const transitionLaneExpirationMs = 5000;

83 export const useModernStrictMode = true;

84 export const enableHydrationLaneScheduling = true;

85 +export const enableYieldingBeforePassive = false;

87 // Flow magic to verify the exports of this file match the original version.

88 ((((null: any): ExportsType): FeatureFlagsType): ExportsType);

packages/shared/forks/ReactFeatureFlags.native-oss.js

@@ -76,6 +76,8 @@ export const enableUseResourceEffectHook = false;

77 export const enableHydrationLaneScheduling = true;

79 +export const enableYieldingBeforePassive = false;

80 +

81 // Profiling Only

82 export const enableProfilerTimer = __PROFILE__;

83 export const enableProfilerCommitHooks = __PROFILE__;

packages/shared/forks/ReactFeatureFlags.test-renderer.js

@@ -72,6 +72,8 @@ export const enableSiblingPrerendering = true;

73 export const enableUseResourceEffectHook = false;

75 +export const enableYieldingBeforePassive = true;

76 +

77 // TODO: This must be in sync with the main ReactFeatureFlags file because

78 // the Test Renderer's value must be the same as the one used by the

79 // react package.

packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js

@@ -70,6 +70,7 @@ export const enableFabricCompleteRootInCommitPhase = false;

70 export const enableSiblingPrerendering = true;

71 export const enableUseResourceEffectHook = true;

72 export const enableHydrationLaneScheduling = true;

73 +export const enableYieldingBeforePassive = true;

75 // Flow magic to verify the exports of this file match the original version.

76 ((((null: any): ExportsType): FeatureFlagsType): ExportsType);

packages/shared/forks/ReactFeatureFlags.test-renderer.www.js

@@ -84,5 +84,7 @@ export const enableUseResourceEffectHook = false;

85 export const enableHydrationLaneScheduling = true;

87 +export const enableYieldingBeforePassive = true;

88 +

89 // Flow magic to verify the exports of this file match the original version.

90 ((((null: any): ExportsType): FeatureFlagsType): ExportsType);

packages/shared/forks/ReactFeatureFlags.www.js

@@ -57,6 +57,8 @@ export const enableMoveBefore = false;

57 export const disableInputAttributeSyncing = false;

58 export const enableLegacyFBSupport = true;

60 +export const enableYieldingBeforePassive = false;

61 +

62 export const enableHydrationLaneScheduling = true;

64 export const enableComponentPerformanceTrack = false;

packages/use-subscription/src/__tests__/useSubscription-test.js

+3 -1

@@ -19,6 +19,7 @@ let ReplaySubject;

19 let assertLog;

20 let waitForAll;

21 let waitFor;

22 +let waitForPaint;

24 describe('useSubscription', () => {

25 beforeEach(() => {

@@ -37,6 +38,7 @@ describe('useSubscription', () => {

39 const InternalTestUtils = require('internal-test-utils');

40 waitForAll = InternalTestUtils.waitForAll;

41 + waitForPaint = InternalTestUtils.waitForPaint;

42 assertLog = InternalTestUtils.assertLog;

43 waitFor = InternalTestUtils.waitFor;

44 });

@@ -595,7 +597,7 @@ describe('useSubscription', () => {

597 React.startTransition(() => {

598 mutate('C');

599 });

598 - await waitFor(['render:first:C', 'render:second:C']);

600 + await waitForPaint(['render:first:C', 'render:second:C']);

601 React.startTransition(() => {

602 mutate('D');

603 });