Skip to content

Commit

Permalink
Replace noop's fake Scheduler implementation with mock Scheduler build (
Browse files Browse the repository at this point in the history 
#14969)

* Replace noop's fake Scheduler implementation with mock Scheduler build

The noop renderer has its own mock implementation of the Scheduler
interface, with the ability to partially render work in tests. Now that
this functionality has been lifted into a proper mock Scheduler build,
we can use that instead.

Most of the existing noop tests were unaffected, but I did have to make
some changes. The biggest one involved passive effects: previously, they
were scheduled on a separate queue from the queue that handles
rendering. After this change, both rendering and effects are scheduled
in the Scheduler queue. I think this is a better approach because tests
no longer have to worry about the difference; if you call `flushAll`,
all the work is flushed, both rendering and effects. But for those few
tests that do care to flush the rendering without the effects, that's
still possible using the `yieldValue` API.

Follow-up: Do the same for test renderer.

* Fix import to scheduler/unstable_mock
  • Loading branch information
@acdlite
acdlite authored Feb 28, 2019
1 parent 3ada82b commit 53e787b
Show file tree
Hide file tree
Showing 14 changed files with 479 additions and 470 deletions.
165 changes: 19 additions & 146 deletions packages/react-noop-renderer/src/createReactNoop.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import type {Fiber} from 'react-reconciler/src/ReactFiber';
import type {UpdateQueue} from 'react-reconciler/src/ReactUpdateQueue';
import type {ReactNodeList} from 'shared/ReactTypes';

import * as Scheduler from 'scheduler/unstable_mock';
import {createPortal} from 'shared/ReactPortal';
import expect from 'expect';
import {REACT_FRAGMENT_TYPE, REACT_ELEMENT_TYPE} from 'shared/ReactSymbols';
Expand Down Expand Up @@ -51,9 +52,6 @@ if (__DEV__) {
}

function createReactNoop(reconciler: Function, useMutation: boolean) {
let scheduledCallback = null;
let scheduledCallbackTimeout = -1;
let scheduledPassiveCallback = null;
let instanceCounter = 0;
let hostDiffCounter = 0;
let hostUpdateCounter = 0;
Expand Down Expand Up @@ -218,8 +216,6 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
);
}

let elapsedTimeInMs = 0;

