Skip to content

Commit

Permalink
Remove disableSchedulerTimeoutInWorkLoop flag (#19902)
Browse files Browse the repository at this point in the history
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.
acdlite authored Sep 28, 2020

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
1 parent 71bc8ac commit ba82eea
Showing 12 changed files with 2 additions and 39 deletions.
15 changes: 1 addition & 14 deletions packages/react-reconciler/src/ReactFiberWorkLoop.new.js
Original file line number Diff line number Diff line change
@@ -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 (
15 changes: 1 addition & 14 deletions packages/react-reconciler/src/ReactFiberWorkLoop.old.js
Original file line number Diff line number Diff line change
@@ -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 (
2 changes: 0 additions & 2 deletions packages/shared/ReactFeatureFlags.js
Original file line number Diff line number Diff line change
@@ -135,6 +135,4 @@ export const enableDiscreteEventFlushingChange = false;

export const enableEagerRootListeners = true;

export const disableSchedulerTimeoutInWorkLoop = false;

export const enableDoubleInvokingEffects = false;
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.native-fb.js
Original file line number Diff line number Diff line change
@@ -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;

1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.native-oss.js
Original file line number Diff line number Diff line change
@@ -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;

1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.test-renderer.js
Original file line number Diff line number Diff line change
@@ -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;

Original file line number Diff line number Diff line change
@@ -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;

Original file line number Diff line number Diff line change
@@ -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;

1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.testing.js
Original file line number Diff line number Diff line change
@@ -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;

1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.testing.www.js
Original file line number Diff line number Diff line change
@@ -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;

1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.www-dynamic.js
Original file line number Diff line number Diff line change
@@ -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;
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.www.js
Original file line number Diff line number Diff line change
@@ -28,7 +28,6 @@ export const {
enableDebugTracing,
skipUnmountedBoundaries,
enableEagerRootListeners,
disableSchedulerTimeoutInWorkLoop,
enableDoubleInvokingEffects,
} = dynamicFeatureFlags;

0 comments on commit ba82eea

Please sign in to comment.