From 0bd3dc3ec1fae1ca4b78cf5a76a61c7af5d11966 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Wed, 29 Mar 2023 18:52:51 -0400 Subject: [PATCH] Add flag to disable behavior change --- ...MServerSelectiveHydration-test.internal.js | 1 + .../src/ReactFiberRootScheduler.js | 36 ++++++++++++++++--- .../__tests__/ReactProfiler-test.internal.js | 17 ++++++++- packages/shared/ReactFeatureFlags.js | 4 +++ .../ReactFeatureFlags.native-fb-dynamic.js | 1 + .../forks/ReactFeatureFlags.native-fb.js | 3 +- .../forks/ReactFeatureFlags.native-oss.js | 1 + .../forks/ReactFeatureFlags.test-renderer.js | 1 + .../ReactFeatureFlags.test-renderer.www.js | 1 + .../forks/ReactFeatureFlags.www-dynamic.js | 1 + .../shared/forks/ReactFeatureFlags.www.js | 1 + scripts/flow/xplat.js | 1 + 12 files changed, 62 insertions(+), 6 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js index 31d3bba687d38..55f4a8d46468a 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js @@ -1067,6 +1067,7 @@ describe('ReactDOMServerSelectiveHydration', () => { assertLog(['Inner Mouse Enter', 'Outer Mouse Enter']); }); + // @gate enableDeferRootSchedulingToMicrotask it('Outer hydrates first then Inner', async () => { dispatchMouseHoverEvent(innerDiv); diff --git a/packages/react-reconciler/src/ReactFiberRootScheduler.js b/packages/react-reconciler/src/ReactFiberRootScheduler.js index 61979266ce121..9a65b5b9d75f9 100644 --- a/packages/react-reconciler/src/ReactFiberRootScheduler.js +++ b/packages/react-reconciler/src/ReactFiberRootScheduler.js @@ -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, @@ -41,6 +42,7 @@ import { cancelCallback as Scheduler_cancelCallback, scheduleCallback as Scheduler_scheduleCallback, now, + ImmediatePriority, } from './Scheduler'; import { DiscreteEventPriority, @@ -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. @@ -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( @@ -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. diff --git a/packages/react/src/__tests__/ReactProfiler-test.internal.js b/packages/react/src/__tests__/ReactProfiler-test.internal.js index e4fd2a323b3e0..de6b7f973e66b 100644 --- a/packages/react/src/__tests__/ReactProfiler-test.internal.js +++ b/packages/react/src/__tests__/ReactProfiler-test.internal.js @@ -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')); diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 08cbbe44e8051..bd8de7f3d44a1 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -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) // diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js b/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js index b7bfa74d1590f..2270b18ae3340 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js @@ -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); diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index e1236b2e4e167..44cf63f3b561a 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -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; diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 9c3d1146fe1a0..cab02515544bd 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -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); diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 21b6f11c1164d..408b539c1bf15 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -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); diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index 780e87a322397..2602acd9f8bfb 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -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); diff --git a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js index 75dc8b4e65105..6af12cb579094 100644 --- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js @@ -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. diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 85e4f3219436e..cd6c09bf30df2 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -28,6 +28,7 @@ export const { enableUnifiedSyncLane, enableTransitionTracing, enableCustomElementPropertySupport, + enableDeferRootSchedulingToMicrotask, } = dynamicFeatureFlags; // On WWW, __EXPERIMENTAL__ is used for a new modern build. diff --git a/scripts/flow/xplat.js b/scripts/flow/xplat.js index 5a0d26b66f667..8e14cf5a04185 100644 --- a/scripts/flow/xplat.js +++ b/scripts/flow/xplat.js @@ -9,4 +9,5 @@ declare module 'ReactNativeInternalFeatureFlags' { declare export var enableUseRefAccessWarning: boolean; + declare export var enableDeferRootSchedulingToMicrotask: boolean; }