From 6568a79931bb335f2c997ed9efbc533f2677df5c Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Sat, 22 Jun 2019 00:15:09 -0700 Subject: [PATCH] [Scheduler] requestPaint (#15960) * [Scheduler] requestPaint Signals to Scheduler that the browser needs to paint the screen. React will call it in the commit phase. Scheduler will yield at the end of the current frame, even if there is no pending input. When `isInputPending` is not available, this has no effect, because we yield at the end of every frame regardless. React will call `requestPaint` in the commit phase as long as there's at least one effect. We could choose not to call it if none of the effects are DOM mutations, but this is so rare that it doesn't seem worthwhile to bother checking. * Fall back gracefully if requestPaint is missing --- packages/react-art/src/ReactARTHostConfig.js | 12 ------ .../src/client/ReactDOMHostConfig.js | 13 ------- .../src/ReactFiberWorkLoop.js | 5 +++ .../src/SchedulerWithReactIntegration.js | 4 ++ ...ReactSchedulerIntegration-test.internal.js | 26 +++++++++++++ .../npm/umd/scheduler.development.js | 8 ++++ .../npm/umd/scheduler.production.min.js | 8 ++++ .../npm/umd/scheduler.profiling.min.js | 8 ++++ packages/scheduler/src/Scheduler.js | 4 ++ .../src/forks/SchedulerHostConfig.default.js | 24 ++++++++---- .../src/forks/SchedulerHostConfig.mock.js | 39 +++++++++++++++++-- packages/scheduler/unstable_mock.js | 1 + packages/shared/forks/Scheduler.umd.js | 2 + 13 files changed, 119 insertions(+), 35 deletions(-) diff --git a/packages/react-art/src/ReactARTHostConfig.js b/packages/react-art/src/ReactARTHostConfig.js index 458c3f0f82..20611f0275 100644 --- a/packages/react-art/src/ReactARTHostConfig.js +++ b/packages/react-art/src/ReactARTHostConfig.js @@ -7,23 +7,11 @@ import Transform from 'art/core/transform'; import Mode from 'art/modes/current'; -import * as Scheduler from 'scheduler'; import invariant from 'shared/invariant'; import {TYPES, EVENT_TYPES, childrenAsString} from './ReactARTInternals'; import type {ReactEventComponentInstance} from 'shared/ReactTypes'; -// Intentionally not named imports because Rollup would -// use dynamic dispatch for CommonJS interop named imports. -const { - unstable_now: now, - unstable_scheduleCallback: scheduleDeferredCallback, - unstable_shouldYield: shouldYield, - unstable_cancelCallback: cancelDeferredCallback, -} = Scheduler; - -export {now, scheduleDeferredCallback, shouldYield, cancelDeferredCallback}; - const pooledTransform = new Transform(); const NO_CONTEXT = {}; diff --git a/packages/react-dom/src/client/ReactDOMHostConfig.js b/packages/react-dom/src/client/ReactDOMHostConfig.js index 39e22e88b6..31f568ccbd 100644 --- a/packages/react-dom/src/client/ReactDOMHostConfig.js +++ b/packages/react-dom/src/client/ReactDOMHostConfig.js @@ -7,8 +7,6 @@ * @flow */ -import * as Scheduler from 'scheduler'; - import {precacheFiberNode, updateFiberProps} from './ReactDOMComponentTree'; import { createElement, @@ -113,17 +111,6 @@ import warning from 'shared/warning'; const {html: HTML_NAMESPACE} = Namespaces; -// Intentionally not named imports because Rollup would -// use dynamic dispatch for CommonJS interop named imports. -const { - unstable_now: now, - unstable_scheduleCallback: scheduleDeferredCallback, - unstable_shouldYield: shouldYield, - unstable_cancelCallback: cancelDeferredCallback, -} = Scheduler; - -export {now, scheduleDeferredCallback, shouldYield, cancelDeferredCallback}; - let SUPPRESS_HYDRATION_WARNING; if (__DEV__) { SUPPRESS_HYDRATION_WARNING = 'suppressHydrationWarning'; diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index af2be67081..66fbb64987 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -36,6 +36,7 @@ import { getCurrentPriorityLevel, runWithPriority, shouldYield, + requestPaint, now, ImmediatePriority, UserBlockingPriority, @@ -1666,6 +1667,10 @@ function commitRootImpl(root) { nextEffect = null; + // Tell Scheduler to yield at the end of the frame, so the browser has an + // opportunity to paint. + requestPaint(); + if (enableSchedulerTracing) { __interactionsRef.current = ((prevInteractions: any): Set); } diff --git a/packages/react-reconciler/src/SchedulerWithReactIntegration.js b/packages/react-reconciler/src/SchedulerWithReactIntegration.js index d01cda3e7a..c5a78ce304 100644 --- a/packages/react-reconciler/src/SchedulerWithReactIntegration.js +++ b/packages/react-reconciler/src/SchedulerWithReactIntegration.js @@ -19,6 +19,7 @@ const { unstable_scheduleCallback: Scheduler_scheduleCallback, unstable_cancelCallback: Scheduler_cancelCallback, unstable_shouldYield: Scheduler_shouldYield, + unstable_requestPaint: Scheduler_requestPaint, unstable_now: Scheduler_now, unstable_getCurrentPriorityLevel: Scheduler_getCurrentPriorityLevel, unstable_ImmediatePriority: Scheduler_ImmediatePriority, @@ -63,6 +64,9 @@ export const IdlePriority: ReactPriorityLevel = 95; export const NoPriority: ReactPriorityLevel = 90; export const shouldYield = Scheduler_shouldYield; +export const requestPaint = + // Fall back gracefully if we're running an older verison of Scheduler. + Scheduler_requestPaint !== undefined ? Scheduler_requestPaint : () => {}; let syncQueue: Array | null = null; let immediateQueueCallbackNode: mixed | null = null; diff --git a/packages/react-reconciler/src/__tests__/ReactSchedulerIntegration-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSchedulerIntegration-test.internal.js index dec4bfbfc5..5e906d0e46 100644 --- a/packages/react-reconciler/src/__tests__/ReactSchedulerIntegration-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSchedulerIntegration-test.internal.js @@ -166,6 +166,32 @@ describe('ReactSchedulerIntegration', () => { expect(Scheduler).toFlushAndYield(['A [UserBlocking]', 'B [Normal]']); }); + it('requests a paint after committing', () => { + const scheduleCallback = Scheduler.unstable_scheduleCallback; + + const root = ReactNoop.createRoot(); + root.render('Initial'); + Scheduler.flushAll(); + + scheduleCallback(NormalPriority, () => Scheduler.yieldValue('A')); + scheduleCallback(NormalPriority, () => Scheduler.yieldValue('B')); + scheduleCallback(NormalPriority, () => Scheduler.yieldValue('C')); + + // Schedule a React render. React will request a paint after committing it. + root.render('Update'); + + // Advance time just to be sure the next tasks have lower priority + Scheduler.advanceTime(2000); + + scheduleCallback(NormalPriority, () => Scheduler.yieldValue('D')); + scheduleCallback(NormalPriority, () => Scheduler.yieldValue('E')); + + // Flush everything up to the next paint. Should yield after the + // React commit. + Scheduler.unstable_flushUntilNextPaint(); + expect(Scheduler).toHaveYielded(['A', 'B', 'C']); + }); + // TODO it.skip('passive effects have render priority even if they are flushed early', () => {}); }); diff --git a/packages/scheduler/npm/umd/scheduler.development.js b/packages/scheduler/npm/umd/scheduler.development.js index 48845964b2..c3e461e872 100644 --- a/packages/scheduler/npm/umd/scheduler.development.js +++ b/packages/scheduler/npm/umd/scheduler.development.js @@ -47,6 +47,13 @@ ); } + function unstable_requestPaint() { + return global.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Scheduler.unstable_requestPaint.apply( + this, + arguments + ); + } + function unstable_runWithPriority() { return global.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Scheduler.unstable_runWithPriority.apply( this, @@ -108,6 +115,7 @@ unstable_scheduleCallback: unstable_scheduleCallback, unstable_cancelCallback: unstable_cancelCallback, unstable_shouldYield: unstable_shouldYield, + unstable_requestPaint: unstable_requestPaint, unstable_runWithPriority: unstable_runWithPriority, unstable_next: unstable_next, unstable_wrapCallback: unstable_wrapCallback, diff --git a/packages/scheduler/npm/umd/scheduler.production.min.js b/packages/scheduler/npm/umd/scheduler.production.min.js index 3d020960b2..8daa4d9981 100644 --- a/packages/scheduler/npm/umd/scheduler.production.min.js +++ b/packages/scheduler/npm/umd/scheduler.production.min.js @@ -47,6 +47,13 @@ ); } + function unstable_requestPaint() { + return global.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Scheduler.unstable_requestPaint.apply( + this, + arguments + ); + } + function unstable_runWithPriority() { return global.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Scheduler.unstable_runWithPriority.apply( this, @@ -102,6 +109,7 @@ unstable_scheduleCallback: unstable_scheduleCallback, unstable_cancelCallback: unstable_cancelCallback, unstable_shouldYield: unstable_shouldYield, + unstable_requestPaint: unstable_requestPaint, unstable_runWithPriority: unstable_runWithPriority, unstable_next: unstable_next, unstable_wrapCallback: unstable_wrapCallback, diff --git a/packages/scheduler/npm/umd/scheduler.profiling.min.js b/packages/scheduler/npm/umd/scheduler.profiling.min.js index 3d020960b2..8daa4d9981 100644 --- a/packages/scheduler/npm/umd/scheduler.profiling.min.js +++ b/packages/scheduler/npm/umd/scheduler.profiling.min.js @@ -47,6 +47,13 @@ ); } + function unstable_requestPaint() { + return global.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Scheduler.unstable_requestPaint.apply( + this, + arguments + ); + } + function unstable_runWithPriority() { return global.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Scheduler.unstable_runWithPriority.apply( this, @@ -102,6 +109,7 @@ unstable_scheduleCallback: unstable_scheduleCallback, unstable_cancelCallback: unstable_cancelCallback, unstable_shouldYield: unstable_shouldYield, + unstable_requestPaint: unstable_requestPaint, unstable_runWithPriority: unstable_runWithPriority, unstable_next: unstable_next, unstable_wrapCallback: unstable_wrapCallback, diff --git a/packages/scheduler/src/Scheduler.js b/packages/scheduler/src/Scheduler.js index c400e095a0..e14c3d8272 100644 --- a/packages/scheduler/src/Scheduler.js +++ b/packages/scheduler/src/Scheduler.js @@ -16,6 +16,7 @@ import { shouldYieldToHost, getCurrentTime, forceFrameRate, + requestPaint, } from './SchedulerHostConfig'; // TODO: Use symbols? @@ -506,6 +507,8 @@ function unstable_shouldYield() { ); } +const unstable_requestPaint = requestPaint; + export { ImmediatePriority as unstable_ImmediatePriority, UserBlockingPriority as unstable_UserBlockingPriority, @@ -519,6 +522,7 @@ export { unstable_wrapCallback, unstable_getCurrentPriorityLevel, unstable_shouldYield, + unstable_requestPaint, unstable_continueExecution, unstable_pauseExecution, unstable_getFirstCallbackNode, diff --git a/packages/scheduler/src/forks/SchedulerHostConfig.default.js b/packages/scheduler/src/forks/SchedulerHostConfig.default.js index c9bff027a8..60e749ba63 100644 --- a/packages/scheduler/src/forks/SchedulerHostConfig.default.js +++ b/packages/scheduler/src/forks/SchedulerHostConfig.default.js @@ -18,6 +18,7 @@ export let cancelHostCallback; export let requestHostTimeout; export let cancelHostTimeout; export let shouldYieldToHost; +export let requestPaint; export let getCurrentTime; export let forceFrameRate; @@ -125,7 +126,7 @@ if ( shouldYieldToHost = function() { return false; }; - forceFrameRate = function() {}; + requestPaint = forceFrameRate = function() {}; } else { if (typeof console !== 'undefined') { // TODO: Remove fb.me link @@ -162,7 +163,8 @@ if ( // TODO: Make this configurable // TODO: Adjust this based on priority? - let maxFrameLength = 300; + let maxFrameLength = 150; + let needsPaint = false; const isInputPending = navigator !== undefined && @@ -181,11 +183,12 @@ if ( // main thread, so the browser can perform high priority tasks. The main // ones are painting and user input. If we're certain there's no user // input, then we can yield less often without making the app less - // responsive. We'll eventually yield regardless, since there could be - // other main thread tasks that we don't know about. - if (isInputPending !== null && !isInputPending()) { - // There's no pending input. Only yield if we've reached the max - // frame length. + // responsive. We'll also check if a paint was requested. We'll eventually + // yield regardless, since there could be other main thread tasks that we + // don't know about. + if (!needsPaint && isInputPending !== null && !isInputPending()) { + // There's no pending input, and no task requested a paint. Only yield + // if we've reached the max frame length. return currentTime >= frameDeadline + maxFrameLength; } // Either there is pending input, or there's no way for us to be sure @@ -242,6 +245,9 @@ if ( port.postMessage(undefined); throw error; } + // Yielding to the browser will give it a chance to paint, so we can + // reset this. + needsPaint = false; } }; @@ -321,4 +327,8 @@ if ( localClearTimeout(timeoutID); timeoutID = -1; }; + + requestPaint = function() { + needsPaint = true; + }; } diff --git a/packages/scheduler/src/forks/SchedulerHostConfig.mock.js b/packages/scheduler/src/forks/SchedulerHostConfig.mock.js index 95d974e0c3..9c1b5bca55 100644 --- a/packages/scheduler/src/forks/SchedulerHostConfig.mock.js +++ b/packages/scheduler/src/forks/SchedulerHostConfig.mock.js @@ -15,6 +15,8 @@ let yieldedValues: Array | null = null; let expectedNumberOfYields: number = -1; let didStop: boolean = false; let isFlushing: boolean = false; +let needsPaint: boolean = false; +let shouldYieldForPaint: boolean = false; export function requestHostCallback(callback: boolean => void) { scheduledCallback = callback; @@ -36,9 +38,10 @@ export function cancelHostTimeout(): void { export function shouldYieldToHost(): boolean { if ( - expectedNumberOfYields !== -1 && - yieldedValues !== null && - yieldedValues.length >= expectedNumberOfYields + (expectedNumberOfYields !== -1 && + yieldedValues !== null && + yieldedValues.length >= expectedNumberOfYields) || + (shouldYieldForPaint && needsPaint) ) { // We yielded at least as many values as expected. Stop flushing. didStop = true; @@ -67,6 +70,7 @@ export function reset() { expectedNumberOfYields = -1; didStop = false; isFlushing = false; + needsPaint = false; } // Should only be used via an assertion helper that inspects the yielded values. @@ -94,6 +98,31 @@ export function unstable_flushNumberOfYields(count: number): void { } } +export function unstable_flushUntilNextPaint(): void { + if (isFlushing) { + throw new Error('Already flushing work.'); + } + if (scheduledCallback !== null) { + const cb = scheduledCallback; + shouldYieldForPaint = true; + needsPaint = false; + isFlushing = true; + try { + let hasMoreWork = true; + do { + hasMoreWork = cb(true, currentTime); + } while (hasMoreWork && !didStop); + if (!hasMoreWork) { + scheduledCallback = null; + } + } finally { + shouldYieldForPaint = false; + didStop = false; + isFlushing = false; + } + } +} + export function unstable_flushExpired() { if (isFlushing) { throw new Error('Already flushing work.'); @@ -181,3 +210,7 @@ export function advanceTime(ms: number) { unstable_flushExpired(); } } + +export function requestPaint() { + needsPaint = true; +} diff --git a/packages/scheduler/unstable_mock.js b/packages/scheduler/unstable_mock.js index 8ab48d336b..2e95ba3772 100644 --- a/packages/scheduler/unstable_mock.js +++ b/packages/scheduler/unstable_mock.js @@ -14,6 +14,7 @@ export { unstable_flushNumberOfYields, unstable_flushExpired, unstable_clearYields, + unstable_flushUntilNextPaint, flushAll, yieldValue, advanceTime, diff --git a/packages/shared/forks/Scheduler.umd.js b/packages/shared/forks/Scheduler.umd.js index 92738cbe99..1fb69c3b8a 100644 --- a/packages/shared/forks/Scheduler.umd.js +++ b/packages/shared/forks/Scheduler.umd.js @@ -16,6 +16,7 @@ const { unstable_now, unstable_scheduleCallback, unstable_shouldYield, + unstable_requestPaint, unstable_getFirstCallbackNode, unstable_runWithPriority, unstable_next, @@ -39,6 +40,7 @@ export { unstable_now, unstable_scheduleCallback, unstable_shouldYield, + unstable_requestPaint, unstable_getFirstCallbackNode, unstable_runWithPriority, unstable_next,