Skip to content

Commit

Permalink
Import Scheduler directly, not via host config (#14984)
Browse files Browse the repository at this point in the history
* Import Scheduler directly, not via host config

We currently schedule asynchronous tasks via the host config. (The host
config is a static/build-time dependency injection system that varies
across different renderers — DOM, native, test, and so on.) Instead of
calling platform APIs like `requestIdleCallback` directly, each renderer
implements a method called `scheduleDeferredCallback`.

We've since discovered that when scheduling tasks, it's crucial that
React work is placed in the same queue as other, non-React work on the
main thread. Otherwise, you easily end up in a starvation scenario where
rendering is constantly interrupted by less important tasks. You need a
centralized coordinator that is used both by React and by other
frameworks and application code. This coordinator must also have a
consistent API across all the different host environments, for
convention's sake and so product code is portable — e.g. so the same
component can work in both React Native and React Native Web.

This turned into the Scheduler package. We will have different builds of
Scheduler for each of our target platforms. With this approach, we treat
Scheduler like a built-in platform primitive that exists wherever React
is supported.

Now that we have this consistent interface, the indirection of the host
config no longer makes sense for the purpose of scheduling tasks. In
fact, we explicitly do not want renderers to scheduled task via any
system except the Scheduler package.

So, this PR removes `scheduleDeferredCallback` and its associated
methods from the host config in favor of directly importing Scheduler.

* Missed an extraneous export
  • Loading branch information
acdlite authored Mar 6, 2019
1 parent 5d49daf commit 1e3b619
Show file tree
Hide file tree
Showing 11 changed files with 24 additions and 123 deletions.
2 changes: 0 additions & 2 deletions packages/react-art/src/ReactARTHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -343,8 +343,6 @@ export function getChildHostContext() {
export const scheduleTimeout = setTimeout;
export const cancelTimeout = clearTimeout;
export const noTimeout = -1;
export const schedulePassiveEffects = scheduleDeferredCallback;
export const cancelPassiveEffects = cancelDeferredCallback;

export function shouldSetTextContent(type, props) {
return (
Expand Down
2 changes: 0 additions & 2 deletions packages/react-dom/src/client/ReactDOMHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -310,8 +310,6 @@ export const scheduleTimeout =
export const cancelTimeout =
typeof clearTimeout === 'function' ? clearTimeout : (undefined: any);
export const noTimeout = -1;
export const schedulePassiveEffects = scheduleDeferredCallback;
export const cancelPassiveEffects = cancelDeferredCallback;

// -------------------
// Mutation
Expand Down
12 changes: 0 additions & 12 deletions packages/react-native-renderer/src/ReactFabricHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,6 @@ import {
warnForStyleProps,
} from './NativeMethodsMixinUtils';
import {create, diff} from './ReactNativeAttributePayload';
import {
now as ReactNativeFrameSchedulingNow,
cancelDeferredCallback as ReactNativeFrameSchedulingCancelDeferredCallback,
scheduleDeferredCallback as ReactNativeFrameSchedulingScheduleDeferredCallback,
shouldYield as ReactNativeFrameSchedulingShouldYield,
} from './ReactNativeFrameScheduling';
import {get as getViewConfigForType} from 'ReactNativeViewConfigRegistry';

import deepFreezeAndThrowOnMutationInDev from 'deepFreezeAndThrowOnMutationInDev';
Expand Down Expand Up @@ -333,16 +327,10 @@ export function shouldSetTextContent(type: string, props: Props): boolean {

// The Fabric renderer is secondary to the existing React Native renderer.
export const isPrimaryRenderer = false;
export const now = ReactNativeFrameSchedulingNow;
export const scheduleDeferredCallback = ReactNativeFrameSchedulingScheduleDeferredCallback;
export const cancelDeferredCallback = ReactNativeFrameSchedulingCancelDeferredCallback;
export const shouldYield = ReactNativeFrameSchedulingShouldYield;

export const scheduleTimeout = setTimeout;
export const cancelTimeout = clearTimeout;
export const noTimeout = -1;
export const schedulePassiveEffects = scheduleDeferredCallback;
export const cancelPassiveEffects = cancelDeferredCallback;

// -------------------
// Persistence
Expand Down
56 changes: 0 additions & 56 deletions packages/react-native-renderer/src/ReactNativeFrameScheduling.js

This file was deleted.

12 changes: 0 additions & 12 deletions packages/react-native-renderer/src/ReactNativeHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,6 @@ import {
updateFiberProps,
} from './ReactNativeComponentTree';
import ReactNativeFiberHostComponent from './ReactNativeFiberHostComponent';
import {
now as ReactNativeFrameSchedulingNow,
cancelDeferredCallback as ReactNativeFrameSchedulingCancelDeferredCallback,
scheduleDeferredCallback as ReactNativeFrameSchedulingScheduleDeferredCallback,
shouldYield as ReactNativeFrameSchedulingShouldYield,
} from './ReactNativeFrameScheduling';

export type Type = string;
export type Props = Object;
Expand Down Expand Up @@ -234,17 +228,11 @@ export function resetAfterCommit(containerInfo: Container): void {
// Noop
}

export const now = ReactNativeFrameSchedulingNow;
export const isPrimaryRenderer = true;
export const scheduleDeferredCallback = ReactNativeFrameSchedulingScheduleDeferredCallback;
export const cancelDeferredCallback = ReactNativeFrameSchedulingCancelDeferredCallback;
export const shouldYield = ReactNativeFrameSchedulingShouldYield;

export const scheduleTimeout = setTimeout;
export const cancelTimeout = clearTimeout;
export const noTimeout = -1;
export const schedulePassiveEffects = scheduleDeferredCallback;
export const cancelPassiveEffects = cancelDeferredCallback;

export function shouldDeprioritizeSubtree(type: string, props: Props): boolean {
return false;
Expand Down
8 changes: 0 additions & 8 deletions packages/react-noop-renderer/src/createReactNoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -304,18 +304,10 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
return inst;
},

scheduleDeferredCallback: Scheduler.unstable_scheduleCallback,
cancelDeferredCallback: Scheduler.unstable_cancelCallback,

shouldYield: Scheduler.unstable_shouldYield,

scheduleTimeout: setTimeout,
cancelTimeout: clearTimeout,
noTimeout: -1,

schedulePassiveEffects: Scheduler.unstable_scheduleCallback,
cancelPassiveEffects: Scheduler.unstable_cancelCallback,

prepareForCommit(): void {},

resetAfterCommit(): void {},
Expand Down
27 changes: 14 additions & 13 deletions packages/react-reconciler/src/ReactFiberScheduler.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@ 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 * as Scheduler from 'scheduler';
import {
__interactionsRef,
__subscriberRef,
Expand Down Expand Up @@ -78,17 +76,11 @@ import {
setCurrentFiber,
} from './ReactCurrentFiber';
import {
now,
scheduleDeferredCallback,
cancelDeferredCallback,
shouldYield,
prepareForCommit,
resetAfterCommit,
scheduleTimeout,
cancelTimeout,
noTimeout,
schedulePassiveEffects,
cancelPassiveEffects,
} from './ReactFiberHostConfig';
import {
markPendingPriorityLevel,
Expand Down Expand Up @@ -172,6 +164,15 @@ import {
} from './ReactFiberCommitWork';
import {ContextOnlyDispatcher} from './ReactFiberHooks';

// Intentionally not named imports because Rollup would use dynamic dispatch for
// CommonJS interop named imports.
const {
unstable_scheduleCallback: scheduleCallback,
unstable_cancelCallback: cancelCallback,
unstable_shouldYield: shouldYield,
unstable_now: now,
} = Scheduler;

export type Thenable = {
then(resolve: () => mixed, reject?: () => mixed): mixed,
};
Expand Down Expand Up @@ -598,7 +599,7 @@ function markLegacyErrorBoundaryAsFailed(instance: mixed) {

function flushPassiveEffects() {
if (passiveEffectCallbackHandle !== null) {
cancelPassiveEffects(passiveEffectCallbackHandle);
cancelCallback(passiveEffectCallbackHandle);
}
if (passiveEffectCallback !== null) {
// We call the scheduled callback instead of commitPassiveEffects directly
Expand Down Expand Up @@ -807,7 +808,7 @@ function commitRoot(root: FiberRoot, finishedWork: Fiber): void {
// here because that code is still in flux.
callback = Scheduler_tracing_wrap(callback);
}
passiveEffectCallbackHandle = schedulePassiveEffects(callback);
passiveEffectCallbackHandle = scheduleCallback(callback);
passiveEffectCallback = callback;
}

Expand Down Expand Up @@ -1978,7 +1979,7 @@ function scheduleCallbackWithExpirationTime(
if (callbackID !== null) {
// Existing callback has insufficient timeout. Cancel and schedule a
// new one.
cancelDeferredCallback(callbackID);
cancelCallback(callbackID);
}
}
// The request callback timer is already running. Don't start a new one.
Expand All @@ -1990,7 +1991,7 @@ function scheduleCallbackWithExpirationTime(
const currentMs = now() - originalStartTimeMs;
const expirationTimeMs = expirationTimeToMs(expirationTime);
const timeout = expirationTimeMs - currentMs;
callbackID = scheduleDeferredCallback(performAsyncWork, {timeout});
callbackID = scheduleCallback(performAsyncWork, {timeout});
}

// For every call to renderRoot, one of onFatal, onComplete, onSuspend, and
Expand Down
6 changes: 5 additions & 1 deletion packages/react-reconciler/src/ReactProfilerTimer.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@ import type {Fiber} from './ReactFiber';

import {enableProfilerTimer} from 'shared/ReactFeatureFlags';

import {now} from './ReactFiberHostConfig';
// Intentionally not named imports because Rollup would use dynamic dispatch for
// CommonJS interop named imports.
import * as Scheduler from 'scheduler';

const {unstable_now: now} = Scheduler;

export type ProfilerTimer = {
getCommitTime(): number,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,9 @@ export const shouldSetTextContent = $$$hostConfig.shouldSetTextContent;
export const shouldDeprioritizeSubtree =
$$$hostConfig.shouldDeprioritizeSubtree;
export const createTextInstance = $$$hostConfig.createTextInstance;
export const scheduleDeferredCallback = $$$hostConfig.scheduleDeferredCallback;
export const cancelDeferredCallback = $$$hostConfig.cancelDeferredCallback;
export const shouldYield = $$$hostConfig.shouldYield;
export const scheduleTimeout = $$$hostConfig.setTimeout;
export const cancelTimeout = $$$hostConfig.clearTimeout;
export const noTimeout = $$$hostConfig.noTimeout;
export const schedulePassiveEffects = $$$hostConfig.schedulePassiveEffects;
export const cancelPassiveEffects = $$$hostConfig.cancelPassiveEffects;
export const now = $$$hostConfig.now;
export const isPrimaryRenderer = $$$hostConfig.isPrimaryRenderer;
export const supportsMutation = $$$hostConfig.supportsMutation;
Expand Down
9 changes: 0 additions & 9 deletions packages/react-test-renderer/src/ReactTestHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
* @flow
*/

import * as Scheduler from 'scheduler/unstable_mock';
import warning from 'shared/warning';

export type Type = string;
Expand Down Expand Up @@ -195,18 +194,10 @@ export function createTextInstance(
}

export const isPrimaryRenderer = false;
// This approach enables `now` to be mocked by tests,
// Even after the reconciler has initialized and read host config values.
export const now = Scheduler.unstable_now;
export const scheduleDeferredCallback = Scheduler.unstable_scheduleCallback;
export const cancelDeferredCallback = Scheduler.unstable_cancelCallback;
export const shouldYield = Scheduler.unstable_shouldYield;

export const scheduleTimeout = setTimeout;
export const cancelTimeout = clearTimeout;
export const noTimeout = -1;
export const schedulePassiveEffects = Scheduler.unstable_scheduleCallback;
export const cancelPassiveEffects = Scheduler.unstable_cancelCallback;

// -------------------
// Mutation
Expand Down
8 changes: 5 additions & 3 deletions packages/react/src/__tests__/ReactProfiler-test.internal.js
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ describe('Profiler', () => {
it('does not record times for components outside of Profiler tree', () => {
// Mock the Scheduler module so we can track how many times the current
// time is read
jest.mock('scheduler/unstable_mock', obj => {
jest.mock('scheduler', obj => {
const ActualScheduler = require.requireActual(
'scheduler/unstable_mock',
);
Expand Down Expand Up @@ -300,8 +300,10 @@ describe('Profiler', () => {
'read current time',
]);

// Remove mock
jest.unmock('scheduler/unstable_mock');
// Restore original mock
jest.mock('scheduler', () =>
require.requireActual('scheduler/unstable_mock'),
);
});

it('logs render times for both mount and update', () => {
Expand Down

0 comments on commit 1e3b619

Please sign in to comment.