Skip to content

Commit

Permalink
Swap expect(ReactNoop) for expect(Scheduler) (#14971)
Browse files Browse the repository at this point in the history
* Swap expect(ReactNoop) for expect(Scheduler)

In the previous commits, I upgraded our custom Jest matchers for the
noop and test renderers to use Scheduler under the hood.

Now that all these matchers are using Scheduler, we can drop
support for passing ReactNoop and test roots and always pass
Scheduler directly.

* Externalize Scheduler in noop and test bundles

I also noticed we don't need to regenerator runtime in noop anymore.
  • Loading branch information
acdlite committed Feb 28, 2019
1 parent ccb2a8a commit 69060e1
Show file tree
Hide file tree
Showing 35 changed files with 1,184 additions and 1,277 deletions.
Expand Up @@ -14,6 +14,7 @@ let BehaviorSubject;
let ReactFeatureFlags;
let React;
let ReactNoop;
let Scheduler;
let ReplaySubject;

describe('createSubscription', () => {
Expand All @@ -24,6 +25,7 @@ describe('createSubscription', () => {
ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false;
React = require('react');
ReactNoop = require('react-noop-renderer');
Scheduler = require('scheduler');

BehaviorSubject = require('rxjs/BehaviorSubject').BehaviorSubject;
ReplaySubject = require('rxjs/ReplaySubject').ReplaySubject;
Expand Down Expand Up @@ -65,16 +67,16 @@ describe('createSubscription', () => {
);

// Updates while subscribed should re-render the child component
expect(ReactNoop).toFlushAndYield(['default']);
expect(Scheduler).toFlushAndYield(['default']);
observable.next(123);
expect(ReactNoop).toFlushAndYield([123]);
expect(Scheduler).toFlushAndYield([123]);
observable.next('abc');
expect(ReactNoop).toFlushAndYield(['abc']);
expect(Scheduler).toFlushAndYield(['abc']);

// Unmounting the subscriber should remove listeners
ReactNoop.render(<div />);
observable.next(456);
expect(ReactNoop).toFlushAndYield([]);
expect(Scheduler).toFlushAndYield([]);
});

it('should support observable types like RxJS ReplaySubject', () => {
Expand Down Expand Up @@ -102,13 +104,13 @@ describe('createSubscription', () => {
const observable = createReplaySubject('initial');

ReactNoop.render(<Subscription source={observable}>{render}</Subscription>);
expect(ReactNoop).toFlushAndYield(['initial']);
expect(Scheduler).toFlushAndYield(['initial']);
observable.next('updated');
expect(ReactNoop).toFlushAndYield(['updated']);
expect(Scheduler).toFlushAndYield(['updated']);

// Unsetting the subscriber prop should reset subscribed values
ReactNoop.render(<Subscription>{render}</Subscription>);
expect(ReactNoop).toFlushAndYield(['default']);
expect(Scheduler).toFlushAndYield(['default']);
});

describe('Promises', () => {
Expand Down Expand Up @@ -141,19 +143,19 @@ describe('createSubscription', () => {

// Test a promise that resolves after render
ReactNoop.render(<Subscription source={promiseA}>{render}</Subscription>);
expect(ReactNoop).toFlushAndYield(['loading']);
expect(Scheduler).toFlushAndYield(['loading']);
resolveA(true);
await promiseA;
expect(ReactNoop).toFlushAndYield(['finished']);
expect(Scheduler).toFlushAndYield(['finished']);

// Test a promise that resolves before render
// Note that this will require an extra render anyway,
// Because there is no way to synchronously get a Promise's value
rejectB(false);
ReactNoop.render(<Subscription source={promiseB}>{render}</Subscription>);
expect(ReactNoop).toFlushAndYield(['loading']);
expect(Scheduler).toFlushAndYield(['loading']);
await promiseB.catch(() => true);
expect(ReactNoop).toFlushAndYield(['failed']);
expect(Scheduler).toFlushAndYield(['failed']);
});

it('should still work if unsubscription is managed incorrectly', async () => {
Expand All @@ -177,17 +179,17 @@ describe('createSubscription', () => {

// Subscribe first to Promise A then Promise B
ReactNoop.render(<Subscription source={promiseA}>{render}</Subscription>);
expect(ReactNoop).toFlushAndYield(['default']);
expect(Scheduler).toFlushAndYield(['default']);
ReactNoop.render(<Subscription source={promiseB}>{render}</Subscription>);
expect(ReactNoop).toFlushAndYield(['default']);
expect(Scheduler).toFlushAndYield(['default']);

// Resolve both Promises
resolveB(123);
resolveA('abc');
await Promise.all([promiseA, promiseB]);

// Ensure that only Promise B causes an update
expect(ReactNoop).toFlushAndYield([123]);
expect(Scheduler).toFlushAndYield([123]);
});

it('should not call setState for a Promise that resolves after unmount', async () => {
Expand All @@ -211,11 +213,11 @@ describe('createSubscription', () => {
});

ReactNoop.render(<Subscription source={promise}>{render}</Subscription>);
expect(ReactNoop).toFlushAndYield(['rendered']);
expect(Scheduler).toFlushAndYield(['rendered']);

// Unmount
ReactNoop.render(null);
expect(ReactNoop).toFlushWithoutYielding();
expect(Scheduler).toFlushWithoutYielding();

// Resolve Promise should not trigger a setState warning
resolvePromise(true);
Expand Down Expand Up @@ -245,21 +247,21 @@ describe('createSubscription', () => {
);

// Updates while subscribed should re-render the child component
expect(ReactNoop).toFlushAndYield(['a-0']);
expect(Scheduler).toFlushAndYield(['a-0']);

// Unsetting the subscriber prop should reset subscribed values
ReactNoop.render(
<Subscription source={observableB}>{render}</Subscription>,
);
expect(ReactNoop).toFlushAndYield(['b-0']);
expect(Scheduler).toFlushAndYield(['b-0']);

// Updates to the old subscribable should not re-render the child component
observableA.next('a-1');
expect(ReactNoop).toFlushAndYield([]);
expect(Scheduler).toFlushAndYield([]);

// Updates to the bew subscribable should re-render the child component
observableB.next('b-1');
expect(ReactNoop).toFlushAndYield(['b-1']);
expect(Scheduler).toFlushAndYield(['b-1']);
});

it('should ignore values emitted by a new subscribable until the commit phase', () => {
Expand Down Expand Up @@ -315,12 +317,12 @@ describe('createSubscription', () => {
const observableB = createBehaviorSubject('b-0');

ReactNoop.render(<Parent observed={observableA} />);
expect(ReactNoop).toFlushAndYield(['Subscriber: a-0', 'Child: a-0']);
expect(Scheduler).toFlushAndYield(['Subscriber: a-0', 'Child: a-0']);
expect(log).toEqual(['Parent.componentDidMount']);

// Start React update, but don't finish
ReactNoop.render(<Parent observed={observableB} />);
expect(ReactNoop).toFlushAndYieldThrough(['Subscriber: b-0']);
expect(Scheduler).toFlushAndYieldThrough(['Subscriber: b-0']);
expect(log).toEqual(['Parent.componentDidMount']);

// Emit some updates from the uncommitted subscribable
Expand All @@ -335,7 +337,7 @@ describe('createSubscription', () => {
// We expect the last emitted update to be rendered (because of the commit phase value check)
// But the intermediate ones should be ignored,
// And the final rendered output should be the higher-priority observable.
expect(ReactNoop).toFlushAndYield([
expect(Scheduler).toFlushAndYield([
'Child: b-0',
'Subscriber: b-3',
'Child: b-3',
Expand Down Expand Up @@ -402,12 +404,12 @@ describe('createSubscription', () => {
const observableB = createBehaviorSubject('b-0');

ReactNoop.render(<Parent observed={observableA} />);
expect(ReactNoop).toFlushAndYield(['Subscriber: a-0', 'Child: a-0']);
expect(Scheduler).toFlushAndYield(['Subscriber: a-0', 'Child: a-0']);
expect(log).toEqual(['Parent.componentDidMount']);

// Start React update, but don't finish
ReactNoop.render(<Parent observed={observableB} />);
expect(ReactNoop).toFlushAndYieldThrough(['Subscriber: b-0']);
expect(Scheduler).toFlushAndYieldThrough(['Subscriber: b-0']);
expect(log).toEqual(['Parent.componentDidMount']);

// Emit some updates from the old subscribable
Expand All @@ -420,7 +422,7 @@ describe('createSubscription', () => {
// Flush everything and ensure that the correct subscribable is used
// We expect the new subscribable to finish rendering,
// But then the updated values from the old subscribable should be used.
expect(ReactNoop).toFlushAndYield([
expect(Scheduler).toFlushAndYield([
'Child: b-0',
'Subscriber: a-2',
'Child: a-2',
Expand All @@ -433,7 +435,7 @@ describe('createSubscription', () => {

// Updates from the new subscribable should be ignored.
observableB.next('b-1');
expect(ReactNoop).toFlushAndYield([]);
expect(Scheduler).toFlushAndYield([]);
expect(log).toEqual([
'Parent.componentDidMount',
'Parent.componentDidUpdate',
Expand Down Expand Up @@ -479,7 +481,7 @@ describe('createSubscription', () => {
<Subscription source={observable}>{value => null}</Subscription>,
);

expect(ReactNoop).toFlushAndThrow(
expect(Scheduler).toFlushAndThrow(
'A subscription must return an unsubscribe function.',
);
});
Expand Down
48 changes: 0 additions & 48 deletions packages/jest-react/src/JestReact.js
Expand Up @@ -35,54 +35,6 @@ function assertYieldsWereCleared(root) {
);
}

export function unstable_toFlushAndYield(root, expectedYields) {
assertYieldsWereCleared(root);
const actualYields = root.unstable_flushAll();
return captureAssertion(() => {
expect(actualYields).toEqual(expectedYields);
});
}

export function unstable_toFlushAndYieldThrough(root, expectedYields) {
assertYieldsWereCleared(root);
const actualYields = root.unstable_flushNumberOfYields(expectedYields.length);
return captureAssertion(() => {
expect(actualYields).toEqual(expectedYields);
});
}

export function unstable_toFlushWithoutYielding(root) {
return unstable_toFlushAndYield(root, []);
}

export function unstable_toHaveYielded(ReactTestRenderer, expectedYields) {
return captureAssertion(() => {
if (
ReactTestRenderer === null ||
typeof ReactTestRenderer !== 'object' ||
typeof ReactTestRenderer.unstable_setNowImplementation !== 'function'
) {
invariant(
false,
'The matcher `unstable_toHaveYielded` expects an instance of React Test ' +
'Renderer.\n\nTry: ' +
'expect(ReactTestRenderer).unstable_toHaveYielded(expectedYields)',
);
}
const actualYields = ReactTestRenderer.unstable_clearYields();
expect(actualYields).toEqual(expectedYields);
});
}

export function unstable_toFlushAndThrow(root, ...rest) {
assertYieldsWereCleared(root);
return captureAssertion(() => {
expect(() => {
root.unstable_flushAll();
}).toThrow(...rest);
});
}

export function unstable_toMatchRenderedOutput(root, expectedJSX) {
assertYieldsWereCleared(root);
const actualJSON = root.toJSON();
Expand Down
5 changes: 3 additions & 2 deletions packages/react-art/src/__tests__/ReactART-test.js
Expand Up @@ -31,6 +31,7 @@ const Wedge = require('react-art/Wedge');
// Isolate the noop renderer
jest.resetModules();
const ReactNoop = require('react-noop-renderer');
const Scheduler = require('scheduler');

let Group;
let Shape;
Expand Down Expand Up @@ -385,7 +386,7 @@ describe('ReactART', () => {
</CurrentRendererContext.Provider>,
);

expect(ReactNoop).toFlushAndYieldThrough(['A']);
expect(Scheduler).toFlushAndYieldThrough(['A']);

ReactDOM.render(
<Surface>
Expand All @@ -400,7 +401,7 @@ describe('ReactART', () => {
expect(ops).toEqual([null, 'ART']);

ops = [];
expect(ReactNoop).toFlushAndYield(['B', 'C']);
expect(Scheduler).toFlushAndYield(['B', 'C']);

expect(ops).toEqual(['Test']);
});
Expand Down

0 comments on commit 69060e1

Please sign in to comment.