From 9d756d903fdd07895fc320962c3fab95fa099bd5 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 1 Mar 2019 13:10:12 -0800 Subject: [PATCH] Revert #14756 changes to ReactFiberScheduler (#14992) * Revert #14756 changes to ReactFiberScheduler This PR introduced some bugs in concurrent mode during internal testing. Until we figure out a proper solution, I'm going to try reverting it. Not totally certain this is sufficient to unbreak the bugs we found, but I'm using this branch to determine that. * Add back commented out Scheduler import With a note not to use named imports next time we import Scheduler in this module. --- .../src/ReactFiberScheduler.js | 153 +++++++++--------- 1 file changed, 81 insertions(+), 72 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberScheduler.js b/packages/react-reconciler/src/ReactFiberScheduler.js index e09ceb403027f..be5062139ab13 100644 --- a/packages/react-reconciler/src/ReactFiberScheduler.js +++ b/packages/react-reconciler/src/ReactFiberScheduler.js @@ -12,12 +12,16 @@ import type {Batch, FiberRoot} from './ReactFiberRoot'; import type {ExpirationTime} from './ReactFiberExpirationTime'; import type {Interaction} from 'scheduler/src/Tracing'; +// Intentionally not named imports because Rollup would use dynamic dispatch for +// CommonJS interop named imports. +// TODO: We're not using this import anymore, but I've left this here so we +// don't accidentally use named imports when we add it back. +// import * as Scheduler from 'scheduler'; import { __interactionsRef, __subscriberRef, unstable_wrap as Scheduler_tracing_wrap, } from 'scheduler/tracing'; -import * as Scheduler from 'scheduler'; import { invokeGuardedCallback, hasCaughtError, @@ -126,7 +130,7 @@ import { computeAsyncExpiration, computeInteractiveExpiration, } from './ReactFiberExpirationTime'; -import {ConcurrentMode, ProfileMode, NoContext} from './ReactTypeOfMode'; +import {ConcurrentMode, ProfileMode} from './ReactTypeOfMode'; import {enqueueUpdate, resetCurrentlyProcessingQueue} from './ReactUpdateQueue'; import {createCapturedValue} from './ReactCapturedValue'; import { @@ -172,19 +176,6 @@ export type Thenable = { then(resolve: () => mixed, reject?: () => mixed): mixed, }; -// Intentionally not named imports because Rollup would -// use dynamic dispatch for CommonJS interop named imports. -const { - unstable_next: Scheduler_next, - unstable_getCurrentPriorityLevel: getCurrentPriorityLevel, - unstable_runWithPriority: runWithPriority, - unstable_ImmediatePriority: ImmediatePriority, - unstable_UserBlockingPriority: UserBlockingPriority, - unstable_NormalPriority: NormalPriority, - unstable_LowPriority: LowPriority, - unstable_IdlePriority: IdlePriority, -} = Scheduler; - const {ReactCurrentDispatcher, ReactCurrentOwner} = ReactSharedInternals; let didWarnAboutStateTransition; @@ -259,6 +250,11 @@ if (__DEV__) { // Used to ensure computeUniqueAsyncExpiration is monotonically decreasing. let lastUniqueAsyncExpiration: number = Sync - 1; +// Represents the expiration time that incoming updates should use. (If this +// is NoWork, use the default strategy: async updates in async mode, sync +// updates in sync mode.) +let expirationContext: ExpirationTime = NoWork; + let isWorking: boolean = false; // The next work in progress fiber that we're currently working on. @@ -811,9 +807,7 @@ function commitRoot(root: FiberRoot, finishedWork: Fiber): void { // here because that code is still in flux. callback = Scheduler_tracing_wrap(callback); } - passiveEffectCallbackHandle = runWithPriority(NormalPriority, () => { - return schedulePassiveEffects(callback); - }); + passiveEffectCallbackHandle = schedulePassiveEffects(callback); passiveEffectCallback = callback; } @@ -1597,58 +1591,52 @@ function computeUniqueAsyncExpiration(): ExpirationTime { } function computeExpirationForFiber(currentTime: ExpirationTime, fiber: Fiber) { - const priorityLevel = getCurrentPriorityLevel(); - let expirationTime; - if ((fiber.mode & ConcurrentMode) === NoContext) { - // Outside of concurrent mode, updates are always synchronous. - expirationTime = Sync; - } else if (isWorking && !isCommitting) { - // During render phase, updates expire during as the current render. - expirationTime = nextRenderExpirationTime; + if (expirationContext !== NoWork) { + // An explicit expiration context was set; + expirationTime = expirationContext; + } else if (isWorking) { + if (isCommitting) { + // Updates that occur during the commit phase should have sync priority + // by default. + expirationTime = Sync; + } else { + // Updates during the render phase should expire at the same time as + // the work that is being rendered. + expirationTime = nextRenderExpirationTime; + } } else { - switch (priorityLevel) { - case ImmediatePriority: - expirationTime = Sync; - break; - case UserBlockingPriority: + // No explicit expiration context was set, and we're not currently + // performing work. Calculate a new expiration time. + if (fiber.mode & ConcurrentMode) { + if (isBatchingInteractiveUpdates) { + // This is an interactive update expirationTime = computeInteractiveExpiration(currentTime); - break; - case NormalPriority: - // This is a normal, concurrent update + } else { + // This is an async update expirationTime = computeAsyncExpiration(currentTime); - break; - case LowPriority: - case IdlePriority: - expirationTime = Never; - break; - default: - invariant( - false, - 'Unknown priority level. This error is likely caused by a bug in ' + - 'React. Please file an issue.', - ); - } - - // If we're in the middle of rendering a tree, do not update at the same - // expiration time that is already rendering. - if (nextRoot !== null && expirationTime === nextRenderExpirationTime) { - expirationTime -= 1; + } + // If we're in the middle of rendering a tree, do not update at the same + // expiration time that is already rendering. + if (nextRoot !== null && expirationTime === nextRenderExpirationTime) { + expirationTime -= 1; + } + } else { + // This is a sync update + expirationTime = Sync; } } - - // Keep track of the lowest pending interactive expiration time. This - // allows us to synchronously flush all interactive updates - // when needed. - // TODO: Move this to renderer? - if ( - priorityLevel === UserBlockingPriority && - (lowestPriorityPendingInteractiveExpirationTime === NoWork || - expirationTime < lowestPriorityPendingInteractiveExpirationTime) - ) { - lowestPriorityPendingInteractiveExpirationTime = expirationTime; + if (isBatchingInteractiveUpdates) { + // This is an interactive update. Keep track of the lowest pending + // interactive expiration time. This allows us to synchronously flush + // all interactive updates when needed. + if ( + lowestPriorityPendingInteractiveExpirationTime === NoWork || + expirationTime < lowestPriorityPendingInteractiveExpirationTime + ) { + lowestPriorityPendingInteractiveExpirationTime = expirationTime; + } } - return expirationTime; } @@ -1909,6 +1897,20 @@ function scheduleWork(fiber: Fiber, expirationTime: ExpirationTime) { } } +function deferredUpdates(fn: () => A): A { + const currentTime = requestCurrentTime(); + const previousExpirationContext = expirationContext; + const previousIsBatchingInteractiveUpdates = isBatchingInteractiveUpdates; + expirationContext = computeAsyncExpiration(currentTime); + isBatchingInteractiveUpdates = false; + try { + return fn(); + } finally { + expirationContext = previousExpirationContext; + isBatchingInteractiveUpdates = previousIsBatchingInteractiveUpdates; + } +} + function syncUpdates( fn: (A, B, C0, D) => R, a: A, @@ -1916,9 +1918,13 @@ function syncUpdates( c: C0, d: D, ): R { - return runWithPriority(ImmediatePriority, () => { + const previousExpirationContext = expirationContext; + expirationContext = Sync; + try { return fn(a, b, c, d); - }); + } finally { + expirationContext = previousExpirationContext; + } } // TODO: Everything below this is written as if it has been lifted to the @@ -1939,6 +1945,7 @@ let unhandledError: mixed | null = null; let isBatchingUpdates: boolean = false; let isUnbatchingUpdates: boolean = false; +let isBatchingInteractiveUpdates: boolean = false; let completedBatches: Array | null = null; @@ -2450,9 +2457,7 @@ function completeRoot( lastCommittedRootDuringThisBatch = root; nestedUpdateCount = 0; } - runWithPriority(ImmediatePriority, () => { - commitRoot(root, finishedWork); - }); + commitRoot(root, finishedWork); } function onUncaughtError(error: mixed) { @@ -2518,6 +2523,9 @@ function flushSync(fn: (a: A) => R, a: A): R { } function interactiveUpdates(fn: (A, B) => R, a: A, b: B): R { + if (isBatchingInteractiveUpdates) { + return fn(a, b); + } // If there are any pending interactive updates, synchronously flush them. // This needs to happen before we read any handlers, because the effect of // the previous event may influence which handlers are called during @@ -2531,13 +2539,14 @@ function interactiveUpdates(fn: (A, B) => R, a: A, b: B): R { performWork(lowestPriorityPendingInteractiveExpirationTime, false); lowestPriorityPendingInteractiveExpirationTime = NoWork; } + const previousIsBatchingInteractiveUpdates = isBatchingInteractiveUpdates; const previousIsBatchingUpdates = isBatchingUpdates; + isBatchingInteractiveUpdates = true; isBatchingUpdates = true; try { - return runWithPriority(UserBlockingPriority, () => { - return fn(a, b); - }); + return fn(a, b); } finally { + isBatchingInteractiveUpdates = previousIsBatchingInteractiveUpdates; isBatchingUpdates = previousIsBatchingUpdates; if (!isBatchingUpdates && !isRendering) { performSyncWork(); @@ -2588,7 +2597,7 @@ export { unbatchedUpdates, flushSync, flushControlled, - Scheduler_next as deferredUpdates, + deferredUpdates, syncUpdates, interactiveUpdates, flushInteractiveUpdates,