Skip to content

Replace mountDepth with isTopLevel #2571

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 19, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it safe here to assume that this is going to be a ReactCompositeComponent?

Edit: Otherwise, preventExtension will complain. 👎

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, it's because the callback is used with a "context". Annoying OO pattern.

Edit: This will probably change but it's currently true.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, thanks!

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