Skip to content

Commit aa0b741

Browse files
GordyDgaearon
authored andcommitted
Rewrite ReactDOMComponentTree-test to test behavior using Public API (#11383)
* Rewrite ReactDOMComponentTree-test to test behavior using Public API - Part of #11299 - I've tried to identify cases where code within ReactDOMComponentTree is exercised and have updated accordingly but I'm not entirely sure whether I'm on the right track. I thought I'd PR to get feedback from the community. Looking forward to comments. * Prettier and lint changes * Remove testing of internals and add test cases for testing behavior exhibited after use of getInstanceFromNode * [RFC] Update testing approach to verify exhibited behavior dependent upon methods in ReactDOMComponentTree * Remove tests from event handlers and use sync tests * Prettier changes * Rename variables to be more semantic * Prettier updates * Update test following review - Use beforeEach and afterEach to set up and tear down container element for use in each test - Move any functions specific to one test to within test body (improves readability imo) * Add coverage for getNodeFromInstance and implementation of getFiberCurrentPropsFromNode - After researching usage of getNodeFromInstance we can test getNodeFromInstance dispatching some events and asserting the id of the currentTarget - After checking git blame for getFiberCurrentPropsFromNode and reading through #8607 I found a test that we can simplify to assert behavior of the function by ensuring event handler props are updated from the fiber props. Swapping out the implementation of this function with `return node[internalInstanceKey].memoizedProps` results in a failure.
1 parent 01a867b commit aa0b741

File tree

1 file changed

+180
-77
lines changed

1 file changed

+180
-77
lines changed

packages/react-dom/src/__tests__/ReactDOMComponentTree-test.js

Lines changed: 180 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -10,114 +10,217 @@
1010
'use strict';
1111

1212
describe('ReactDOMComponentTree', () => {
13-
var React;
14-
var ReactDOM;
15-
var ReactDOMComponentTree;
16-
var ReactDOMServer;
17-
18-
function renderMarkupIntoDocument(elt) {
19-
var container = document.createElement('div');
20-
// Force server-rendering path:
21-
container.innerHTML = ReactDOMServer.renderToString(elt);
22-
return ReactDOM.hydrate(elt, container);
23-
}
24-
25-
function getTypeOf(instance) {
26-
return instance.type;
27-
}
28-
29-
function getTextOf(instance) {
30-
return instance.memoizedProps;
31-
}
13+
let React;
14+
let ReactDOM;
15+
let container;
3216

3317
beforeEach(() => {
3418
React = require('react');
3519
ReactDOM = require('react-dom');
36-
// TODO: can we express this test with only public API?
37-
ReactDOMComponentTree = require('../client/ReactDOMComponentTree');
38-
ReactDOMServer = require('react-dom/server');
20+
container = document.createElement('div');
21+
document.body.appendChild(container);
3922
});
4023

41-
it('finds nodes for instances', () => {
42-
// This is a little hard to test directly. But refs rely on it -- so we
43-
// check that we can find a ref at arbitrary points in the tree, even if
44-
// other nodes don't have a ref.
24+
afterEach(() => {
25+
document.body.removeChild(container);
26+
container = null;
27+
});
28+
29+
it('finds nodes for instances on events', () => {
30+
const mouseOverID = 'mouseOverID';
31+
const clickID = 'clickID';
32+
let currentTargetID = null;
33+
// the current target of an event is set to result of getNodeFromInstance
34+
// when an event is dispatched so we can test behavior by invoking
35+
// events on elements in the tree and confirming the expected node is
36+
// set as the current target
4537
class Component extends React.Component {
38+
handler = e => {
39+
currentTargetID = e.currentTarget.id;
40+
};
4641
render() {
47-
var toRef = this.props.toRef;
4842
return (
49-
<div ref={toRef === 'div' ? 'target' : null}>
50-
<h1 ref={toRef === 'h1' ? 'target' : null}>hello</h1>
51-
<p ref={toRef === 'p' ? 'target' : null}>
52-
<input ref={toRef === 'input' ? 'target' : null} />
53-
</p>
54-
goodbye.
43+
<div id={mouseOverID} onMouseOver={this.handler}>
44+
<div id={clickID} onClick={this.handler} />
5545
</div>
5646
);
5747
}
5848
}
5949

60-
function renderAndGetRef(toRef) {
61-
var inst = renderMarkupIntoDocument(<Component toRef={toRef} />);
62-
return inst.refs.target.nodeName;
50+
function simulateMouseEvent(elem, type) {
51+
const event = new MouseEvent(type, {
52+
bubbles: true,
53+
});
54+
elem.dispatchEvent(event);
6355
}
6456

65-
expect(renderAndGetRef('div')).toBe('DIV');
66-
expect(renderAndGetRef('h1')).toBe('H1');
67-
expect(renderAndGetRef('p')).toBe('P');
68-
expect(renderAndGetRef('input')).toBe('INPUT');
57+
const component = <Component />;
58+
ReactDOM.render(component, container);
59+
expect(currentTargetID).toBe(null);
60+
simulateMouseEvent(document.getElementById(mouseOverID), 'mouseover');
61+
expect(currentTargetID).toBe(mouseOverID);
62+
simulateMouseEvent(document.getElementById(clickID), 'click');
63+
expect(currentTargetID).toBe(clickID);
6964
});
7065

71-
it('finds instances for nodes', () => {
72-
class Component extends React.Component {
66+
it('finds closest instance for node when an event happens', () => {
67+
const nonReactElemID = 'aID';
68+
const innerHTML = {__html: `<div id="${nonReactElemID}"></div>`};
69+
const closestInstanceID = 'closestInstance';
70+
let currentTargetID = null;
71+
72+
class ClosestInstance extends React.Component {
73+
_onClick = e => {
74+
currentTargetID = e.currentTarget.id;
75+
};
7376
render() {
7477
return (
75-
<div>
76-
<h1>hello</h1>
77-
<p>
78-
<input />
79-
</p>
80-
goodbye.
81-
<main dangerouslySetInnerHTML={{__html: '<b><img></b>'}} />
82-
</div>
78+
<div
79+
id={closestInstanceID}
80+
onClick={this._onClick}
81+
dangerouslySetInnerHTML={innerHTML}
82+
/>
8383
);
8484
}
8585
}
8686

87-
function renderAndQuery(sel) {
88-
var root = renderMarkupIntoDocument(
89-
<section>
90-
<Component />
91-
</section>,
92-
);
93-
return sel ? root.querySelector(sel) : root;
87+
function simulateClick(elem) {
88+
const event = new MouseEvent('click', {
89+
bubbles: true,
90+
});
91+
elem.dispatchEvent(event);
92+
}
93+
94+
const component = <ClosestInstance />;
95+
ReactDOM.render(<section>{component}</section>, container);
96+
expect(currentTargetID).toBe(null);
97+
simulateClick(document.getElementById(nonReactElemID));
98+
expect(currentTargetID).toBe(closestInstanceID);
99+
});
100+
101+
it('updates event handlers from fiber props', () => {
102+
let action = '';
103+
let instance;
104+
const handlerA = () => (action = 'A');
105+
const handlerB = () => (action = 'B');
106+
107+
function simulateMouseOver(target) {
108+
const event = new MouseEvent('mouseover', {
109+
bubbles: true,
110+
});
111+
target.dispatchEvent(event);
112+
}
113+
114+
class HandlerFlipper extends React.Component {
115+
state = {flip: false};
116+
flip() {
117+
this.setState({flip: true});
118+
}
119+
render() {
120+
return (
121+
<div
122+
id="update"
123+
onMouseOver={this.state.flip ? handlerB : handlerA}
124+
/>
125+
);
126+
}
94127
}
95128

96-
function renderAndGetInstance(sel) {
97-
return ReactDOMComponentTree.getInstanceFromNode(renderAndQuery(sel));
129+
ReactDOM.render(
130+
<HandlerFlipper key="1" ref={n => (instance = n)} />,
131+
container,
132+
);
133+
const node = container.firstChild;
134+
simulateMouseOver(node);
135+
expect(action).toEqual('A');
136+
action = '';
137+
// Render with the other event handler.
138+
instance.flip();
139+
simulateMouseOver(node);
140+
expect(action).toEqual('B');
141+
});
142+
143+
it('finds a controlled instance from node and gets its current fiber props', () => {
144+
const inputID = 'inputID';
145+
const startValue = undefined;
146+
const finishValue = 'finish';
147+
148+
class Controlled extends React.Component {
149+
state = {value: startValue};
150+
a = null;
151+
_onChange = e => this.setState({value: e.currentTarget.value});
152+
render() {
153+
return (
154+
<input
155+
id={inputID}
156+
type="text"
157+
ref={n => (this.a = n)}
158+
value={this.state.value}
159+
onChange={this._onChange}
160+
/>
161+
);
162+
}
98163
}
99164

100-
function renderAndGetClosest(sel) {
101-
return ReactDOMComponentTree.getClosestInstanceFromNode(
102-
renderAndQuery(sel),
103-
);
165+
const setUntrackedInputValue = Object.getOwnPropertyDescriptor(
166+
HTMLInputElement.prototype,
167+
'value',
168+
).set;
169+
170+
function simulateInput(elem, value) {
171+
const inputEvent = new Event('input', {
172+
bubbles: true,
173+
});
174+
setUntrackedInputValue.call(elem, value);
175+
elem.dispatchEvent(inputEvent);
104176
}
105177

106-
expect(getTypeOf(renderAndGetInstance(null))).toBe('section');
107-
expect(getTypeOf(renderAndGetInstance('div'))).toBe('div');
108-
expect(getTypeOf(renderAndGetInstance('h1'))).toBe('h1');
109-
expect(getTypeOf(renderAndGetInstance('p'))).toBe('p');
110-
expect(getTypeOf(renderAndGetInstance('input'))).toBe('input');
111-
expect(getTypeOf(renderAndGetInstance('main'))).toBe('main');
112-
113-
// This one's a text component!
114-
var root = renderAndQuery(null);
115-
var inst = ReactDOMComponentTree.getInstanceFromNode(
116-
root.children[0].childNodes[2],
178+
const component = <Controlled />;
179+
const instance = ReactDOM.render(component, container);
180+
spyOn(console, 'error');
181+
expectDev(console.error.calls.count()).toBe(0);
182+
simulateInput(instance.a, finishValue);
183+
expectDev(console.error.calls.count()).toBe(1);
184+
expectDev(console.error.calls.argsFor(0)[0]).toContain(
185+
'Warning: A component is changing an uncontrolled input of ' +
186+
'type text to be controlled. Input elements should not ' +
187+
'switch from uncontrolled to controlled (or vice versa). ' +
188+
'Decide between using a controlled or uncontrolled input ' +
189+
'element for the lifetime of the component. More info: ' +
190+
'https://fb.me/react-controlled-components',
117191
);
118-
expect(getTextOf(inst)).toBe('goodbye.');
192+
});
119193

120-
expect(getTypeOf(renderAndGetClosest('b'))).toBe('main');
121-
expect(getTypeOf(renderAndGetClosest('img'))).toBe('main');
194+
it('finds instance of node that is attempted to be unmounted', () => {
195+
spyOn(console, 'error');
196+
const component = <div />;
197+
const node = ReactDOM.render(<div>{component}</div>, container);
198+
ReactDOM.unmountComponentAtNode(node);
199+
expectDev(console.error.calls.count()).toBe(1);
200+
expectDev(console.error.calls.argsFor(0)[0]).toContain(
201+
"unmountComponentAtNode(): The node you're attempting to unmount " +
202+
'was rendered by React and is not a top-level container. You may ' +
203+
'have accidentally passed in a React root node instead of its ' +
204+
'container.',
205+
);
206+
});
207+
208+
it('finds instance from node to stop rendering over other react rendered components', () => {
209+
spyOn(console, 'error');
210+
const component = (
211+
<div>
212+
<span>Hello</span>
213+
</div>
214+
);
215+
const anotherComponent = <div />;
216+
const instance = ReactDOM.render(component, container);
217+
ReactDOM.render(anotherComponent, instance);
218+
expectDev(console.error.calls.count()).toBe(1);
219+
expectDev(console.error.calls.argsFor(0)[0]).toContain(
220+
'render(...): Replacing React-rendered children with a new root ' +
221+
'component. If you intended to update the children of this node, ' +
222+
'you should instead have the existing children update their state ' +
223+
'and render the new components instead of calling ReactDOM.render.',
224+
);
122225
});
123226
});

0 commit comments

Comments
 (0)