Skip to content

Commit

Permalink
Add flag to disable behavior change
Browse files Browse the repository at this point in the history
  • Loading branch information
acdlite committed Mar 30, 2023
1 parent 267bee9 commit 0bd3dc3
Show file tree
Hide file tree
Showing 12 changed files with 62 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1067,6 +1067,7 @@ describe('ReactDOMServerSelectiveHydration', () => {
assertLog(['Inner Mouse Enter', 'Outer Mouse Enter']);
});

// @gate enableDeferRootSchedulingToMicrotask
it('Outer hydrates first then Inner', async () => {
dispatchMouseHoverEvent(innerDiv);

Expand Down
36 changes: 32 additions & 4 deletions packages/react-reconciler/src/ReactFiberRootScheduler.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type {FiberRoot} from './ReactInternalTypes';
import type {Lane} from './ReactFiberLane';
import type {PriorityLevel} from 'scheduler/src/SchedulerPriorities';

import {enableDeferRootSchedulingToMicrotask} from 'shared/ReactFeatureFlags';
import {
NoLane,
NoLanes,
Expand Down Expand Up @@ -41,6 +42,7 @@ import {
cancelCallback as Scheduler_cancelCallback,
scheduleCallback as Scheduler_scheduleCallback,
now,
ImmediatePriority,
} from './Scheduler';
import {
DiscreteEventPriority,
Expand Down Expand Up @@ -111,14 +113,30 @@ export function ensureRootIsScheduled(root: FiberRoot): void {
scheduleImmediateTask(processRootScheduleInMicrotask);
}
}

if (!enableDeferRootSchedulingToMicrotask) {
// While this flag is disabled, we schedule the task immediately instead
// of waiting for the microtask to fire.
// TODO: We need to land enableDeferRootSchedulingToMicrotask ASAP to
// unblock additional features we have planned.
scheduleTaskForRootDuringMicrotask(root, now());
if (mightHavePendingSyncWork) {
// Before enableDeferRootSchedulingToMicrotask, we used to flush
// synchronous work in a macrotask instead of a microtask. (Except for
// discrete events or flushSync, which will force this to run
// even sooner.)
scheduleCallback(ImmediatePriority, flushSyncCallbacks);
}
}
}

// TODO: Rename to something else. This isn't a generic callback queue anymore.
// I only kept the existing name to reduce noise in the initial PR diff.
export function flushSyncCallbacks() {
export function flushSyncCallbacks(): null {
// This is allowed to be called synchronously, but the caller should check
// the execution context first.
flushSyncWorkAcrossRoots_impl(false);
return null;
}

// TODO: Rename to something else. This isn't a generic callback queue anymore.
Expand Down Expand Up @@ -254,9 +272,16 @@ function processRootScheduleInMicrotask() {
root = next;
}

// At the end of the microtask, flush any pending synchronous work. This has
// to come at the end, because it does actual rendering work that might throw.
flushSyncCallbacks();
if (enableDeferRootSchedulingToMicrotask) {
// At the end of the microtask, flush any pending synchronous work. This has
// to come at the end, because it does actual rendering work that might throw.
flushSyncCallbacks();
} else {
// Before enableDeferRootSchedulingToMicrotask, we used to flush synchronous
// work in a macrotask instead of a microtask. (Except for discrete events
// or flushSync, which will force this to run even sooner.)
scheduleCallback(ImmediatePriority, flushSyncCallbacks);
}
}

function scheduleTaskForRootDuringMicrotask(
Expand All @@ -267,6 +292,9 @@ function scheduleTaskForRootDuringMicrotask(
// rendering task right before we yield to the main thread. It should never be
// called synchronously.
//
// TODO: Unless enableDeferRootSchedulingToMicrotask is off. We need to land
// that ASAP to unblock additional features we have planned.
//
// This function also never performs React work synchronously; it should
// only schedule work to be performed later, in a separate task or microtask.

Expand Down
17 changes: 16 additions & 1 deletion packages/react/src/__tests__/ReactProfiler-test.internal.js
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,22 @@ describe(`onRender`, () => {

// TODO: unstable_now is called by more places than just the profiler.
// Rewrite this test so it's less fragile.
assertLog(['read current time', 'read current time', 'read current time']);
if (gate(flags => flags.enableDeferRootSchedulingToMicrotask)) {
assertLog([
'read current time',
'read current time',
'read current time',
]);
} else {
assertLog([
'read current time',
'read current time',
'read current time',
'read current time',
'read current time',
'read current time',
]);
}

// Restore original mock
jest.mock('scheduler', () => jest.requireActual('scheduler/unstable_mock'));
Expand Down
4 changes: 4 additions & 0 deletions packages/shared/ReactFeatureFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ export const enableSchedulerDebugging = false;
// Need to remove didTimeout argument from Scheduler before landing
export const disableSchedulerTimeoutInWorkLoop = false;

// This will break some internal tests at Meta so we need to gate this until
// those can be fixed.
export const enableDeferRootSchedulingToMicrotask = true;

// -----------------------------------------------------------------------------
// Slated for removal in the future (significant effort)
//
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import typeof * as DynamicFlagsType from 'ReactNativeInternalFeatureFlags';
// update the test configuration.

export const enableUseRefAccessWarning = __VARIANT__;
export const enableDeferRootSchedulingToMicrotask = __VARIANT__;

// Flow magic to verify the exports of this file match the original version.
((((null: any): ExportsType): DynamicFlagsType): ExportsType);
3 changes: 2 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.native-fb.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ import * as dynamicFlags from 'ReactNativeInternalFeatureFlags';

// We destructure each value before re-exporting to avoid a dynamic look-up on
// the exports object every time a flag is read.
export const {enableUseRefAccessWarning} = dynamicFlags;
export const {enableUseRefAccessWarning, enableDeferRootSchedulingToMicrotask} =
dynamicFlags;

// The rest of the flags are static for better dead code elimination.
export const enableDebugTracing = false;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-oss.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ export const enableHostSingletons = true;

export const useModernStrictMode = false;
export const enableFizzExternalRuntime = false;
export const enableDeferRootSchedulingToMicrotask = true;

// Flow magic to verify the exports of this file match the original version.
((((null: any): ExportsType): FeatureFlagsType): ExportsType);
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.test-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ export const enableHostSingletons = true;

export const useModernStrictMode = false;
export const enableFizzExternalRuntime = false;
export const enableDeferRootSchedulingToMicrotask = true;

// Flow magic to verify the exports of this file match the original version.
((((null: any): ExportsType): FeatureFlagsType): ExportsType);
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ export const enableHostSingletons = true;

export const useModernStrictMode = false;
export const enableFizzExternalRuntime = false;
export const enableDeferRootSchedulingToMicrotask = true;

// Flow magic to verify the exports of this file match the original version.
((((null: any): ExportsType): FeatureFlagsType): ExportsType);
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.www-dynamic.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export const enableLazyContextPropagation = __VARIANT__;
export const enableUnifiedSyncLane = __VARIANT__;
export const enableTransitionTracing = __VARIANT__;
export const enableCustomElementPropertySupport = __VARIANT__;
export const enableDeferRootSchedulingToMicrotask = __VARIANT__;

// Enable this flag to help with concurrent mode debugging.
// It logs information to the console about React scheduling, rendering, and commit phases.
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.www.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export const {
enableUnifiedSyncLane,
enableTransitionTracing,
enableCustomElementPropertySupport,
enableDeferRootSchedulingToMicrotask,
} = dynamicFeatureFlags;

// On WWW, __EXPERIMENTAL__ is used for a new modern build.
Expand Down
1 change: 1 addition & 0 deletions scripts/flow/xplat.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,5 @@

declare module 'ReactNativeInternalFeatureFlags' {
declare export var enableUseRefAccessWarning: boolean;
declare export var enableDeferRootSchedulingToMicrotask: boolean;
}

0 comments on commit 0bd3dc3

Please sign in to comment.