From ba82eea3837e4aaeb5a30b7827b664a8c2128d2e Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 28 Sep 2020 12:19:14 -0500 Subject: [PATCH] Remove disableSchedulerTimeoutInWorkLoop flag (#19902) We found and mitigated the root cause of the regression that led us to temporarily revert this change. So now I'm un-reverting it. --- .../src/ReactFiberWorkLoop.new.js | 15 +-------------- .../src/ReactFiberWorkLoop.old.js | 15 +-------------- packages/shared/ReactFeatureFlags.js | 2 -- .../shared/forks/ReactFeatureFlags.native-fb.js | 1 - .../shared/forks/ReactFeatureFlags.native-oss.js | 1 - .../forks/ReactFeatureFlags.test-renderer.js | 1 - .../ReactFeatureFlags.test-renderer.native.js | 1 - .../forks/ReactFeatureFlags.test-renderer.www.js | 1 - .../shared/forks/ReactFeatureFlags.testing.js | 1 - .../shared/forks/ReactFeatureFlags.testing.www.js | 1 - .../shared/forks/ReactFeatureFlags.www-dynamic.js | 1 - packages/shared/forks/ReactFeatureFlags.www.js | 1 - 12 files changed, 2 insertions(+), 39 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index f9439601f5c50..bd8ef8ce5e3c6 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -30,7 +30,6 @@ import { enableSchedulingProfiler, enableScopeAPI, skipUnmountedBoundaries, - disableSchedulerTimeoutInWorkLoop, enableDoubleInvokingEffects, } from 'shared/ReactFeatureFlags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; @@ -745,7 +744,7 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) { // This is the entry point for every concurrent task, i.e. anything that // goes through Scheduler. -function performConcurrentWorkOnRoot(root, didTimeout) { +function performConcurrentWorkOnRoot(root) { // Since we know we're in a React event, we can clear the current // event time. The next update will compute a new event time. currentEventTime = NoTimestamp; @@ -785,18 +784,6 @@ function performConcurrentWorkOnRoot(root, didTimeout) { return null; } - // TODO: We only check `didTimeout` defensively, to account for a Scheduler - // bug we're still investigating. Once the bug in Scheduler is fixed, - // we can remove this, since we track expiration ourselves. - if (!disableSchedulerTimeoutInWorkLoop && didTimeout) { - // Something expired. Flush synchronously until there's no expired - // work left. - markRootExpired(root, lanes); - // This will schedule a synchronous callback. - ensureRootIsScheduled(root, now()); - return null; - } - let exitStatus = renderRootConcurrent(root, lanes); if ( diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index 49de7469eda68..4d0ee94c82be9 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -30,7 +30,6 @@ import { enableSchedulingProfiler, enableScopeAPI, skipUnmountedBoundaries, - disableSchedulerTimeoutInWorkLoop, } from 'shared/ReactFeatureFlags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import invariant from 'shared/invariant'; @@ -739,7 +738,7 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) { // This is the entry point for every concurrent task, i.e. anything that // goes through Scheduler. -function performConcurrentWorkOnRoot(root, didTimeout) { +function performConcurrentWorkOnRoot(root) { // Since we know we're in a React event, we can clear the current // event time. The next update will compute a new event time. currentEventTime = NoTimestamp; @@ -779,18 +778,6 @@ function performConcurrentWorkOnRoot(root, didTimeout) { return null; } - // TODO: We only check `didTimeout` defensively, to account for a Scheduler - // bug we're still investigating. Once the bug in Scheduler is fixed, - // we can remove this, since we track expiration ourselves. - if (!disableSchedulerTimeoutInWorkLoop && didTimeout) { - // Something expired. Flush synchronously until there's no expired - // work left. - markRootExpired(root, lanes); - // This will schedule a synchronous callback. - ensureRootIsScheduled(root, now()); - return null; - } - let exitStatus = renderRootConcurrent(root, lanes); if ( diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 1d2516ab84385..1302c0f6ff7f6 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -135,6 +135,4 @@ export const enableDiscreteEventFlushingChange = false; export const enableEagerRootListeners = true; -export const disableSchedulerTimeoutInWorkLoop = false; - export const enableDoubleInvokingEffects = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 62669046467f4..bd4a939be4329 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -50,7 +50,6 @@ export const deferRenderPhaseUpdateToNextBatch = true; export const decoupleUpdatePriorityFromScheduler = false; export const enableDiscreteEventFlushingChange = false; export const enableEagerRootListeners = true; -export const disableSchedulerTimeoutInWorkLoop = false; export const enableDoubleInvokingEffects = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index ab03949d136d3..7a120940842d0 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -49,7 +49,6 @@ export const deferRenderPhaseUpdateToNextBatch = true; export const decoupleUpdatePriorityFromScheduler = false; export const enableDiscreteEventFlushingChange = false; export const enableEagerRootListeners = true; -export const disableSchedulerTimeoutInWorkLoop = false; export const enableDoubleInvokingEffects = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 14d831d5b3049..6804c1923bf81 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -49,7 +49,6 @@ export const deferRenderPhaseUpdateToNextBatch = true; export const decoupleUpdatePriorityFromScheduler = false; export const enableDiscreteEventFlushingChange = false; export const enableEagerRootListeners = true; -export const disableSchedulerTimeoutInWorkLoop = false; export const enableDoubleInvokingEffects = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js index bdf71ac54b3ee..f4ccc11a08cfe 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js @@ -49,7 +49,6 @@ export const deferRenderPhaseUpdateToNextBatch = true; export const decoupleUpdatePriorityFromScheduler = false; export const enableDiscreteEventFlushingChange = false; export const enableEagerRootListeners = true; -export const disableSchedulerTimeoutInWorkLoop = false; export const enableDoubleInvokingEffects = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index c4bb6b4e3c54f..b4b7a56097c7e 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -49,7 +49,6 @@ export const deferRenderPhaseUpdateToNextBatch = true; export const decoupleUpdatePriorityFromScheduler = false; export const enableDiscreteEventFlushingChange = false; export const enableEagerRootListeners = true; -export const disableSchedulerTimeoutInWorkLoop = false; export const enableDoubleInvokingEffects = false; diff --git a/packages/shared/forks/ReactFeatureFlags.testing.js b/packages/shared/forks/ReactFeatureFlags.testing.js index 9710569716ccd..f28ad3567e0e8 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.js @@ -49,7 +49,6 @@ export const deferRenderPhaseUpdateToNextBatch = true; export const decoupleUpdatePriorityFromScheduler = false; export const enableDiscreteEventFlushingChange = false; export const enableEagerRootListeners = true; -export const disableSchedulerTimeoutInWorkLoop = false; export const enableDoubleInvokingEffects = false; diff --git a/packages/shared/forks/ReactFeatureFlags.testing.www.js b/packages/shared/forks/ReactFeatureFlags.testing.www.js index ae72c2ae4f956..a962b280a23a7 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.www.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.www.js @@ -49,7 +49,6 @@ export const deferRenderPhaseUpdateToNextBatch = true; export const decoupleUpdatePriorityFromScheduler = false; export const enableDiscreteEventFlushingChange = true; export const enableEagerRootListeners = true; -export const disableSchedulerTimeoutInWorkLoop = false; export const enableDoubleInvokingEffects = false; diff --git a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js index 838f478b218b4..aa3cf1a64a848 100644 --- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js @@ -47,6 +47,5 @@ export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__; // to __VARIANT__. export const enableTrustedTypesIntegration = false; export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; -export const disableSchedulerTimeoutInWorkLoop = __VARIANT__; export const enableDoubleInvokingEffects = false; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 3bb82b29e91fa..ea9610713d057 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -28,7 +28,6 @@ export const { enableDebugTracing, skipUnmountedBoundaries, enableEagerRootListeners, - disableSchedulerTimeoutInWorkLoop, enableDoubleInvokingEffects, } = dynamicFeatureFlags;