Skip to content

Commit

Permalink
Use native event dispatching instead of Simulate or SimulateNative (#…
Browse files Browse the repository at this point in the history
…13023)

* Use native event dispatching instead of Simulate or SimulateNative

In #12629 @gaearon suggested that it would be better to drop usage of
`ReactTestUtils.Simulate` and `ReactTestUtils.SimulateNative`. In this
PR I’m attempting at removing it from a lot of places with only a few
leftovers.

Those leftovers can be categorized into three groups:

1. Anything that tests that `SimulateNative` throws. This is a property
   that native event dispatching doesn’t have so I can’t convert that
   easily. Affected test suites: `EventPluginHub-test`,
   `ReactBrowserEventEmitter-test`.
2. Anything that tests `ReactTestUtils` directly. Affected test suites:
   `ReactBrowserEventEmitter-test` (this file has one test that reads
    "should have mouse enter simulated by test utils"),
    `ReactTestUtils-test`.
3. Anything that dispatches a `change` event. The reason here goes a bit
   deeper and is rooted in the way we shim onChange. Usually when using
   native event dispatching, you would set the node’s `.value` and then
   dispatch the event. However inside [`inputValueTracking.js`][] we
   install a setter on the node’s `.value` that will ignore the next
   `change` event (I found [this][near-perfect-oninput-shim] article
   from Sophie that explains that this is to avoid onChange when
   updating the value via JavaScript).

All remaining usages of `Simulate` or `SimulateNative` can be avoided
by mounting the containers inside the `document` and dispatching native
events.

Here some remarks:

1. I’m using `Element#click()` instead of `dispatchEvent`. In the jsdom
   changelog I read that `click()` now properly sets the correct values
   (you can also verify it does the same thing by looking at the
   [source][jsdom-source]).
2. I had to update jsdom in order to get `TouchEvent` constructors
   working (and while doing so also updated jest). There was one
   unexpected surprise: `ReactScheduler-test` was relying on not having
   `window.performance` available. I’ve recreated the previous
   environment by deleting this property from the global object.
3. I was a bit confused that `ReactTestUtils.renderIntoDocument()` does
   not render into the document 🤷‍

[`inputValueTracking.js`]: https://github.com/facebook/react/blob/392530104c00c25074ce38e1f7e1dd363018c7ce/packages/react-dom/src/client/inputValueTracking.js#L79
[near-perfect-oninput-shim]: https://sophiebits.com/2013/06/18/a-near-perfect-oninput-shim-for-ie-8-and-9.html
[jsdom-source]: https://github.com/jsdom/jsdom/blob/45b77f5d21cef74cad278d089937d8462c29acce/lib/jsdom/living/nodes/HTMLElement-impl.js#L43-L76

* Make sure contains are unlinked from the document even if the test fails

* Remove unnecessary findDOMNode calls
  • Loading branch information
philipp-spiess authored and gaearon committed Jun 13, 2018
1 parent 945fc1b commit 036ae3c
Show file tree
Hide file tree
Showing 23 changed files with 959 additions and 832 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@
"gzip-js": "~0.3.2",
"gzip-size": "^3.0.0",
"jasmine-check": "^1.0.0-rc.0",
"jest": "^23.0.1",
"jest": "^23.1.0",
"jest-diff": "^23.0.1",
"merge-stream": "^1.0.0",
"minimatch": "^3.0.4",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ let getListener;
let putListener;
let deleteAllListeners;

let container;

