diff --git a/packages/react-reconciler/src/ReactFiberScheduler.js b/packages/react-reconciler/src/ReactFiberScheduler.js index 9c1065a568781..edb3304ce244e 100644 --- a/packages/react-reconciler/src/ReactFiberScheduler.js +++ b/packages/react-reconciler/src/ReactFiberScheduler.js @@ -234,13 +234,12 @@ let interruptedBy: Fiber | null = null; // In other words, because expiration times determine how updates are batched, // we want all updates of like priority that occur within the same event to // receive the same expiration time. Otherwise we get tearing. -let initialTimeMs: number = now(); let currentEventTime: ExpirationTime = NoWork; export function requestCurrentTime() { if (workPhase === RenderPhase || workPhase === CommitPhase) { // We're inside React, so it's fine to read the actual time. - return msToExpirationTime(now() - initialTimeMs); + return msToExpirationTime(now()); } // We're not inside React, so we may be in the middle of a browser event. if (currentEventTime !== NoWork) { @@ -248,7 +247,7 @@ export function requestCurrentTime() { return currentEventTime; } // This is the first update since React yielded. Compute a new start time. - currentEventTime = msToExpirationTime(now() - initialTimeMs); + currentEventTime = msToExpirationTime(now()); return currentEventTime; } @@ -453,10 +452,18 @@ function scheduleCallbackForRoot( cancelCallback(existingCallbackNode); } root.callbackExpirationTime = expirationTime; - const options = - expirationTime === Sync - ? null - : {timeout: expirationTimeToMs(expirationTime)}; + + let options = null; + if (expirationTime !== Sync && expirationTime !== Never) { + let timeout = expirationTimeToMs(expirationTime) - now(); + if (timeout > 5000) { + // Sanity check. Should never take longer than 5 seconds. + // TODO: Add internal warning? + timeout = 5000; + } + options = {timeout}; + } + root.callbackNode = scheduleCallback( priorityLevel, runRootCallback.bind( @@ -950,7 +957,7 @@ function inferTimeFromExpirationTime(expirationTime: ExpirationTime): number { // We don't know exactly when the update was scheduled, but we can infer an // approximate start time from the expiration time. const earliestExpirationTimeMs = expirationTimeToMs(expirationTime); - return earliestExpirationTimeMs - LOW_PRIORITY_EXPIRATION + initialTimeMs; + return earliestExpirationTimeMs - LOW_PRIORITY_EXPIRATION; } function workLoopSync() { @@ -1861,7 +1868,7 @@ function computeMsUntilTimeout( // Compute the time until this render pass would expire. const timeUntilExpirationMs = - expirationTimeToMs(committedExpirationTime) + initialTimeMs - currentTimeMs; + expirationTimeToMs(committedExpirationTime) - currentTimeMs; // Clamp the timeout to the expiration time. // TODO: Once the event time is exact instead of inferred from expiration time diff --git a/packages/react-reconciler/src/SchedulerWithReactIntegration.js b/packages/react-reconciler/src/SchedulerWithReactIntegration.js index 3a30a2ea2e48a..0463ab03b8ec3 100644 --- a/packages/react-reconciler/src/SchedulerWithReactIntegration.js +++ b/packages/react-reconciler/src/SchedulerWithReactIntegration.js @@ -65,7 +65,6 @@ export const IdlePriority: ReactPriorityLevel = 95; // NoPriority is the absence of priority. Also React-only. export const NoPriority: ReactPriorityLevel = 90; -export const now = Scheduler_now; export const shouldYield = disableYielding ? () => false // Never yield when `disableYielding` is on : Scheduler_shouldYield; @@ -73,6 +72,17 @@ export const shouldYield = disableYielding let immediateQueue: Array | null = null; let immediateQueueCallbackNode: mixed | null = null; let isFlushingImmediate: boolean = false; +let initialTimeMs: number = Scheduler_now(); + +// If the initial timestamp is reasonably small, use Scheduler's `now` directly. +// This will be the case for modern browsers that support `performance.now`. In +// older browsers, Scheduler falls back to `Date.now`, which returns a Unix +// timestamp. In that case, subtract the module initialization time to simulate +// the behavior of performance.now and keep our times small enough to fit +// within 32 bits. +// TODO: Consider lifting this into Scheduler. +export const now = + initialTimeMs < 10000 ? Scheduler_now : () => Scheduler_now() - initialTimeMs; export function getCurrentPriorityLevel(): ReactPriorityLevel { switch (Scheduler_getCurrentPriorityLevel()) { diff --git a/packages/react-reconciler/src/__tests__/ReactExpiration-test.internal.js b/packages/react-reconciler/src/__tests__/ReactExpiration-test.internal.js index 9a9402ecb90d2..4569250c48e06 100644 --- a/packages/react-reconciler/src/__tests__/ReactExpiration-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactExpiration-test.internal.js @@ -272,4 +272,25 @@ describe('ReactExpiration', () => { expect(Scheduler).toFlushExpired([]); expect(ReactNoop).toMatchRenderedOutput('Hi'); }); + + it('should measure callback timeout relative to current time, not start-up time', () => { + // Corresponds to a bugfix: https://github.com/facebook/react/pull/15479 + // The bug wasn't caught by other tests because we use virtual times that + // default to 0, and most tests don't advance time. + + // Before scheduling an update, advance the current time. + Scheduler.advanceTime(10000); + + ReactNoop.render('Hi'); + expect(Scheduler).toFlushExpired([]); + expect(ReactNoop).toMatchRenderedOutput(null); + + // Advancing by ~5 seconds should be sufficient to expire the update. (I + // used a slightly larger number to allow for possible rounding.) + Scheduler.advanceTime(6000); + + ReactNoop.render('Hi'); + expect(Scheduler).toFlushExpired([]); + expect(ReactNoop).toMatchRenderedOutput('Hi'); + }); });