const sharedHostConfig = {
getRootHostContext() {
return NO_CONTEXT;
Expand Down Expand Up @@ -308,66 +304,23 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
return inst;
},

scheduleDeferredCallback(callback, options) {
if (scheduledCallback) {
throw new Error(
'Scheduling a callback twice is excessive. Instead, keep track of ' +
'whether the callback has already been scheduled.',
);
}
scheduledCallback = callback;
if (
typeof options === 'object' &&
options !== null &&
typeof options.timeout === 'number'
) {
const newTimeout = options.timeout;
if (
scheduledCallbackTimeout === -1 ||
scheduledCallbackTimeout > newTimeout
) {
scheduledCallbackTimeout = elapsedTimeInMs + newTimeout;
}
}
return 0;
},

cancelDeferredCallback() {
if (scheduledCallback === null) {
throw new Error('No callback is scheduled.');
}
scheduledCallback = null;
scheduledCallbackTimeout = -1;
},
scheduleDeferredCallback: Scheduler.unstable_scheduleCallback,
cancelDeferredCallback: Scheduler.unstable_cancelCallback,

shouldYield,
shouldYield: Scheduler.unstable_shouldYield,

scheduleTimeout: setTimeout,
cancelTimeout: clearTimeout,
noTimeout: -1,
schedulePassiveEffects(callback) {
if (scheduledCallback) {
throw new Error(
'Scheduling a callback twice is excessive. Instead, keep track of ' +
'whether the callback has already been scheduled.',
);
}
scheduledPassiveCallback = callback;
},
cancelPassiveEffects() {
if (scheduledPassiveCallback === null) {
throw new Error('No passive effects callback is scheduled.');
}
scheduledPassiveCallback = null;
},

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

prepareForCommit(): void {},

resetAfterCommit(): void {},

now(): number {
return elapsedTimeInMs;
},
now: Scheduler.unstable_now,

isPrimaryRenderer: true,
supportsHydration: false,
Expand Down Expand Up @@ -534,71 +487,6 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
const roots = new Map();
const DEFAULT_ROOT_ID = '<default>';

let yieldedValues: Array<mixed> = [];
let didStop: boolean = false;
let expectedNumberOfYields: number = -1;

function shouldYield() {
if (
expectedNumberOfYields !== -1 &&
yieldedValues.length >= expectedNumberOfYields &&
(scheduledCallbackTimeout === -1 ||
elapsedTimeInMs < scheduledCallbackTimeout)
) {
// We yielded at least as many values as expected. Stop rendering.
didStop = true;
return true;
}
// Keep rendering.
return false;
}

function flushAll(): Array<mixed> {
yieldedValues = [];
while (scheduledCallback !== null) {
const cb = scheduledCallback;
scheduledCallback = null;
const didTimeout =
scheduledCallbackTimeout !== -1 &&
scheduledCallbackTimeout < elapsedTimeInMs;
cb(didTimeout);
}
const values = yieldedValues;
yieldedValues = [];
return values;
}

function flushNumberOfYields(count: number): Array<mixed> {
expectedNumberOfYields = count;
didStop = false;
yieldedValues = [];
try {
while (scheduledCallback !== null && !didStop) {
const cb = scheduledCallback;
scheduledCallback = null;
const didTimeout =
scheduledCallbackTimeout !== -1 &&
scheduledCallbackTimeout < elapsedTimeInMs;
cb(didTimeout);
}
return yieldedValues;
} finally {
expectedNumberOfYields = -1;
didStop = false;
yieldedValues = [];
}
}

function yieldValue(value: mixed): void {
yieldedValues.push(value);
}

function clearYields(): Array<mixed> {
const values = yieldedValues;
yieldedValues = [];
return values;
}

function childToJSX(child, text) {
if (text !== null) {
return text;
Expand Down Expand Up @@ -653,6 +541,8 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
}

const ReactNoop = {
_Scheduler: Scheduler,

getChildren(rootID: string = DEFAULT_ROOT_ID) {
const container = rootContainers.get(rootID);
if (container) {
Expand Down Expand Up @@ -763,14 +653,9 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
return NoopRenderer.findHostInstance(component);
},

// TODO: Should only be used via a Jest plugin (like we do with the
// test renderer).
unstable_flushWithoutYielding: flushAll,
unstable_flushNumberOfYields: flushNumberOfYields,
unstable_clearYields: clearYields,
flushNextYield(): Array<mixed> {
return flushNumberOfYields(1);
Scheduler.unstable_flushNumberOfYields(1);
return Scheduler.unstable_clearYields();
},

flushWithHostCounters(
Expand All @@ -788,7 +673,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
hostUpdateCounter = 0;
hostCloneCounter = 0;
try {
flushAll();
Scheduler.flushAll();
return useMutation
? {
hostDiffCounter,
Expand All @@ -805,24 +690,13 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
}
},

expire(ms: number): Array<mixed> {
ReactNoop.advanceTime(ms);
return ReactNoop.flushExpired();
},
advanceTime(ms: number): void {
elapsedTimeInMs += ms;
},
expire: Scheduler.advanceTime,

flushExpired(): Array<mixed> {
return flushNumberOfYields(0);
return Scheduler.unstable_flushExpired();
},

yield: yieldValue,
hasScheduledCallback() {
return !!scheduledCallback;
},
yield: Scheduler.yieldValue,

batchedUpdates: NoopRenderer.batchedUpdates,

Expand Down Expand Up @@ -870,9 +744,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
},

flushSync(fn: () => mixed) {
yieldedValues = [];
NoopRenderer.flushSync(fn);
return yieldedValues;
},

flushPassiveEffects() {
Expand Down Expand Up @@ -997,12 +869,13 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
_next: null,
};
root.firstBatch = batch;
const actual = flushAll();
Scheduler.unstable_flushWithoutYielding();
const actual = Scheduler.unstable_clearYields();
expect(actual).toEqual(expectedFlush);
return (expectedCommit: Array<mixed>) => {
batch._defer = false;
NoopRenderer.flushRoot(root, expiration);
expect(yieldedValues).toEqual(expectedCommit);
expect(Scheduler.unstable_clearYields()).toEqual(expectedCommit);
};
},

Expand Down
67 changes: 44 additions & 23 deletions packages/react-reconciler/src/__tests__/ReactExpiration-test.internal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
let React;
let ReactFeatureFlags;
let ReactNoop;
let Scheduler;

describe('ReactExpiration', () => {
beforeEach(() => {
Expand All @@ -20,6 +21,7 @@ describe('ReactExpiration', () => {
ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false;
React = require('react');
ReactNoop = require('react-noop-renderer');
Scheduler = require('scheduler');
});

function span(prop) {
Expand Down Expand Up @@ -60,19 +62,33 @@ describe('ReactExpiration', () => {
}
}

function interrupt() {
ReactNoop.flushSync(() => {
ReactNoop.renderToRootWithID(null, 'other-root');
});
}

// First, show what happens for updates in two separate events.
// Schedule an update.
ReactNoop.render(<Text text="A" />);
// Advance the timer and flush any work that expired. Flushing the expired
// work signals to the renderer that the event has ended.
ReactNoop.advanceTime(2000);
// Advance the timer.
Scheduler.advanceTime(2000);
// Partially flush the the first update, then interrupt it.
expect(ReactNoop).toFlushAndYieldThrough(['A [render]']);
interrupt();

// Don't advance time by enough to expire the first update.
expect(ReactNoop.flushExpired()).toEqual([]);
expect(Scheduler).toHaveYielded([]);
expect(ReactNoop.getChildren()).toEqual([]);

// Schedule another update.
ReactNoop.render(<Text text="B" />);
// The updates should flush in separate batches, since sufficient time
// passed in between them *and* they occurred in separate events.
// Note: This isn't necessarily the ideal behavior. It might be better to
// batch these two updates together. The fact that they aren't batched
// is an implementation detail. The important part of this unit test is that
// they are batched if it's possible that they happened in the same event.
expect(ReactNoop).toFlushAndYield([
'A [render]',
'A [commit]',
Expand All @@ -84,10 +100,7 @@ describe('ReactExpiration', () => {
// Now do the same thing again, except this time don't flush any work in
// between the two updates.
ReactNoop.render(<Text text="A" />);
// Advance the timer, but don't flush the expired work. Because we still
// haven't entered an idle callback, the scheduler must assume that we're
// inside the same event.
ReactNoop.advanceTime(2000);
Scheduler.advanceTime(2000);
expect(ReactNoop).toHaveYielded([]);
expect(ReactNoop.getChildren()).toEqual([span('B')]);
// Schedule another update.
Expand All @@ -114,19 +127,33 @@ describe('ReactExpiration', () => {
}
}

function interrupt() {
ReactNoop.flushSync(() => {
ReactNoop.renderToRootWithID(null, 'other-root');
});
}

// First, show what happens for updates in two separate events.
// Schedule an update.
ReactNoop.render(<Text text="A" />);
// Advance the timer and flush any work that expired. Flushing the expired
// work signals to the renderer that the event has ended.
ReactNoop.advanceTime(2000);
// Advance the timer.
Scheduler.advanceTime(2000);
// Partially flush the the first update, then interrupt it.
expect(ReactNoop).toFlushAndYieldThrough(['A [render]']);
interrupt();

// Don't advance time by enough to expire the first update.
expect(ReactNoop.flushExpired()).toEqual([]);
expect(Scheduler).toHaveYielded([]);
expect(ReactNoop.getChildren()).toEqual([]);

// Schedule another update.
ReactNoop.render(<Text text="B" />);
// The updates should flush in separate batches, since sufficient time
// passed in between them *and* they occurred in separate events.
// Note: This isn't necessarily the ideal behavior. It might be better to
// batch these two updates together. The fact that they aren't batched
// is an implementation detail. The important part of this unit test is that
// they are batched if it's possible that they happened in the same event.
expect(ReactNoop).toFlushAndYield([
'A [render]',
'A [commit]',
Expand All @@ -138,21 +165,15 @@ describe('ReactExpiration', () => {
// Now do the same thing again, except this time don't flush any work in
// between the two updates.
ReactNoop.render(<Text text="A" />);
// Advance the timer, but don't flush the expired work. Because we still
// haven't entered an idle callback, the scheduler must assume that we're
// inside the same event.
ReactNoop.advanceTime(2000);
Scheduler.advanceTime(2000);
expect(ReactNoop).toHaveYielded([]);
expect(ReactNoop.getChildren()).toEqual([span('B')]);

// Perform some synchronous work. Again, the scheduler must assume we're
// inside the same event.
ReactNoop.flushSync(() => {
ReactNoop.renderToRootWithID('1', 'second-root');
});
// Perform some synchronous work. The scheduler must assume we're inside
// the same event.
interrupt();

// Even though React flushed a sync update, it should not have updated the
// current time. Schedule another update.
// Schedule another update.
ReactNoop.render(<Text text="B" />);
// The updates should flush in the same batch, since as far as the scheduler
// knows, they may have occurred inside the same event.
Expand Down
Loading

0 comments on commit 53e787b

Please sign in to comment.