Replace `mountDepth` with `isTopLevel`
Summary: After #2570, `mountDepth` is only used to enforce that `setProps` and `replaceProps` is only invoked on the top-level component. This replaces `mountDepth` with a simpler `isTopLevel` boolean set by `ReactMount` which reduces the surface area of the internal API and removes the need to thread `mountDepth` throughout React core. Reviewers: @sebmarkbage @zpao Test Plan: Ran unit tests successfully: ``` npm run jest ```
yungsters committed
Nov 4, 2014 at 14:25 UTC
fc7cf2ff639099538fbfea1bbf1a74907348c9ed
12 files changed
+44
-109
src/browser/server/ReactServerRendering.js
+2
-2
@@ -40,7 +40,7 @@ function renderToString(element, context) {
40
41
return transaction.perform(function() {
42
var componentInstance = instantiateReactComponent(element, null);
43
- var markup = componentInstance.mountComponent(id, transaction, 0, context);
43
+ var markup = componentInstance.mountComponent(id, transaction, context);
44
return ReactMarkupChecksum.addChecksumToMarkup(markup);
45
}, null);
46
} finally {
@@ -68,7 +68,7 @@ function renderToStaticMarkup(element, context) {
68
69
return transaction.perform(function() {
70
var componentInstance = instantiateReactComponent(element, null);
71
- return componentInstance.mountComponent(id, transaction, 0, context);
71
+ return componentInstance.mountComponent(id, transaction, context);
72
}, null);
73
} finally {
74
ReactServerRenderingTransaction.release(transaction);
src/browser/ui/ReactDOMComponent.js
+1
-3
@@ -168,15 +168,13 @@ ReactDOMComponent.Mixin = {
168
* @internal
169
* @param {string} rootID The root DOM ID for this node.
170
* @param {ReactReconcileTransaction|ReactServerRenderingTransaction} transaction
171
- * @param {number} mountDepth number of components in the owner hierarchy
171
* @return {string} The computed markup.
172
*/
174
- mountComponent: function(rootID, transaction, mountDepth, context) {
173
+ mountComponent: function(rootID, transaction, context) {
174
ReactComponent.Mixin.mountComponent.call(
175
this,
176
rootID,
177
transaction,
179
- mountDepth,
178
context
179
);
180
this._rootNodeID = rootID;
src/browser/ui/ReactDOMTextComponent.js
+1
-2
@@ -62,11 +62,10 @@ assign(ReactDOMTextComponent.prototype, {
62
*
63
* @param {string} rootID DOM ID of the root node.
64
* @param {ReactReconcileTransaction|ReactServerRenderingTransaction} transaction
65
- * @param {number} mountDepth number of components in the owner hierarchy
65
* @return {string} Markup for this text node.
66
* @internal
67
*/
69
- mountComponent: function(rootID, transaction, mountDepth, context) {
68
+ mountComponent: function(rootID, transaction, context) {
69
this._rootNodeID = rootID;
70
var escapedText = escapeTextForBrowser(this._stringText);
71
src/browser/ui/ReactMount.js
+2
-1
@@ -225,7 +225,8 @@ function mountComponentIntoNode(
225
container,
226
transaction,
227
shouldReuseMarkup) {
228
- var markup = this.mountComponent(rootID, transaction, 0, emptyObject);
228
+ var markup = this.mountComponent(rootID, transaction, emptyObject);
229
+ this._isTopLevel = true;
230
ReactMount._mountImageIntoNode(markup, container, shouldReuseMarkup);
231
}
232
src/browser/ui/__tests__/ReactDOMComponent-test.js
+1
-1
@@ -328,7 +328,7 @@ describe('ReactDOMComponent', function() {
328
_owner: null,
329
_context: null
330
});
331
- return stubComponent.mountComponent('test', transaction, 0, {});
331
+ return stubComponent.mountComponent('test', transaction, {});
332
};
333
});
334
src/browser/ui/dom/__tests__/Danger-test.js
+3
-3
@@ -32,7 +32,7 @@ describe('Danger', function() {
32
it('should render markup', function() {
33
var markup = instantiateReactComponent(
34
<div />
35
- ).mountComponent('.rX', transaction, 0, {});
35
+ ).mountComponent('.rX', transaction, {});
36
var output = Danger.dangerouslyRenderMarkup([markup])[0];
37
38
expect(output.nodeName).toBe('DIV');
@@ -44,7 +44,7 @@ describe('Danger', function() {
44
).mountComponent(
45
'.rX',
46
transaction,
47
- 0, {}
47
+ {}
48
);
49
var output = Danger.dangerouslyRenderMarkup([markup])[0];
50
@@ -55,7 +55,7 @@ describe('Danger', function() {
55
it('should render wrapped markup', function() {
56
var markup = instantiateReactComponent(
57
<th />
58
- ).mountComponent('.rX', transaction, 0, {});
58
+ ).mountComponent('.rX', transaction, {});
59
var output = Danger.dangerouslyRenderMarkup([markup])[0];
60
61
expect(output.nodeName).toBe('TH');
src/core/ReactComponent.js
+1
-4
@@ -90,7 +90,6 @@ var ReactComponent = {
90
// to track updates.
91
this._currentElement = element;
92
this._mountIndex = 0;
93
- this._mountDepth = 0;
93
},
94
95
/**
@@ -103,17 +102,15 @@ var ReactComponent = {
102
*
103
* @param {string} rootID DOM ID of the root node.
104
* @param {ReactReconcileTransaction|ReactServerRenderingTransaction} transaction
106
- * @param {number} mountDepth number of components in the owner hierarchy.
105
* @return {?string} Rendered markup to be inserted into the DOM.
106
* @internal
107
*/
110
- mountComponent: function(rootID, transaction, mountDepth, context) {
108
+ mountComponent: function(rootID, transaction, context) {
109
var ref = this._currentElement.ref;
110
if (ref != null) {
111
var owner = this._currentElement._owner;
112
attachRef(ref, this, owner);
113
}
116
- this._mountDepth = mountDepth;
114
// Effectively: return '';
115
},
116
src/core/ReactCompositeComponent.js
+4
-9
@@ -141,6 +141,7 @@ var ReactCompositeComponentMixin = assign({},
141
142
this._context = null;
143
this._mountOrder = 0;
144
+ this._isTopLevel = false;
145
146
// See ReactUpdates.
147
this._pendingCallbacks = null;
@@ -161,17 +162,15 @@ var ReactCompositeComponentMixin = assign({},
162
*
163
* @param {string} rootID DOM ID of the root node.
164
* @param {ReactReconcileTransaction|ReactServerRenderingTransaction} transaction
164
- * @param {number} mountDepth number of components in the owner hierarchy
165
* @return {?string} Rendered markup to be inserted into the DOM.
166
* @final
167
* @internal
168
*/
169
- mountComponent: function(rootID, transaction, mountDepth, context) {
169
+ mountComponent: function(rootID, transaction, context) {
170
ReactComponent.Mixin.mountComponent.call(
171
this,
172
rootID,
173
transaction,
174
- mountDepth,
174
context
175
);
176
@@ -233,7 +232,6 @@ var ReactCompositeComponentMixin = assign({},
232
var markup = this._renderedComponent.mountComponent(
233
rootID,
234
transaction,
236
- mountDepth + 1,
235
this._processChildContext(context)
236
);
237
if (inst.componentDidMount) {
@@ -313,7 +311,7 @@ var ReactCompositeComponentMixin = assign({},
311
*/
312
replaceProps: function(props, callback) {
313
invariant(
316
- this._mountDepth === 0,
314
+ this._isTopLevel,
315
'replaceProps(...): You called `setProps` or `replaceProps` on a ' +
316
'component with a parent. This is an anti-pattern since props will ' +
317
'get reactively updated when rendered. Instead, change the owner\'s ' +
@@ -781,7 +779,6 @@ var ReactCompositeComponentMixin = assign({},
779
var nextMarkup = this._renderedComponent.mountComponent(
780
thisID,
781
transaction,
784
- this._mountDepth + 1,
782
context
783
);
784
ReactComponentEnvironment.replaceNodeWithMarkupByID(
@@ -890,17 +887,15 @@ var ShallowMixin = assign({},
887
*
888
* @param {string} rootID DOM ID of the root node.
889
* @param {ReactReconcileTransaction|ReactServerRenderingTransaction} transaction
893
- * @param {number} mountDepth number of components in the owner hierarchy
890
* @return {ReactElement} Shallow rendering of the component.
891
* @final
892
* @internal
893
*/
898
- mountComponent: function(rootID, transaction, mountDepth, context) {
894
+ mountComponent: function(rootID, transaction, context) {
895
ReactComponent.Mixin.mountComponent.call(
896
this,
897
rootID,
898
transaction,
903
- mountDepth,
899
context
900
);
901
src/core/ReactMultiChild.js
-2
@@ -195,7 +195,6 @@ var ReactMultiChild = {
195
var mountImage = childInstance.mountComponent(
196
rootID,
197
transaction,
198
- this._mountDepth + 1,
198
context
199
);
200
childInstance._mountIndex = index;
@@ -401,7 +400,6 @@ var ReactMultiChild = {
400
var mountImage = child.mountComponent(
401
rootID,
402
transaction,
404
- this._mountDepth + 1,
403
context
404
);
405
child._mountIndex = index;
src/core/__tests__/ReactComponent-test.js
-81
@@ -16,7 +16,6 @@ var ReactInstanceMap;
16
var ReactTestUtils;
17
18
var reactComponentExpect;
19
-var getMountDepth;
19
20
describe('ReactComponent', function() {
21
beforeEach(function() {
@@ -24,10 +23,6 @@ describe('ReactComponent', function() {
23
ReactInstanceMap = require('ReactInstanceMap');
24
ReactTestUtils = require('ReactTestUtils');
25
reactComponentExpect = require('reactComponentExpect');
27
-
28
- getMountDepth = function(instance) {
29
- return ReactInstanceMap.get(instance)._mountDepth;
30
- };
26
});
27
28
it('should throw on invalid render targets', function() {
@@ -231,80 +226,4 @@ describe('ReactComponent', function() {
226
var instance = ReactTestUtils.renderIntoDocument(element);
227
expect(instance.isMounted()).toBeTruthy();
228
});
234
-
235
- it('should know its simple mount depth', function() {
236
- var Owner = React.createClass({
237
- render: function() {
238
- return <Child ref="child" />;
239
- }
240
- });
241
-
242
- var Child = React.createClass({
243
- render: function() {
244
- return <div />;
245
- }
246
- });
247
-
248
- var instance = <Owner />;
249
- instance = ReactTestUtils.renderIntoDocument(instance);
250
- expect(getMountDepth(instance)).toBe(0);
251
- expect(getMountDepth(instance.refs.child)).toBe(1);
252
- });
253
-
254
- it('should know its (complicated) mount depth', function() {
255
- var Box = React.createClass({
256
- render: function() {
257
- return <div ref="boxDiv">{this.props.children}</div>;
258
- }
259
- });
260
-
261
- var Child = React.createClass({
262
- render: function() {
263
- return <span ref="span">child</span>;
264
- }
265
- });
266
-
267
- var Switcher = React.createClass({
268
- getInitialState: function() {
269
- return {tabKey: 'hello'};
270
- },
271
-
272
- render: function() {
273
- var child = this.props.children;
274
-
275
- return (
276
- <Box ref="box">
277
- <div
278
- ref="switcherDiv"
279
- style={{
280
- display: this.state.tabKey === child.key ? '' : 'none'
281
- }}>
282
- {child}
283
- </div>
284
- </Box>
285
- );
286
- }
287
- });
288
-
289
- var App = React.createClass({
290
- render: function() {
291
- return (
292
- <Switcher ref="switcher">
293
- <Child key="hello" ref="child" />
294
- </Switcher>
295
- );
296
- }
297
- });
298
-
299
- var root = <App />;
300
- root = ReactTestUtils.renderIntoDocument(root);
301
-
302
- expect(getMountDepth(root)).toBe(0);
303
- expect(getMountDepth(root.refs.switcher)).toBe(1);
304
- expect(getMountDepth(root.refs.switcher.refs.box)).toBe(2);
305
- expect(getMountDepth(root.refs.switcher.refs.switcherDiv)).toBe(5);
306
- expect(getMountDepth(root.refs.child)).toBe(7);
307
- expect(getMountDepth(root.refs.switcher.refs.box.refs.boxDiv)).toBe(3);
308
- expect(getMountDepth(root.refs.child.refs.span)).toBe(8);
309
- });
229
});
src/core/__tests__/ReactCompositeComponent-test.js
+28
@@ -529,6 +529,34 @@ describe('ReactCompositeComponent', function() {
529
);
530
});
531
532
+ it('should only allow `setProps` on top-level components', function() {
533
+ var container = document.createElement('div');
534
+ document.documentElement.appendChild(container);
535
+
536
+ var innerInstance;
537
+
538
+ var Component = React.createClass({
539
+ render: function() {
540
+ return <div><div ref="inner" /></div>;
541
+ },
542
+ componentDidMount: function() {
543
+ innerInstance = this.refs.inner;
544
+ }
545
+ });
546
+ React.render(<Component />, container);
547
+
548
+ expect(innerInstance).not.toBe(undefined);
549
+ expect(function() {
550
+ innerInstance.setProps({value: 1});
551
+ }).toThrow(
552
+ 'Invariant Violation: replaceProps(...): You called `setProps` or ' +
553
+ '`replaceProps` on a component with a parent. This is an anti-pattern ' +
554
+ 'since props will get reactively updated when rendered. Instead, ' +
555
+ 'change the owner\'s `render` method to pass the correct value as ' +
556
+ 'props to the component where it is created.'
557
+ );
558
+ });
559
+
560
it('should cleanup even if render() fatals', function() {
561
var BadComponent = React.createClass({
562
render: function() {
src/test/ReactTestUtils.js
+1
-1
@@ -340,7 +340,7 @@ ReactShallowRenderer.prototype._render = function(element, transaction, context)
340
var instance = new ShallowComponentWrapper(new element.type(element.props));
341
instance.construct(element);
342
343
- instance.mountComponent(rootID, transaction, 0, context);
343
+ instance.mountComponent(rootID, transaction, context);
344
345
this._instance = instance;
346
} else {