Skip to content

Commit

Permalink
Warn when rendering tests in concurrent/batched mode without a mocked…
Browse files Browse the repository at this point in the history 
… scheduler

Concurrent/Batched mode tests should always be run with a mocked scheduler (v17 or not). This PR adds a warning for the same. I'll put up a separate PR to the docs with a page detailing how to mock the scheduler.
  • Loading branch information
@threepointone
threepointone committed Jul 25, 2019
1 parent 5b08f7b commit eb9908c
Show file tree
Hide file tree
Showing 7 changed files with 216 additions and 111 deletions.
141 changes: 93 additions & 48 deletions fixtures/dom/src/index.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,23 +24,7 @@ function App(props) {
return 'hello world';
}

describe('legacy mode', () => {
runTests();
});

describe('mocked scheduler', () => {
beforeEach(() => {
jest.mock('scheduler', () =>
require.requireActual('scheduler/unstable_mock')
);
});
afterEach(() => {
jest.unmock('scheduler');
});
runTests();
});

function runTests() {
describe('wrong act warnings', () => {
beforeEach(() => {
jest.resetModules();
React = require('react');
Expand Down Expand Up @@ -111,17 +95,6 @@ function runTests() {
});
});

it('warns when using createRoot() + .render', () => {
const root = ReactDOM.unstable_createRoot(document.createElement('div'));
expect(() => {
TestRenderer.act(() => {
root.render(<App />);
});
}).toWarnDev(["It looks like you're using the wrong act()"], {
withoutStack: true,
});
});

it('warns when using the wrong act version - test + dom: render', () => {
expect(() => {
TestRenderer.act(() => {
Expand Down Expand Up @@ -203,31 +176,103 @@ function runTests() {
});
});

it('flushes work only outside the outermost act(), even when nested from different renderers', () => {
const log = [];
function Effecty() {
React.useEffect(() => {
log.push('called');
}, []);
return null;
}
// in legacy mode, this tests whether an act only flushes its own effects
// with a mocked scheduler, this tests whether it flushes all work only on the outermost act
TestRenderer.act(() => {
it('warns when using createRoot() + .render', () => {
const root = ReactDOM.unstable_createRoot(document.createElement('div'));
expect(() => {
TestRenderer.act(() => {
root.render(<App />);
});
}).toWarnDev(
[
'In Concurrent or Sync modes, the "scheduler" module needs to be mocked',
"It looks like you're using the wrong act()",
],
{
withoutStack: true,
}
);
});
});

describe('flush work on nested act', () => {
describe('unmocked scheduler', () => {
beforeEach(() => {
jest.resetModules();
React = require('react');
ReactDOM = require('react-dom');
TestUtils = require('react-dom/test-utils');
TestRenderer = require('react-test-renderer');
});

it('flushes work only outside the outermost act(), even when nested from different renderers', () => {
const log = [];
function Effecty() {
React.useEffect(() => {
log.push('called');
}, []);
return null;
}
// in legacy mode, this tests whether an act only flushes its own effects
TestRenderer.act(() => {
TestUtils.act(() => {
TestRenderer.create(<Effecty />);
});
expect(log).toEqual([]);
});
expect(log).toEqual(['called']);

log.splice(0);
// for doublechecking, we flip it inside out, and assert on the outermost
TestUtils.act(() => {
TestRenderer.create(<Effecty />);
TestRenderer.act(() => {
TestRenderer.create(<Effecty />);
});
});
expect(log).toEqual([]);
expect(log).toEqual(['called']);
});
expect(log).toEqual(['called']);
});

log.splice(0);
// for doublechecking, we flip it inside out, and assert on the outermost
TestUtils.act(() => {
describe('mocked scheduler', () => {
beforeEach(() => {
jest.resetModules();
jest.mock('scheduler', () =>
require.requireActual('scheduler/unstable_mock')
);
React = require('react');
ReactDOM = require('react-dom');
TestUtils = require('react-dom/test-utils');
TestRenderer = require('react-test-renderer');
});

afterEach(() => {
jest.unmock('scheduler');
});

it('flushes work only outside the outermost act(), even when nested from different renderers', () => {
const log = [];
function Effecty() {
React.useEffect(() => {
log.push('called');
}, []);
return null;
}
// with a mocked scheduler, this tests whether it flushes all work only on the outermost act
TestRenderer.act(() => {
TestRenderer.create(<Effecty />);
TestUtils.act(() => {
TestRenderer.create(<Effecty />);
});
expect(log).toEqual([]);
});
expect(log).toEqual(['called']);

log.splice(0);
// for doublechecking, we flip it inside out, and assert on the outermost
TestUtils.act(() => {
TestRenderer.act(() => {
TestRenderer.create(<Effecty />);
});
});
expect(log).toEqual(['called']);
});
expect(log).toEqual(['called']);
});
}
});
101 changes: 86 additions & 15 deletions packages/react-dom/src/__tests__/ReactTestUtilsAct-test.internal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,93 @@
* @emails react-core
*/

let React;
let ReactDOM;
let ReactFeatureFlags;
let act;
describe('mocked scheduler', () => {
beforeEach(() => {
jest.resetModules();
ReactFeatureFlags = require('shared/ReactFeatureFlags');
ReactFeatureFlags.warnAboutMissingMockScheduler = true;
jest.unmock('scheduler');
act = require('react-dom/test-utils').act;

jest.unmock('scheduler');

function App() {
return null;
}

describe('unmocked scheduler warning', () => {
describe('warns on mount', () => {
beforeEach(() => {
jest.resetModules();
React = require('react');
ReactDOM = require('react-dom');
});

it('does not warn when rendering in sync mode', () => {
expect(() => {
ReactDOM.render(<App />, document.createElement('div'));
}).toWarnDev([]);
});

it('should warn when rendering in concurrent mode', () => {
expect(() => {
ReactDOM.unstable_createRoot(document.createElement('div')).render(
<App />,
);
}).toWarnDev(
'In Concurrent or Sync modes, the "scheduler" module needs to be mocked ' +
'to guarantee consistent behaviour across tests and browsers.',
{withoutStack: true},
);
// does not warn twice
expect(() => {
ReactDOM.unstable_createRoot(document.createElement('div')).render(
<App />,
);
}).toWarnDev([]);
});

it('should warn when rendering in batched mode', () => {
expect(() => {
ReactDOM.unstable_createSyncRoot(document.createElement('div')).render(
<App />,
);
}).toWarnDev(
'In Concurrent or Sync modes, the "scheduler" module needs to be mocked ' +
'to guarantee consistent behaviour across tests and browsers.',
{withoutStack: true},
);
// does not warn twice
expect(() => {
ReactDOM.unstable_createSyncRoot(document.createElement('div')).render(
<App />,
);
}).toWarnDev([]);
});
});
it("should warn when the scheduler isn't mocked", () => {
expect(() => act(() => {})).toWarnDev(
[
'Starting from React v17, the "scheduler" module will need to be mocked',
],
{withoutStack: true},
);

describe('warns with the feature flag', () => {
beforeEach(() => {
jest.resetModules();
React = require('react');
ReactDOM = require('react-dom');
ReactFeatureFlags = require('shared/ReactFeatureFlags');
ReactFeatureFlags.warnAboutMissingMockScheduler = true;
});

afterEach(() => {
ReactFeatureFlags.warnAboutMissingMockScheduler = false;
});

it('should warn in sync mode when the feature flag is enabled', () => {
expect(() => {
ReactDOM.render(<App />, document.createElement('div'));
}).toWarnDev(
[
'Starting from React v17, the "scheduler" module will need to be mocked',
],
{withoutStack: true},
);
// does not warn twice
expect(() => {
ReactDOM.render(<App />, document.createElement('div'));
}).toWarnDev([]);
});
});
});
16 changes: 0 additions & 16 deletions packages/react-dom/src/test-utils/ReactTestUtilsAct.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import type {Thenable} from 'react-reconciler/src/ReactFiberWorkLoop';
import warningWithoutStack from 'shared/warningWithoutStack';
import ReactDOM from 'react-dom';
import ReactSharedInternals from 'shared/ReactSharedInternals';
import {warnAboutMissingMockScheduler} from 'shared/ReactFeatureFlags';
import enqueueTask from 'shared/enqueueTask';
import * as Scheduler from 'scheduler';

Expand Down Expand Up @@ -43,26 +42,11 @@ const {IsSomeRendererActing} = ReactSharedInternals;
// this implementation should be exactly the same in
// ReactTestUtilsAct.js, ReactTestRendererAct.js, createReactNoop.js

let hasWarnedAboutMissingMockScheduler = false;
const isSchedulerMocked =
typeof Scheduler.unstable_flushAllWithoutAsserting === 'function';
const flushWork =
Scheduler.unstable_flushAllWithoutAsserting ||
function() {
if (warnAboutMissingMockScheduler === true) {
if (hasWarnedAboutMissingMockScheduler === false) {
warningWithoutStack(
null,
'Starting from React v17, the "scheduler" module will need to be mocked ' +
'to guarantee consistent behaviour across tests and browsers. To fix this, add the following ' +
"to the top of your tests, or in your framework's global config file -\n\n" +
'As an example, for jest - \n' +
"jest.mock('scheduler', () => require.requireActual('scheduler/unstable_mock'));\n\n" +
'For more info, visit https://fb.me/react-mock-scheduler',
);
hasWarnedAboutMissingMockScheduler = true;
}
}
while (flushPassiveEffects()) {}
};

Expand Down
16 changes: 0 additions & 16 deletions packages/react-noop-renderer/src/createReactNoop.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import {REACT_FRAGMENT_TYPE, REACT_ELEMENT_TYPE} from 'shared/ReactSymbols';
import enqueueTask from 'shared/enqueueTask';
import ReactSharedInternals from 'shared/ReactSharedInternals';
import warningWithoutStack from 'shared/warningWithoutStack';
import {warnAboutMissingMockScheduler} from 'shared/ReactFeatureFlags';
import {ConcurrentRoot, BatchedRoot, LegacyRoot} from 'shared/ReactRootTags';

type Container = {
Expand Down Expand Up @@ -599,26 +598,11 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
// this act() implementation should be exactly the same in
// ReactTestUtilsAct.js, ReactTestRendererAct.js, createReactNoop.js

let hasWarnedAboutMissingMockScheduler = false;
const isSchedulerMocked =
typeof Scheduler.unstable_flushAllWithoutAsserting === 'function';
const flushWork =
Scheduler.unstable_flushAllWithoutAsserting ||
function() {
if (warnAboutMissingMockScheduler === true) {
if (hasWarnedAboutMissingMockScheduler === false) {
warningWithoutStack(
null,
'Starting from React v17, the "scheduler" module will need to be mocked ' +
'to guarantee consistent behaviour across tests and browsers. To fix this, add the following ' +
"to the top of your tests, or in your framework's global config file -\n\n" +
'As an example, for jest - \n' +
"jest.mock('scheduler', () => require.requireActual('scheduler/unstable_mock'));\n\n" +
'For more info, visit https://fb.me/react-mock-scheduler',
);
hasWarnedAboutMissingMockScheduler = true;
}
}
while (flushPassiveEffects()) {}
};

Expand Down
2 changes: 2 additions & 0 deletions packages/react-reconciler/src/ReactFiberReconciler.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ import {
flushDiscreteUpdates,
flushPassiveEffects,
warnIfNotScopedWithMatchingAct,
warnIfNotMockedSchedulerInConcurrentOrBatchedMode,
IsThisRendererActing,
} from './ReactFiberWorkLoop';
import {createUpdate, enqueueUpdate} from './ReactUpdateQueue';
Expand Down Expand Up @@ -314,6 +315,7 @@ export function updateContainer(
if (__DEV__) {
// $FlowExpectedError - jest isn't a global, and isn't recognized outside of tests
if ('undefined' !== typeof jest) {
warnIfNotMockedSchedulerInConcurrentOrBatchedMode(current);
warnIfNotScopedWithMatchingAct(current);
}
}
Expand Down
Loading

0 comments on commit eb9908c

Please sign in to comment.