function registerSimpleTestHandler() {
putListener(CHILD, ON_CLICK_KEY, LISTENER);
const listener = getListener(CHILD, ON_CLICK_KEY);
Expand All @@ -63,7 +65,8 @@ describe('ReactBrowserEventEmitter', () => {
ReactBrowserEventEmitter = require('../events/ReactBrowserEventEmitter');
ReactTestUtils = require('react-dom/test-utils');

const container = document.createElement('div');
container = document.createElement('div');
document.body.appendChild(container);

let GRANDPARENT_PROPS = {};
let PARENT_PROPS = {};
Expand Down Expand Up @@ -129,6 +132,11 @@ describe('ReactBrowserEventEmitter', () => {
idCallOrder = [];
});

afterEach(() => {
document.body.removeChild(container);
container = null;
});

it('should store a listener correctly', () => {
registerSimpleTestHandler();
const listener = getListener(CHILD, ON_CLICK_KEY);
Expand All @@ -150,25 +158,25 @@ describe('ReactBrowserEventEmitter', () => {

it('should invoke a simple handler registered on a node', () => {
registerSimpleTestHandler();
ReactTestUtils.Simulate.click(CHILD);
CHILD.click();
expect(LISTENER).toHaveBeenCalledTimes(1);
});

it('should not invoke handlers if ReactBrowserEventEmitter is disabled', () => {
registerSimpleTestHandler();
ReactBrowserEventEmitter.setEnabled(false);
ReactTestUtils.SimulateNative.click(CHILD);
CHILD.click();
expect(LISTENER).toHaveBeenCalledTimes(0);
ReactBrowserEventEmitter.setEnabled(true);
ReactTestUtils.SimulateNative.click(CHILD);
CHILD.click();
expect(LISTENER).toHaveBeenCalledTimes(1);
});

it('should bubble simply', () => {
putListener(CHILD, ON_CLICK_KEY, recordID.bind(null, CHILD));
putListener(PARENT, ON_CLICK_KEY, recordID.bind(null, PARENT));
putListener(GRANDPARENT, ON_CLICK_KEY, recordID.bind(null, GRANDPARENT));
ReactTestUtils.Simulate.click(CHILD);
CHILD.click();
expect(idCallOrder.length).toBe(3);
expect(idCallOrder[0]).toBe(CHILD);
expect(idCallOrder[1]).toBe(PARENT);
Expand All @@ -179,7 +187,7 @@ describe('ReactBrowserEventEmitter', () => {
putListener(GRANDPARENT, ON_CLICK_KEY, recordID.bind(null, 'GRANDPARENT'));
putListener(PARENT, ON_CLICK_KEY, recordID.bind(null, 'PARENT'));
putListener(CHILD, ON_CLICK_KEY, recordID.bind(null, 'CHILD'));
ReactTestUtils.Simulate.click(CHILD);
CHILD.click();
expect(idCallOrder).toEqual(['CHILD', 'PARENT', 'GRANDPARENT']);

idCallOrder = [];
Expand All @@ -191,7 +199,7 @@ describe('ReactBrowserEventEmitter', () => {
recordID.bind(null, 'UPDATED_GRANDPARENT'),
);

ReactTestUtils.Simulate.click(CHILD);
CHILD.click();
expect(idCallOrder).toEqual(['CHILD', 'PARENT', 'UPDATED_GRANDPARENT']);
});

Expand Down Expand Up @@ -224,7 +232,7 @@ describe('ReactBrowserEventEmitter', () => {
recordID(GRANDPARENT);
expect(event.currentTarget).toBe(GRANDPARENT);
});
ReactTestUtils.Simulate.click(CHILD);
CHILD.click();
expect(idCallOrder.length).toBe(3);
expect(idCallOrder[0]).toBe(CHILD);
expect(idCallOrder[1]).toBe(PARENT);
Expand All @@ -239,7 +247,7 @@ describe('ReactBrowserEventEmitter', () => {
recordIDAndStopPropagation.bind(null, PARENT),
);
putListener(GRANDPARENT, ON_CLICK_KEY, recordID.bind(null, GRANDPARENT));
ReactTestUtils.Simulate.click(CHILD);
CHILD.click();
expect(idCallOrder.length).toBe(2);
expect(idCallOrder[0]).toBe(CHILD);
expect(idCallOrder[1]).toBe(PARENT);
Expand All @@ -254,7 +262,7 @@ describe('ReactBrowserEventEmitter', () => {
e.isPropagationStopped = () => true;
});
putListener(GRANDPARENT, ON_CLICK_KEY, recordID.bind(null, GRANDPARENT));
ReactTestUtils.Simulate.click(CHILD);
CHILD.click();
expect(idCallOrder.length).toBe(2);
expect(idCallOrder[0]).toBe(CHILD);
expect(idCallOrder[1]).toBe(PARENT);
Expand All @@ -268,7 +276,7 @@ describe('ReactBrowserEventEmitter', () => {
);
putListener(PARENT, ON_CLICK_KEY, recordID.bind(null, PARENT));
putListener(GRANDPARENT, ON_CLICK_KEY, recordID.bind(null, GRANDPARENT));
ReactTestUtils.Simulate.click(CHILD);
CHILD.click();
expect(idCallOrder.length).toBe(1);
expect(idCallOrder[0]).toBe(CHILD);
});
Expand All @@ -277,7 +285,7 @@ describe('ReactBrowserEventEmitter', () => {
putListener(CHILD, ON_CLICK_KEY, recordIDAndReturnFalse.bind(null, CHILD));
putListener(PARENT, ON_CLICK_KEY, recordID.bind(null, PARENT));
putListener(GRANDPARENT, ON_CLICK_KEY, recordID.bind(null, GRANDPARENT));
ReactTestUtils.Simulate.click(CHILD);
CHILD.click();
expect(idCallOrder.length).toBe(3);
expect(idCallOrder[0]).toBe(CHILD);
expect(idCallOrder[1]).toBe(PARENT);
Expand All @@ -300,7 +308,7 @@ describe('ReactBrowserEventEmitter', () => {
};
putListener(CHILD, ON_CLICK_KEY, handleChildClick);
putListener(PARENT, ON_CLICK_KEY, handleParentClick);
ReactTestUtils.Simulate.click(CHILD);
CHILD.click();
expect(handleParentClick).toHaveBeenCalledTimes(1);
});

Expand All @@ -310,7 +318,7 @@ describe('ReactBrowserEventEmitter', () => {
putListener(PARENT, ON_CLICK_KEY, handleParentClick);
};
putListener(CHILD, ON_CLICK_KEY, handleChildClick);
ReactTestUtils.Simulate.click(CHILD);
CHILD.click();
expect(handleParentClick).toHaveBeenCalledTimes(0);
});

Expand Down
2 changes: 1 addition & 1 deletion packages/react-dom/src/__tests__/ReactComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ describe('ReactComponent', () => {
componentDidMount() {
// Check .props.title to make sure we got the right elements back
expect(this.wrapperRef.getTitle()).toBe('wrapper');
expect(ReactDOM.findDOMNode(this.innerRef).className).toBe('inner');
expect(this.innerRef.className).toBe('inner');
mounted = true;
}
}
Expand Down
26 changes: 16 additions & 10 deletions packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1065,16 +1065,22 @@ describe('ReactComponentLifeCycle', () => {
}
}

ReactTestUtils.renderIntoDocument(<Parent />);
expect(divRef.current.textContent).toBe('remote:0, local:0');

// Trigger setState() calls
childInstance.updateState();
expect(divRef.current.textContent).toBe('remote:1, local:1');

// Trigger batched setState() calls
ReactTestUtils.Simulate.click(divRef.current);
expect(divRef.current.textContent).toBe('remote:2, local:2');
const container = document.createElement('div');
document.body.appendChild(container);
try {
ReactDOM.render(<Parent />, container);
expect(divRef.current.textContent).toBe('remote:0, local:0');

// Trigger setState() calls
childInstance.updateState();
expect(divRef.current.textContent).toBe('remote:1, local:1');

// Trigger batched setState() calls
divRef.current.click();
expect(divRef.current.textContent).toBe('remote:2, local:2');
} finally {
document.body.removeChild(container);
}
});

it('should pass the return value from getSnapshotBeforeUpdate to componentDidUpdate', () => {
Expand Down
25 changes: 15 additions & 10 deletions packages/react-dom/src/__tests__/ReactCompositeComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,23 +140,28 @@ describe('ReactCompositeComponent', () => {
});

it('should react to state changes from callbacks', () => {
const instance = ReactTestUtils.renderIntoDocument(<MorphingComponent />);
let el = ReactDOM.findDOMNode(instance);
expect(el.tagName).toBe('A');

ReactTestUtils.Simulate.click(el);
el = ReactDOM.findDOMNode(instance);
expect(el.tagName).toBe('B');
const container = document.createElement('div');
document.body.appendChild(container);
try {
const instance = ReactDOM.render(<MorphingComponent />, container);
let el = ReactDOM.findDOMNode(instance);
expect(el.tagName).toBe('A');
el.click();
el = ReactDOM.findDOMNode(instance);
expect(el.tagName).toBe('B');
} finally {
document.body.removeChild(container);
}
});

it('should rewire refs when rendering to different child types', () => {
const instance = ReactTestUtils.renderIntoDocument(<MorphingComponent />);

expect(ReactDOM.findDOMNode(instance.refs.x).tagName).toBe('A');
expect(instance.refs.x.tagName).toBe('A');
instance._toggleActivatedState();
expect(ReactDOM.findDOMNode(instance.refs.x).tagName).toBe('B');
expect(instance.refs.x.tagName).toBe('B');
instance._toggleActivatedState();
expect(ReactDOM.findDOMNode(instance.refs.x).tagName).toBe('A');
expect(instance.refs.x.tagName).toBe('A');
});

it('should not cache old DOM nodes when switching constructors', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,11 @@

let React;
let ReactDOM;
let ReactTestUtils;

describe('ReactCompositeComponentNestedState-state', () => {
beforeEach(() => {
React = require('react');
ReactDOM = require('react-dom');
ReactTestUtils = require('react-dom/test-utils');
});

it('should provide up to date values for props', () => {
Expand Down Expand Up @@ -102,7 +100,7 @@ describe('ReactCompositeComponentNestedState-state', () => {
void ReactDOM.render(<ParentComponent logger={logger} />, container);

// click "light green"
ReactTestUtils.Simulate.click(container.childNodes[0].childNodes[3]);
container.childNodes[0].childNodes[3].click();

expect(logger.mock.calls).toEqual([
['parent-render', 'blue'],
Expand Down

0 comments on commit 036ae3c

Please sign in to comment.