Skip to content

Commit 0285c01

Browse files
committed
Fix MultiChild tests so they work with Fiber
Dropped the unnecessary use of findDOMNode. Dropped unnecessary arrow functions. Math.random() -> id counter, since this used to be non-deterministic which is not ideal for unit tests. getOriginalKeys() used to rely on implementation details to scan that the internal data structure maintained its structure, however, since that is unobservable we don't need to test the internal data structure itself. We're already testing refs and dom structure which is the only observable effect of the reorder.
1 parent 2450a17 commit 0285c01

File tree

2 files changed

+39
-63
lines changed

2 files changed

+39
-63
lines changed

src/renderers/shared/stack/reconciler/__tests__/ReactMultiChildReconcile-test.js

Lines changed: 33 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,10 @@
1313

1414
var React = require('React');
1515
var ReactDOM = require('ReactDOM');
16-
var ReactDOMComponentTree = require('ReactDOMComponentTree');
17-
var ReactInstanceMap = require('ReactInstanceMap');
1816

1917
var stripEmptyValues = function(obj) {
2018
var ret = {};
21-
var name;
22-
for (name in obj) {
19+
for (var name in obj) {
2320
if (!obj.hasOwnProperty(name)) {
2421
continue;
2522
}
@@ -30,31 +27,23 @@ var stripEmptyValues = function(obj) {
3027
return ret;
3128
};
3229

33-
/**
34-
* Child key names are wrapped like '.$key:0'. We strip the extra chars out
35-
* here. This relies on an implementation detail of the rendering system.
36-
*/
37-
var getOriginalKey = function(childName) {
38-
var match = childName.match(/^\.\$([^.]+)$/);
39-
expect(match).not.toBeNull();
40-
return match[1];
41-
};
30+
var idCounter = 123;
4231

4332
/**
4433
* Contains internal static internal state in order to test that updates to
4534
* existing children won't reinitialize components, when moving children -
4635
* reusing existing DOM/memory resources.
4736
*/
4837
class StatusDisplay extends React.Component {
49-
state = {internalState: Math.random()};
38+
state = {internalState: idCounter++};
5039

51-
getStatus = () => {
40+
getStatus() {
5241
return this.props.status;
53-
};
42+
}
5443

55-
getInternalState = () => {
44+
getInternalState() {
5645
return this.state.internalState;
57-
};
46+
}
5847

5948
componentDidMount() {
6049
this.props.onFlush();
@@ -67,7 +56,7 @@ class StatusDisplay extends React.Component {
6756
render() {
6857
return (
6958
<div>
70-
{this.state.internalState}
59+
{this.props.contentKey}
7160
</div>
7261
);
7362
}
@@ -82,39 +71,29 @@ class FriendsStatusDisplay extends React.Component {
8271
* Refs are not maintained in the rendered order, and neither is
8372
* `this._renderedChildren` (surprisingly).
8473
*/
85-
getOriginalKeys = () => {
74+
getOriginalKeys() {
8675
var originalKeys = [];
87-
// TODO: Update this to a better test that doesn't rely so much on internal
88-
// implementation details.
89-
var statusDisplays =
90-
ReactInstanceMap.get(this)
91-
._renderedComponent
92-
._renderedChildren;
93-
var name;
94-
for (name in statusDisplays) {
95-
var child = statusDisplays[name];
96-
var isPresent = !!child;
97-
if (isPresent) {
98-
originalKeys[child._mountIndex] = getOriginalKey(name);
76+
for (var key in this.props.usernameToStatus) {
77+
if (this.props.usernameToStatus[key]) {
78+
originalKeys.push(key);
9979
}
10080
}
10181
return originalKeys;
102-
};
82+
}
10383

10484
/**
10585
* Retrieves the rendered children in a nice format for comparing to the input
10686
* `this.props.usernameToStatus`.
10787
*/
108-
getStatusDisplays = () => {
88+
getStatusDisplays() {
10989
var res = {};
110-
var i;
11190
var originalKeys = this.getOriginalKeys();
112-
for (i = 0; i < originalKeys.length; i++) {
91+
for (var i = 0; i < originalKeys.length; i++) {
11392
var key = originalKeys[i];
11493
res[key] = this.refs[key];
11594
}
11695
return res;
117-
};
96+
}
11897

11998
/**
12099
* Verifies that by the time a child is flushed, the refs that appeared
@@ -123,29 +102,28 @@ class FriendsStatusDisplay extends React.Component {
123102
* but our internal layer API depends on this assumption. We need to change
124103
* it to be more declarative before making ref resolution indeterministic.
125104
*/
126-
verifyPreviousRefsResolved = (flushedKey) => {
127-
var i;
105+
verifyPreviousRefsResolved(flushedKey) {
128106
var originalKeys = this.getOriginalKeys();
129-
for (i = 0; i < originalKeys.length; i++) {
107+
for (var i = 0; i < originalKeys.length; i++) {
130108
var key = originalKeys[i];
131109
if (key === flushedKey) {
132110
// We are only interested in children up to the current key.
133111
return;
134112
}
135113
expect(this.refs[key]).toBeTruthy();
136114
}
137-
};
115+
}
138116

139117
render() {
140118
var children = [];
141-
var key;
142-
for (key in this.props.usernameToStatus) {
119+
for (var key in this.props.usernameToStatus) {
143120
var status = this.props.usernameToStatus[key];
144121
children.push(
145122
!status ? null :
146123
<StatusDisplay
147124
key={key}
148125
ref={key}
126+
contentKey={key}
149127
onFlush={this.verifyPreviousRefsResolved.bind(this, key)}
150128
status={status}
151129
/>
@@ -223,35 +201,33 @@ function verifyStatesPreserved(lastInternalStates, statusDisplays) {
223201
* Verifies that the internal representation of a set of `renderedChildren`
224202
* accurately reflects what is in the DOM.
225203
*/
226-
function verifyDomOrderingAccurate(parentInstance, statusDisplays) {
227-
var containerNode = ReactDOM.findDOMNode(parentInstance);
204+
function verifyDomOrderingAccurate(outerContainer, statusDisplays) {
205+
var containerNode = outerContainer.firstChild;
228206
var statusDisplayNodes = containerNode.childNodes;
229-
var i;
230-
var orderedDomIDs = [];
231-
for (i = 0; i < statusDisplayNodes.length; i++) {
232-
var inst = ReactDOMComponentTree.getInstanceFromNode(statusDisplayNodes[i]);
233-
orderedDomIDs.push(inst._rootNodeID);
207+
var orderedDomKeys = [];
208+
for (var i = 0; i < statusDisplayNodes.length; i++) {
209+
var contentKey = statusDisplayNodes[i].textContent;
210+
orderedDomKeys.push(contentKey);
234211
}
235212

236-
var orderedLogicalIDs = [];
213+
var orderedLogicalKeys = [];
237214
var username;
238215
for (username in statusDisplays) {
239216
if (!statusDisplays.hasOwnProperty(username)) {
240217
continue;
241218
}
242219
var statusDisplay = statusDisplays[username];
243-
orderedLogicalIDs.push(
244-
ReactInstanceMap.get(statusDisplay)._renderedComponent._rootNodeID
220+
orderedLogicalKeys.push(
221+
statusDisplay.props.contentKey
245222
);
246223
}
247-
expect(orderedDomIDs).toEqual(orderedLogicalIDs);
224+
expect(orderedDomKeys).toEqual(orderedLogicalKeys);
248225
}
249226

250227
/**
251228
* Todo: Check that internal state is preserved across transitions
252229
*/
253230
function testPropsSequence(sequence) {
254-
var i;
255231
var container = document.createElement('div');
256232
var parentInstance = ReactDOM.render(
257233
<FriendsStatusDisplay {...sequence[0]} />,
@@ -261,15 +237,15 @@ function testPropsSequence(sequence) {
261237
var lastInternalStates = getInternalStateByUserName(statusDisplays);
262238
verifyStatuses(statusDisplays, sequence[0]);
263239

264-
for (i = 1; i < sequence.length; i++) {
240+
for (var i = 1; i < sequence.length; i++) {
265241
ReactDOM.render(
266242
<FriendsStatusDisplay {...sequence[i]} />,
267243
container
268244
);
269245
statusDisplays = parentInstance.getStatusDisplays();
270246
verifyStatuses(statusDisplays, sequence[i]);
271247
verifyStatesPreserved(lastInternalStates, statusDisplays);
272-
verifyDomOrderingAccurate(parentInstance, statusDisplays);
248+
verifyDomOrderingAccurate(container, statusDisplays);
273249

274250
lastInternalStates = getInternalStateByUserName(statusDisplays);
275251
}

src/renderers/shared/stack/reconciler/__tests__/ReactMultiChildText-test.js

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,17 +26,17 @@ var testAllPermutations = function(testCases) {
2626
var expectedResultAfterUpdate = testCases[j + 1];
2727

2828
var container = document.createElement('div');
29-
var d = ReactDOM.render(<div>{renderWithChildren}</div>, container);
30-
expectChildren(d, expectedResultAfterRender);
29+
ReactDOM.render(<div>{renderWithChildren}</div>, container);
30+
expectChildren(container, expectedResultAfterRender);
3131

32-
d = ReactDOM.render(<div>{updateWithChildren}</div>, container);
33-
expectChildren(d, expectedResultAfterUpdate);
32+
ReactDOM.render(<div>{updateWithChildren}</div>, container);
33+
expectChildren(container, expectedResultAfterUpdate);
3434
}
3535
}
3636
};
3737

38-
var expectChildren = function(d, children) {
39-
var outerNode = ReactDOM.findDOMNode(d);
38+
var expectChildren = function(container, children) {
39+
var outerNode = container.firstChild;
4040
var textNode;
4141
if (typeof children === 'string') {
4242
textNode = outerNode.firstChild;

0 commit comments

Comments
 (0)