Skip to content

Commit

Permalink
Merge pull request #2571 from yungsters/cleanup
Browse files Browse the repository at this point in the history
Replace `mountDepth` with `isTopLevel`
  • Loading branch information
yungsters committed Nov 19, 2014
2 parents 70b1426 + fc7cf2f commit 006bc28
Show file tree
Hide file tree
Showing 12 changed files with 44 additions and 109 deletions.
4 changes: 2 additions & 2 deletions src/browser/server/ReactServerRendering.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ function renderToString(element, context) {

return transaction.perform(function() {
var componentInstance = instantiateReactComponent(element, null);
var markup = componentInstance.mountComponent(id, transaction, 0, context);
var markup = componentInstance.mountComponent(id, transaction, context);
return ReactMarkupChecksum.addChecksumToMarkup(markup);
}, null);
} finally {
Expand Down Expand Up @@ -68,7 +68,7 @@ function renderToStaticMarkup(element, context) {

return transaction.perform(function() {
var componentInstance = instantiateReactComponent(element, null);
return componentInstance.mountComponent(id, transaction, 0, context);
return componentInstance.mountComponent(id, transaction, context);
}, null);
} finally {
ReactServerRenderingTransaction.release(transaction);
Expand Down
4 changes: 1 addition & 3 deletions src/browser/ui/ReactDOMComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -168,15 +168,13 @@ ReactDOMComponent.Mixin = {
* @internal
* @param {string} rootID The root DOM ID for this node.
* @param {ReactReconcileTransaction|ReactServerRenderingTransaction} transaction
* @param {number} mountDepth number of components in the owner hierarchy
* @return {string} The computed markup.
*/
mountComponent: function(rootID, transaction, mountDepth, context) {
mountComponent: function(rootID, transaction, context) {
ReactComponent.Mixin.mountComponent.call(
this,
rootID,
transaction,
mountDepth,
context
);
this._rootNodeID = rootID;
Expand Down
3 changes: 1 addition & 2 deletions src/browser/ui/ReactDOMTextComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,10 @@ assign(ReactDOMTextComponent.prototype, {
*
* @param {string} rootID DOM ID of the root node.
* @param {ReactReconcileTransaction|ReactServerRenderingTransaction} transaction
* @param {number} mountDepth number of components in the owner hierarchy
* @return {string} Markup for this text node.
* @internal
*/
mountComponent: function(rootID, transaction, mountDepth, context) {
mountComponent: function(rootID, transaction, context) {
this._rootNodeID = rootID;
var escapedText = escapeTextForBrowser(this._stringText);

Expand Down
3 changes: 2 additions & 1 deletion src/browser/ui/ReactMount.js
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,8 @@ function mountComponentIntoNode(
container,
transaction,
shouldReuseMarkup) {
var markup = this.mountComponent(rootID, transaction, 0, emptyObject);
var markup = this.mountComponent(rootID, transaction, emptyObject);
this._isTopLevel = true;
ReactMount._mountImageIntoNode(markup, container, shouldReuseMarkup);
}

Expand Down
2 changes: 1 addition & 1 deletion src/browser/ui/__tests__/ReactDOMComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ describe('ReactDOMComponent', function() {
_owner: null,
_context: null
});
return stubComponent.mountComponent('test', transaction, 0, {});
return stubComponent.mountComponent('test', transaction, {});
};
});

Expand Down
6 changes: 3 additions & 3 deletions src/browser/ui/dom/__tests__/Danger-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ describe('Danger', function() {
it('should render markup', function() {
var markup = instantiateReactComponent(
<div />
).mountComponent('.rX', transaction, 0, {});
).mountComponent('.rX', transaction, {});
var output = Danger.dangerouslyRenderMarkup([markup])[0];

expect(output.nodeName).toBe('DIV');
Expand All @@ -44,7 +44,7 @@ describe('Danger', function() {
).mountComponent(
'.rX',
transaction,
0, {}
{}
);
var output = Danger.dangerouslyRenderMarkup([markup])[0];

Expand All @@ -55,7 +55,7 @@ describe('Danger', function() {
it('should render wrapped markup', function() {
var markup = instantiateReactComponent(
<th />
).mountComponent('.rX', transaction, 0, {});
).mountComponent('.rX', transaction, {});
var output = Danger.dangerouslyRenderMarkup([markup])[0];

expect(output.nodeName).toBe('TH');
Expand Down
5 changes: 1 addition & 4 deletions src/core/ReactComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ var ReactComponent = {
// to track updates.
this._currentElement = element;
this._mountIndex = 0;
this._mountDepth = 0;
},

/**
Expand All @@ -103,17 +102,15 @@ var ReactComponent = {
*
* @param {string} rootID DOM ID of the root node.
* @param {ReactReconcileTransaction|ReactServerRenderingTransaction} transaction
* @param {number} mountDepth number of components in the owner hierarchy.
* @return {?string} Rendered markup to be inserted into the DOM.
* @internal
*/
mountComponent: function(rootID, transaction, mountDepth, context) {
mountComponent: function(rootID, transaction, context) {
var ref = this._currentElement.ref;
if (ref != null) {
var owner = this._currentElement._owner;
attachRef(ref, this, owner);
}
this._mountDepth = mountDepth;
// Effectively: return '';
},

Expand Down
13 changes: 4 additions & 9 deletions src/core/ReactCompositeComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ var ReactCompositeComponentMixin = assign({},

this._context = null;
this._mountOrder = 0;
this._isTopLevel = false;

// See ReactUpdates.
this._pendingCallbacks = null;
Expand All @@ -161,17 +162,15 @@ var ReactCompositeComponentMixin = assign({},
*
* @param {string} rootID DOM ID of the root node.
* @param {ReactReconcileTransaction|ReactServerRenderingTransaction} transaction
* @param {number} mountDepth number of components in the owner hierarchy
* @return {?string} Rendered markup to be inserted into the DOM.
* @final
* @internal
*/
mountComponent: function(rootID, transaction, mountDepth, context) {
mountComponent: function(rootID, transaction, context) {
ReactComponent.Mixin.mountComponent.call(
this,
rootID,
transaction,
mountDepth,
context
);

Expand Down Expand Up @@ -233,7 +232,6 @@ var ReactCompositeComponentMixin = assign({},
var markup = this._renderedComponent.mountComponent(
rootID,
transaction,
mountDepth + 1,
this._processChildContext(context)
);
if (inst.componentDidMount) {
Expand Down Expand Up @@ -313,7 +311,7 @@ var ReactCompositeComponentMixin = assign({},
*/
replaceProps: function(props, callback) {
invariant(
this._mountDepth === 0,
this._isTopLevel,
'replaceProps(...): You called `setProps` or `replaceProps` on a ' +
'component with a parent. This is an anti-pattern since props will ' +
'get reactively updated when rendered. Instead, change the owner\'s ' +
Expand Down Expand Up @@ -781,7 +779,6 @@ var ReactCompositeComponentMixin = assign({},
var nextMarkup = this._renderedComponent.mountComponent(
thisID,
transaction,
this._mountDepth + 1,
context
);
ReactComponentEnvironment.replaceNodeWithMarkupByID(
Expand Down Expand Up @@ -890,17 +887,15 @@ var ShallowMixin = assign({},
*
* @param {string} rootID DOM ID of the root node.
* @param {ReactReconcileTransaction|ReactServerRenderingTransaction} transaction
* @param {number} mountDepth number of components in the owner hierarchy
* @return {ReactElement} Shallow rendering of the component.
* @final
* @internal
*/
mountComponent: function(rootID, transaction, mountDepth, context) {
mountComponent: function(rootID, transaction, context) {
ReactComponent.Mixin.mountComponent.call(
this,
rootID,
transaction,
mountDepth,
context
);

Expand Down
2 changes: 0 additions & 2 deletions src/core/ReactMultiChild.js
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,6 @@ var ReactMultiChild = {
var mountImage = childInstance.mountComponent(
rootID,
transaction,
this._mountDepth + 1,
context
);
childInstance._mountIndex = index;
Expand Down Expand Up @@ -401,7 +400,6 @@ var ReactMultiChild = {
var mountImage = child.mountComponent(
rootID,
transaction,
this._mountDepth + 1,
context
);
child._mountIndex = index;
Expand Down
81 changes: 0 additions & 81 deletions src/core/__tests__/ReactComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,13 @@ var ReactInstanceMap;
var ReactTestUtils;

var reactComponentExpect;
var getMountDepth;

describe('ReactComponent', function() {
beforeEach(function() {
React = require('React');
ReactInstanceMap = require('ReactInstanceMap');
ReactTestUtils = require('ReactTestUtils');
reactComponentExpect = require('reactComponentExpect');

getMountDepth = function(instance) {
return ReactInstanceMap.get(instance)._mountDepth;
};
});

it('should throw on invalid render targets', function() {
Expand Down Expand Up @@ -231,80 +226,4 @@ describe('ReactComponent', function() {
var instance = ReactTestUtils.renderIntoDocument(element);
expect(instance.isMounted()).toBeTruthy();
});

it('should know its simple mount depth', function() {
var Owner = React.createClass({
render: function() {
return <Child ref="child" />;
}
});

var Child = React.createClass({
render: function() {
return <div />;
}
});

var instance = <Owner />;
instance = ReactTestUtils.renderIntoDocument(instance);
expect(getMountDepth(instance)).toBe(0);
expect(getMountDepth(instance.refs.child)).toBe(1);
});

it('should know its (complicated) mount depth', function() {
var Box = React.createClass({
render: function() {
return <div ref="boxDiv">{this.props.children}</div>;
}
});

var Child = React.createClass({
render: function() {
return <span ref="span">child</span>;
}
});

var Switcher = React.createClass({
getInitialState: function() {
return {tabKey: 'hello'};
},

render: function() {
var child = this.props.children;

return (
<Box ref="box">
<div
ref="switcherDiv"
style={{
display: this.state.tabKey === child.key ? '' : 'none'
}}>
{child}
</div>
</Box>
);
}
});

var App = React.createClass({
render: function() {
return (
<Switcher ref="switcher">
<Child key="hello" ref="child" />
</Switcher>
);
}
});

var root = <App />;
root = ReactTestUtils.renderIntoDocument(root);

expect(getMountDepth(root)).toBe(0);
expect(getMountDepth(root.refs.switcher)).toBe(1);
expect(getMountDepth(root.refs.switcher.refs.box)).toBe(2);
expect(getMountDepth(root.refs.switcher.refs.switcherDiv)).toBe(5);
expect(getMountDepth(root.refs.child)).toBe(7);
expect(getMountDepth(root.refs.switcher.refs.box.refs.boxDiv)).toBe(3);
expect(getMountDepth(root.refs.child.refs.span)).toBe(8);
});
});
28 changes: 28 additions & 0 deletions src/core/__tests__/ReactCompositeComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,34 @@ describe('ReactCompositeComponent', function() {
);
});

it('should only allow `setProps` on top-level components', function() {
var container = document.createElement('div');
document.documentElement.appendChild(container);

var innerInstance;

var Component = React.createClass({
render: function() {
return <div><div ref="inner" /></div>;
},
componentDidMount: function() {
innerInstance = this.refs.inner;
}
});
React.render(<Component />, container);

expect(innerInstance).not.toBe(undefined);
expect(function() {
innerInstance.setProps({value: 1});
}).toThrow(
'Invariant Violation: replaceProps(...): You called `setProps` or ' +
'`replaceProps` on a component with a parent. This is an anti-pattern ' +
'since props will get reactively updated when rendered. Instead, ' +
'change the owner\'s `render` method to pass the correct value as ' +
'props to the component where it is created.'
);
});

it('should cleanup even if render() fatals', function() {
var BadComponent = React.createClass({
render: function() {
Expand Down
2 changes: 1 addition & 1 deletion src/test/ReactTestUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ ReactShallowRenderer.prototype._render = function(element, transaction, context)
var instance = new ShallowComponentWrapper(new element.type(element.props));
instance.construct(element);

instance.mountComponent(rootID, transaction, 0, context);
instance.mountComponent(rootID, transaction, context);

this._instance = instance;
} else {
Expand Down

0 comments on commit 006bc28

Please sign in to comment.