From 4de5afdd3f23deea7b58f23949324f13c118b9a1 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 | 38 +++++++++---------- .../src/ReactFiberRootScheduler.js | 12 ++++++ .../__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, 58 insertions(+), 23 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..6ec4ce1c3f75e 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js @@ -1072,53 +1072,49 @@ describe('ReactDOMServerSelectiveHydration', () => { await act(async () => { resolveOuter(); - await outerPromise; - Scheduler.unstable_flushAllWithoutAsserting(); - OuterScheduler.unstable_flushAllWithoutAsserting(); - InnerScheduler.unstable_flushAllWithoutAsserting(); }); + await act(async () => Scheduler.unstable_flushAllWithoutAsserting()); + await act(async () => OuterScheduler.unstable_flushAllWithoutAsserting()); + await act(async () => InnerScheduler.unstable_flushAllWithoutAsserting()); // Outer resolves and scheduled replay OuterTestUtils.assertLog(['Outer']); // Inner App is still blocked - InnerTestUtils.assertLog([]); + InnerTestUtils.assertLog(['Suspend Inner']); // Replay outer event - await act(() => { - Scheduler.unstable_flushAllWithoutAsserting(); - OuterScheduler.unstable_flushAllWithoutAsserting(); - InnerScheduler.unstable_flushAllWithoutAsserting(); - }); + await act(async () => Scheduler.unstable_flushAllWithoutAsserting()); + await act(async () => OuterScheduler.unstable_flushAllWithoutAsserting()); + await act(async () => InnerScheduler.unstable_flushAllWithoutAsserting()); // Inner is still blocked so when Outer replays the event in capture phase // inner ends up caling stopPropagation assertLog([]); OuterTestUtils.assertLog([]); - InnerTestUtils.assertLog(['Suspend Inner']); + InnerTestUtils.assertLog([]); - dispatchMouseHoverEvent(innerDiv); + await act(async () => { + dispatchMouseHoverEvent(innerDiv); + }); OuterTestUtils.assertLog([]); InnerTestUtils.assertLog([]); assertLog([]); await act(async () => { resolveInner(); - await innerPromise; - Scheduler.unstable_flushAllWithoutAsserting(); - OuterScheduler.unstable_flushAllWithoutAsserting(); - InnerScheduler.unstable_flushAllWithoutAsserting(); }); + await act(async () => Scheduler.unstable_flushAllWithoutAsserting()); + await act(async () => OuterScheduler.unstable_flushAllWithoutAsserting()); + await act(async () => InnerScheduler.unstable_flushAllWithoutAsserting()); // Inner hydrates InnerTestUtils.assertLog(['Inner']); // Outer was hydrated earlier OuterTestUtils.assertLog([]); - await act(() => { - Scheduler.unstable_flushAllWithoutAsserting(); - OuterScheduler.unstable_flushAllWithoutAsserting(); - InnerScheduler.unstable_flushAllWithoutAsserting(); - }); + await act(async () => Scheduler.unstable_flushAllWithoutAsserting()); + await act(async () => OuterScheduler.unstable_flushAllWithoutAsserting()); + await act(async () => InnerScheduler.unstable_flushAllWithoutAsserting()); // First Inner Mouse Enter fires then Outer Mouse Enter assertLog(['Inner Mouse Enter', 'Outer Mouse Enter']); diff --git a/packages/react-reconciler/src/ReactFiberRootScheduler.js b/packages/react-reconciler/src/ReactFiberRootScheduler.js index 61979266ce121..deed7b83020ab 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, @@ -111,6 +112,14 @@ export function ensureRootIsScheduled(root: FiberRoot): void { scheduleImmediateTask(processRootScheduleInMicrotask); } } + + if (!enableDeferRootSchedulingToMicrotask) { + // While this flag is disabled, we schedule the render task immediately + // instead of waiting a microtask. + // TODO: We need to land enableDeferRootSchedulingToMicrotask ASAP to + // unblock additional features we have planned. + scheduleTaskForRootDuringMicrotask(root, now()); + } } // TODO: Rename to something else. This isn't a generic callback queue anymore. @@ -267,6 +276,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; }