diff --git a/src/core/ReactComponent.js b/src/core/ReactComponent.js index 2f048502e6..833d5ba5ac 100644 --- a/src/core/ReactComponent.js +++ b/src/core/ReactComponent.js @@ -335,10 +335,11 @@ var ReactComponent = { * * @param {string} rootID DOM ID of the root node. * @param {ReactReconcileTransaction} 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) { + mountComponent: function(rootID, transaction, mountDepth) { invariant( !this.isMounted(), 'mountComponent(%s, ...): Can only mount an unmounted component.', @@ -350,6 +351,7 @@ var ReactComponent = { } this._rootNodeID = rootID; this._lifeCycleState = ComponentLifeCycle.MOUNTED; + this._mountDepth = mountDepth; // Effectively: return ''; }, @@ -485,7 +487,7 @@ var ReactComponent = { container, transaction, shouldReuseMarkup) { - var markup = this.mountComponent(rootID, transaction); + var markup = this.mountComponent(rootID, transaction, 0); ReactComponent.mountImageIntoNode(markup, container, shouldReuseMarkup); }, diff --git a/src/core/ReactCompositeComponent.js b/src/core/ReactCompositeComponent.js index 2d4b09efd3..50405a7ddb 100644 --- a/src/core/ReactCompositeComponent.js +++ b/src/core/ReactCompositeComponent.js @@ -531,6 +531,7 @@ var ReactCompositeComponentMixin = { * * @param {string} rootID DOM ID of the root node. * @param {ReactReconcileTransaction} transaction + * @param {number} mountDepth number of components in the owner hierarchy * @return {?string} Rendered markup to be inserted into the DOM. * @final * @internal @@ -538,8 +539,13 @@ var ReactCompositeComponentMixin = { mountComponent: ReactPerf.measure( 'ReactCompositeComponent', 'mountComponent', - function(rootID, transaction) { - ReactComponent.Mixin.mountComponent.call(this, rootID, transaction); + function(rootID, transaction, mountDepth) { + ReactComponent.Mixin.mountComponent.call( + this, + rootID, + transaction, + mountDepth + ); this._compositeLifeCycleState = CompositeLifeCycle.MOUNTING; this._defaultProps = this.getDefaultProps ? this.getDefaultProps() : null; @@ -567,7 +573,11 @@ var ReactCompositeComponentMixin = { // Done with mounting, `setState` will now trigger UI changes. this._compositeLifeCycleState = null; - var markup = this._renderedComponent.mountComponent(rootID, transaction); + var markup = this._renderedComponent.mountComponent( + rootID, + transaction, + mountDepth + 1 + ); if (this.componentDidMount) { transaction.getReactOnDOMReady().enqueue(this, this.componentDidMount); } @@ -788,7 +798,11 @@ var ReactCompositeComponentMixin = { var thisID = this._rootNodeID; var currentComponentID = currentComponent._rootNodeID; currentComponent.unmountComponent(); - var nextMarkup = nextComponent.mountComponent(thisID, transaction); + var nextMarkup = nextComponent.mountComponent( + thisID, + transaction, + this._mountDepth + 1 + ); ReactComponent.DOMIDOperations.dangerouslyReplaceNodeWithMarkupByID( currentComponentID, nextMarkup diff --git a/src/core/ReactMultiChild.js b/src/core/ReactMultiChild.js index 79c493423d..affb0d67c8 100644 --- a/src/core/ReactMultiChild.js +++ b/src/core/ReactMultiChild.js @@ -202,7 +202,11 @@ var ReactMultiChild = { if (children.hasOwnProperty(name) && child) { // Inlined for performance, see `ReactInstanceHandles.createReactID`. var rootID = this._rootNodeID + '.' + name; - var mountImage = child.mountComponent(rootID, transaction); + var mountImage = child.mountComponent( + rootID, + transaction, + this._mountDepth + 1 + ); child._mountImage = mountImage; child._mountIndex = index; mountImages.push(mountImage); @@ -395,7 +399,11 @@ var ReactMultiChild = { _mountChildByNameAtIndex: function(child, name, index, transaction) { // Inlined for performance, see `ReactInstanceHandles.createReactID`. var rootID = this._rootNodeID + '.' + name; - var mountImage = child.mountComponent(rootID, transaction); + var mountImage = child.mountComponent( + rootID, + transaction, + this._mountDepth + 1 + ); child._mountImage = mountImage; child._mountIndex = index; this.createChild(child); diff --git a/src/core/ReactNativeComponent.js b/src/core/ReactNativeComponent.js index 2ccc69b44f..4fc4d67da8 100644 --- a/src/core/ReactNativeComponent.js +++ b/src/core/ReactNativeComponent.js @@ -84,13 +84,19 @@ ReactNativeComponent.Mixin = { * @internal * @param {string} rootID The root DOM ID for this node. * @param {ReactReconcileTransaction} transaction + * @param {number} mountDepth number of components in the owner hierarchy * @return {string} The computed markup. */ mountComponent: ReactPerf.measure( 'ReactNativeComponent', 'mountComponent', - function(rootID, transaction) { - ReactComponent.Mixin.mountComponent.call(this, rootID, transaction); + function(rootID, transaction, mountDepth) { + ReactComponent.Mixin.mountComponent.call( + this, + rootID, + transaction, + mountDepth + ); assertValidProps(this.props); return ( this._createOpenTagMarkup() + diff --git a/src/core/ReactTextComponent.js b/src/core/ReactTextComponent.js index 24c07bd704..235f0dad41 100644 --- a/src/core/ReactTextComponent.js +++ b/src/core/ReactTextComponent.js @@ -52,11 +52,18 @@ mixInto(ReactTextComponent, { * any features besides containing text content. * * @param {string} rootID DOM ID of the root node. + * @param {ReactReconcileTransaction} transaction + * @param {number} mountDepth number of components in the owner hierarchy * @return {string} Markup for this text node. * @internal */ - mountComponent: function(rootID) { - ReactComponent.Mixin.mountComponent.call(this, rootID); + mountComponent: function(rootID, transaction, mountDepth) { + ReactComponent.Mixin.mountComponent.call( + this, + rootID, + transaction, + mountDepth + ); return ( '' + escapeTextForBrowser(this.props.text) + diff --git a/src/core/ReactUpdates.js b/src/core/ReactUpdates.js index bcb1f68c78..d866eec089 100644 --- a/src/core/ReactUpdates.js +++ b/src/core/ReactUpdates.js @@ -33,8 +33,24 @@ function batchedUpdates(callback, param) { batchingStrategy.batchedUpdates(callback, param); } +/** + * Array comparator for ReactComponents by owner depth + * + * @param {ReactComponent} c1 first component you're comparing + * @param {ReactComponent} c2 second component you're comparing + * @return {number} Return value usable by Array.prototype.sort(). + */ +function mountDepthComparator(c1, c2) { + return c1._mountDepth - c2._mountDepth; +} + function runBatchedUpdates() { - // TODO: Sort components by depth such that parent components update first + // Since reconciling a component higher in the owner hierarchy usually (not + // always -- see shouldComponentUpdate()) will reconcile children, reconcile + // them before their children by sorting the array. + + dirtyComponents.sort(mountDepthComparator); + for (var i = 0; i < dirtyComponents.length; i++) { // If a component is unmounted before pending changes apply, ignore them // TODO: Queue unmounts in the same list to avoid this happening at all diff --git a/src/core/__tests__/ReactComponent-test.js b/src/core/__tests__/ReactComponent-test.js index 9c47a1e0f1..95e0882bd5 100644 --- a/src/core/__tests__/ReactComponent-test.js +++ b/src/core/__tests__/ReactComponent-test.js @@ -133,4 +133,79 @@ describe('ReactComponent', function() { expect(instance.isMounted()).toBeTruthy(); }); + it('should know its simple mount depth', function() { + var Owner = React.createClass({ + render: function() { + return ; + } + }); + + var Child = React.createClass({ + render: function() { + return
; + } + }); + + var instance = ; + ReactTestUtils.renderIntoDocument(instance); + expect(instance._mountDepth).toBe(0); + expect(instance.refs.child._mountDepth).toBe(1); + }); + + it('should know its (complicated) mount depth', function() { + var Box = React.createClass({ + render: function() { + return
{this.props.children}
; + } + }); + + var Child = React.createClass({ + render: function() { + return child; + } + }); + + var Switcher = React.createClass({ + getInitialState: function() { + return {tabKey: 'hello'}; + }, + + render: function() { + var child = this.props.children; + + return ( + +
+ {child} +
+ + ); + } + }); + + var App = React.createClass({ + render: function() { + return ( + + + + ); + } + }); + + var root = ; + ReactTestUtils.renderIntoDocument(root); + + expect(root._mountDepth).toBe(0); + expect(root.refs.switcher._mountDepth).toBe(1); + expect(root.refs.switcher.refs.box._mountDepth).toBe(2); + expect(root.refs.switcher.refs.switcherDiv._mountDepth).toBe(4); + expect(root.refs.child._mountDepth).toBe(5); + expect(root.refs.switcher.refs.box.refs.boxDiv._mountDepth).toBe(3); + expect(root.refs.child.refs.span._mountDepth).toBe(6); + }); }); diff --git a/src/core/__tests__/ReactNativeComponent-test.js b/src/core/__tests__/ReactNativeComponent-test.js index 29a9498a86..f7c7dae3d1 100644 --- a/src/core/__tests__/ReactNativeComponent-test.js +++ b/src/core/__tests__/ReactNativeComponent-test.js @@ -290,7 +290,7 @@ describe('ReactNativeComponent', function() { mountComponent = function(props) { var transaction = new ReactReconcileTransaction(); var stubComponent = new StubNativeComponent(props); - return stubComponent.mountComponent('test', transaction); + return stubComponent.mountComponent('test', transaction, 0); }; }); diff --git a/src/core/__tests__/ReactUpdates-test.js b/src/core/__tests__/ReactUpdates-test.js index d73bad3739..ee0267b5c8 100644 --- a/src/core/__tests__/ReactUpdates-test.js +++ b/src/core/__tests__/ReactUpdates-test.js @@ -210,10 +210,8 @@ describe('ReactUpdates', function() { expect(child.state.y).toBe(2); expect(parentUpdateCount).toBe(1); - // When we update the child first, we currently incur two updates because - // we aren't smart about what order to process the components in. - // TODO: Reduce the update count here to 1 - expect(childUpdateCount).toBe(2); + // Batching reduces the number of updates here to 1. + expect(childUpdateCount).toBe(1); }); it('should support chained state updates', function() { @@ -293,4 +291,169 @@ describe('ReactUpdates', function() { expect(instance.state.x).toBe(1); expect(updateCount).toBe(1); }); + + it('should update children even if parent blocks updates', function() { + var parentRenderCount = 0; + var childRenderCount = 0; + + var Parent = React.createClass({ + shouldComponentUpdate: function() { + return false; + }, + + render: function() { + parentRenderCount++; + return ; + } + }); + + var Child = React.createClass({ + render: function() { + childRenderCount++; + return
; + } + }); + + expect(parentRenderCount).toBe(0); + expect(childRenderCount).toBe(0); + + var instance = ; + ReactTestUtils.renderIntoDocument(instance); + + expect(parentRenderCount).toBe(1); + expect(childRenderCount).toBe(1); + + ReactUpdates.batchedUpdates(function() { + instance.setState({x: 1}); + }); + + expect(parentRenderCount).toBe(1); + expect(childRenderCount).toBe(1); + + ReactUpdates.batchedUpdates(function() { + instance.refs.child.setState({x: 1}); + }); + + expect(parentRenderCount).toBe(1); + expect(childRenderCount).toBe(2); + }); + + it('should flow updates correctly', function() { + var willUpdates = []; + var didUpdates = []; + + var UpdateLoggingMixin = { + componentWillUpdate: function() { + willUpdates.push(this.constructor.displayName); + }, + componentDidUpdate: function() { + didUpdates.push(this.constructor.displayName); + } + }; + + var Box = React.createClass({ + mixins: [UpdateLoggingMixin], + + render: function() { + return
{this.props.children}
; + } + }); + + var Child = React.createClass({ + mixins: [UpdateLoggingMixin], + + render: function() { + return child; + } + }); + + var Switcher = React.createClass({ + mixins: [UpdateLoggingMixin], + + getInitialState: function() { + return {tabKey: 'hello'}; + }, + + render: function() { + var child = this.props.children; + + return ( + +
+ {child} +
+ + ); + } + }); + + var App = React.createClass({ + mixins: [UpdateLoggingMixin], + + render: function() { + return ( + + + + ); + } + }); + + var root = ; + ReactTestUtils.renderIntoDocument(root); + + function expectUpdates(sequence) { + // didUpdate() occurs in reverse order + didUpdates.reverse(); + expect(willUpdates).toEqual(didUpdates); + expect(willUpdates).toEqual(sequence); + willUpdates.length = 0; + didUpdates.length = 0; + } + + function triggerUpdate(c) { + c.setState({x: 1}); + } + + function testUpdates(components, expectation) { + var i; + + ReactUpdates.batchedUpdates(function() { + for (i = 0; i < components.length; i++) { + triggerUpdate(components[i]); + } + }); + + expectUpdates(expectation); + + // Try them in reverse order + + ReactUpdates.batchedUpdates(function() { + for (i = components.length - 1; i >= 0; i--) { + triggerUpdate(components[i]); + } + }); + + expectUpdates(expectation); + } + + testUpdates( + [root.refs.switcher.refs.box, root.refs.switcher], + ['Switcher', 'Box', 'Child'] + ); + + testUpdates( + [root.refs.child, root.refs.switcher.refs.box], + ['Box', 'Child'] + ); + + testUpdates( + [root.refs.child, root.refs.switcher], + ['Switcher', 'Box', 'Child'] + ); + }); }); diff --git a/src/dom/__tests__/Danger-test.js b/src/dom/__tests__/Danger-test.js index d3f94a4244..f82a97241e 100644 --- a/src/dom/__tests__/Danger-test.js +++ b/src/dom/__tests__/Danger-test.js @@ -36,14 +36,18 @@ describe('Danger', function() { }); it('should render markup', function() { - var markup = (
).mountComponent('.rX', transaction); + var markup = (
).mountComponent('.rX', transaction, 0); var output = Danger.dangerouslyRenderMarkup([markup])[0]; expect(output.nodeName).toBe('DIV'); }); it('should render markup with props', function() { - var markup = (
).mountComponent('.rX', transaction); + var markup = (
).mountComponent( + '.rX', + transaction, + 0 + ); var output = Danger.dangerouslyRenderMarkup([markup])[0]; expect(output.nodeName).toBe('DIV'); @@ -51,7 +55,7 @@ describe('Danger', function() { }); it('should render wrapped markup', function() { - var markup = ().mountComponent('.rX', transaction); + var markup = ().mountComponent('.rX', transaction, 0); var output = Danger.dangerouslyRenderMarkup([markup])[0]; expect(output.nodeName).toBe('TH'); diff --git a/src/environment/ReactServerRendering.js b/src/environment/ReactServerRendering.js index 748f9f35cc..626cf117b0 100644 --- a/src/environment/ReactServerRendering.js +++ b/src/environment/ReactServerRendering.js @@ -34,7 +34,7 @@ function renderComponentToString(component, callback) { transaction.reinitializeTransaction(); try { transaction.perform(function() { - var markup = component.mountComponent(id, transaction); + var markup = component.mountComponent(id, transaction, 0); markup = ReactMarkupChecksum.addChecksumToMarkup(markup); callback(markup); }, null);