Skip to content

Commit 036ae3c

Browse files
philipp-spiessgaearon
philipp-spiess
authored and
gaearon
committed
Use native event dispatching instead of Simulate or SimulateNative (#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
1 parent 945fc1b commit 036ae3c

23 files changed

+959
-832
lines changed

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@
6969
"gzip-js": "~0.3.2",
7070
"gzip-size": "^3.0.0",
7171
"jasmine-check": "^1.0.0-rc.0",
72-
"jest": "^23.0.1",
72+
"jest": "^23.1.0",
7373
"jest-diff": "^23.0.1",
7474
"merge-stream": "^1.0.0",
7575
"minimatch": "^3.0.4",

‎packages/react-dom/src/__tests__/ReactBrowserEventEmitter-test.internal.js‎

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ let getListener;
4242
let putListener;
4343
let deleteAllListeners;
4444

45+
let container;
46+
4547
function registerSimpleTestHandler() {
4648
putListener(CHILD, ON_CLICK_KEY, LISTENER);
4749
const listener = getListener(CHILD, ON_CLICK_KEY);
@@ -63,7 +65,8 @@ describe('ReactBrowserEventEmitter', () => {
6365
ReactBrowserEventEmitter = require('../events/ReactBrowserEventEmitter');
6466
ReactTestUtils = require('react-dom/test-utils');
6567

66-
const container = document.createElement('div');
68+
container = document.createElement('div');
69+
document.body.appendChild(container);
6770

6871
let GRANDPARENT_PROPS = {};
6972
let PARENT_PROPS = {};
@@ -129,6 +132,11 @@ describe('ReactBrowserEventEmitter', () => {
129132
idCallOrder = [];
130133
});
131134

135+
afterEach(() => {
136+
document.body.removeChild(container);
137+
container = null;
138+
});
139+
132140
it('should store a listener correctly', () => {
133141
registerSimpleTestHandler();
134142
const listener = getListener(CHILD, ON_CLICK_KEY);
@@ -150,25 +158,25 @@ describe('ReactBrowserEventEmitter', () => {
150158

151159
it('should invoke a simple handler registered on a node', () => {
152160
registerSimpleTestHandler();
153-
ReactTestUtils.Simulate.click(CHILD);
161+
CHILD.click();
154162
expect(LISTENER).toHaveBeenCalledTimes(1);
155163
});
156164

157165
it('should not invoke handlers if ReactBrowserEventEmitter is disabled', () => {
158166
registerSimpleTestHandler();
159167
ReactBrowserEventEmitter.setEnabled(false);
160-
ReactTestUtils.SimulateNative.click(CHILD);
168+
CHILD.click();
161169
expect(LISTENER).toHaveBeenCalledTimes(0);
162170
ReactBrowserEventEmitter.setEnabled(true);
163-
ReactTestUtils.SimulateNative.click(CHILD);
171+
CHILD.click();
164172
expect(LISTENER).toHaveBeenCalledTimes(1);
165173
});
166174

167175
it('should bubble simply', () => {
168176
putListener(CHILD, ON_CLICK_KEY, recordID.bind(null, CHILD));
169177
putListener(PARENT, ON_CLICK_KEY, recordID.bind(null, PARENT));
170178
putListener(GRANDPARENT, ON_CLICK_KEY, recordID.bind(null, GRANDPARENT));
171-
ReactTestUtils.Simulate.click(CHILD);
179+
CHILD.click();
172180
expect(idCallOrder.length).toBe(3);
173181
expect(idCallOrder[0]).toBe(CHILD);
174182
expect(idCallOrder[1]).toBe(PARENT);
@@ -179,7 +187,7 @@ describe('ReactBrowserEventEmitter', () => {
179187
putListener(GRANDPARENT, ON_CLICK_KEY, recordID.bind(null, 'GRANDPARENT'));
180188
putListener(PARENT, ON_CLICK_KEY, recordID.bind(null, 'PARENT'));
181189
putListener(CHILD, ON_CLICK_KEY, recordID.bind(null, 'CHILD'));
182-
ReactTestUtils.Simulate.click(CHILD);
190+
CHILD.click();
183191
expect(idCallOrder).toEqual(['CHILD', 'PARENT', 'GRANDPARENT']);
184192

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

194-
ReactTestUtils.Simulate.click(CHILD);
202+
CHILD.click();
195203
expect(idCallOrder).toEqual(['CHILD', 'PARENT', 'UPDATED_GRANDPARENT']);
196204
});
197205

@@ -224,7 +232,7 @@ describe('ReactBrowserEventEmitter', () => {
224232
recordID(GRANDPARENT);
225233
expect(event.currentTarget).toBe(GRANDPARENT);
226234
});
227-
ReactTestUtils.Simulate.click(CHILD);
235+
CHILD.click();
228236
expect(idCallOrder.length).toBe(3);
229237
expect(idCallOrder[0]).toBe(CHILD);
230238
expect(idCallOrder[1]).toBe(PARENT);
@@ -239,7 +247,7 @@ describe('ReactBrowserEventEmitter', () => {
239247
recordIDAndStopPropagation.bind(null, PARENT),
240248
);
241249
putListener(GRANDPARENT, ON_CLICK_KEY, recordID.bind(null, GRANDPARENT));
242-
ReactTestUtils.Simulate.click(CHILD);
250+
CHILD.click();
243251
expect(idCallOrder.length).toBe(2);
244252
expect(idCallOrder[0]).toBe(CHILD);
245253
expect(idCallOrder[1]).toBe(PARENT);
@@ -254,7 +262,7 @@ describe('ReactBrowserEventEmitter', () => {
254262
e.isPropagationStopped = () => true;
255263
});
256264
putListener(GRANDPARENT, ON_CLICK_KEY, recordID.bind(null, GRANDPARENT));
257-
ReactTestUtils.Simulate.click(CHILD);
265+
CHILD.click();
258266
expect(idCallOrder.length).toBe(2);
259267
expect(idCallOrder[0]).toBe(CHILD);
260268
expect(idCallOrder[1]).toBe(PARENT);
@@ -268,7 +276,7 @@ describe('ReactBrowserEventEmitter', () => {
268276
);
269277
putListener(PARENT, ON_CLICK_KEY, recordID.bind(null, PARENT));
270278
putListener(GRANDPARENT, ON_CLICK_KEY, recordID.bind(null, GRANDPARENT));
271-
ReactTestUtils.Simulate.click(CHILD);
279+
CHILD.click();
272280
expect(idCallOrder.length).toBe(1);
273281
expect(idCallOrder[0]).toBe(CHILD);
274282
});
@@ -277,7 +285,7 @@ describe('ReactBrowserEventEmitter', () => {
277285
putListener(CHILD, ON_CLICK_KEY, recordIDAndReturnFalse.bind(null, CHILD));
278286
putListener(PARENT, ON_CLICK_KEY, recordID.bind(null, PARENT));
279287
putListener(GRANDPARENT, ON_CLICK_KEY, recordID.bind(null, GRANDPARENT));
280-
ReactTestUtils.Simulate.click(CHILD);
288+
CHILD.click();
281289
expect(idCallOrder.length).toBe(3);
282290
expect(idCallOrder[0]).toBe(CHILD);
283291
expect(idCallOrder[1]).toBe(PARENT);
@@ -300,7 +308,7 @@ describe('ReactBrowserEventEmitter', () => {
300308
};
301309
putListener(CHILD, ON_CLICK_KEY, handleChildClick);
302310
putListener(PARENT, ON_CLICK_KEY, handleParentClick);
303-
ReactTestUtils.Simulate.click(CHILD);
311+
CHILD.click();
304312
expect(handleParentClick).toHaveBeenCalledTimes(1);
305313
});
306314

@@ -310,7 +318,7 @@ describe('ReactBrowserEventEmitter', () => {
310318
putListener(PARENT, ON_CLICK_KEY, handleParentClick);
311319
};
312320
putListener(CHILD, ON_CLICK_KEY, handleChildClick);
313-
ReactTestUtils.Simulate.click(CHILD);
321+
CHILD.click();
314322
expect(handleParentClick).toHaveBeenCalledTimes(0);
315323
});
316324

‎packages/react-dom/src/__tests__/ReactComponent-test.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ describe('ReactComponent', () => {
278278
componentDidMount() {
279279
// Check .props.title to make sure we got the right elements back
280280
expect(this.wrapperRef.getTitle()).toBe('wrapper');
281-
expect(ReactDOM.findDOMNode(this.innerRef).className).toBe('inner');
281+
expect(this.innerRef.className).toBe('inner');
282282
mounted = true;
283283
}
284284
}

‎packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1065,16 +1065,22 @@ describe('ReactComponentLifeCycle', () => {
10651065
}
10661066
}
10671067

1068-
ReactTestUtils.renderIntoDocument(<Parent />);
1069-
expect(divRef.current.textContent).toBe('remote:0, local:0');
1070-
1071-
// Trigger setState() calls
1072-
childInstance.updateState();
1073-
expect(divRef.current.textContent).toBe('remote:1, local:1');
1074-
1075-
// Trigger batched setState() calls
1076-
ReactTestUtils.Simulate.click(divRef.current);
1077-
expect(divRef.current.textContent).toBe('remote:2, local:2');
1068+
const container = document.createElement('div');
1069+
document.body.appendChild(container);
1070+
try {
1071+
ReactDOM.render(<Parent />, container);
1072+
expect(divRef.current.textContent).toBe('remote:0, local:0');
1073+
1074+
// Trigger setState() calls
1075+
childInstance.updateState();
1076+
expect(divRef.current.textContent).toBe('remote:1, local:1');
1077+
1078+
// Trigger batched setState() calls
1079+
divRef.current.click();
1080+
expect(divRef.current.textContent).toBe('remote:2, local:2');
1081+
} finally {
1082+
document.body.removeChild(container);
1083+
}
10781084
});
10791085

10801086
it('should pass the return value from getSnapshotBeforeUpdate to componentDidUpdate', () => {

‎packages/react-dom/src/__tests__/ReactCompositeComponent-test.js‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -140,23 +140,28 @@ describe('ReactCompositeComponent', () => {
140140
});
141141

142142
it('should react to state changes from callbacks', () => {
143-
const instance = ReactTestUtils.renderIntoDocument(<MorphingComponent />);
144-
let el = ReactDOM.findDOMNode(instance);
145-
expect(el.tagName).toBe('A');
146-
147-
ReactTestUtils.Simulate.click(el);
148-
el = ReactDOM.findDOMNode(instance);
149-
expect(el.tagName).toBe('B');
143+
const container = document.createElement('div');
144+
document.body.appendChild(container);
145+
try {
146+
const instance = ReactDOM.render(<MorphingComponent />, container);
147+
let el = ReactDOM.findDOMNode(instance);
148+
expect(el.tagName).toBe('A');
149+
el.click();
150+
el = ReactDOM.findDOMNode(instance);
151+
expect(el.tagName).toBe('B');
152+
} finally {
153+
document.body.removeChild(container);
154+
}
150155
});
151156

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

155-
expect(ReactDOM.findDOMNode(instance.refs.x).tagName).toBe('A');
160+
expect(instance.refs.x.tagName).toBe('A');
156161
instance._toggleActivatedState();
157-
expect(ReactDOM.findDOMNode(instance.refs.x).tagName).toBe('B');
162+
expect(instance.refs.x.tagName).toBe('B');
158163
instance._toggleActivatedState();
159-
expect(ReactDOM.findDOMNode(instance.refs.x).tagName).toBe('A');
164+
expect(instance.refs.x.tagName).toBe('A');
160165
});
161166

162167
it('should not cache old DOM nodes when switching constructors', () => {

‎packages/react-dom/src/__tests__/ReactCompositeComponentNestedState-test.js‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,11 @@
1111

1212
let React;
1313
let ReactDOM;
14-
let ReactTestUtils;
1514

1615
describe('ReactCompositeComponentNestedState-state', () => {
1716
beforeEach(() => {
1817
React = require('react');
1918
ReactDOM = require('react-dom');
20-
ReactTestUtils = require('react-dom/test-utils');
2119
});
2220

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

104102
// click "light green"
105-
ReactTestUtils.Simulate.click(container.childNodes[0].childNodes[3]);
103+
container.childNodes[0].childNodes[3].click();
106104

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

0 commit comments

Comments
 (0)