diff --git a/src/browser/ReactTextComponent.js b/src/browser/ReactTextComponent.js index f87930f3f7..8750377391 100644 --- a/src/browser/ReactTextComponent.js +++ b/src/browser/ReactTextComponent.js @@ -18,6 +18,7 @@ var ReactElement = require('ReactElement'); var assign = require('Object.assign'); var escapeTextForBrowser = require('escapeTextForBrowser'); +var invariant = require('invariant'); /** * Text nodes violate a couple assumptions that React makes about components: @@ -84,12 +85,22 @@ assign(ReactTextComponent.prototype, ReactComponent.Mixin, { receiveComponent: function(nextComponent, transaction) { var nextProps = nextComponent.props; if (nextProps !== this.props) { + // TODO: Save this as pending props and use performUpdateIfNecessary + // and/or updateComponent to do the actual update for consistency with + // other component types? this.props = nextProps; ReactComponent.BackendIDOperations.updateTextContentByID( this._rootNodeID, nextProps ); } + }, + + updateComponent: function() { + invariant( + false, + 'ReactTextComponent: updateComponent() should never be called' + ); } }); diff --git a/src/browser/ui/ReactDOMComponent.js b/src/browser/ui/ReactDOMComponent.js index 72a4aadf8b..71f80ecfbb 100644 --- a/src/browser/ui/ReactDOMComponent.js +++ b/src/browser/ui/ReactDOMComponent.js @@ -303,18 +303,20 @@ ReactDOMComponent.Mixin = { * * @param {ReactReconcileTransaction} transaction * @param {ReactElement} prevElement + * @param {ReactElement} nextElement * @internal * @overridable */ updateComponent: ReactPerf.measure( 'ReactDOMComponent', 'updateComponent', - function(transaction, prevElement) { + function(transaction, prevElement, nextElement) { assertValidProps(this._currentElement.props); ReactComponent.Mixin.updateComponent.call( this, transaction, - prevElement + prevElement, + nextElement ); this._updateDOMProperties(prevElement.props, transaction); this._updateDOMChildren(prevElement.props, transaction); diff --git a/src/core/ReactComponent.js b/src/core/ReactComponent.js index bc9e0c01d5..9d4e4b7e58 100644 --- a/src/core/ReactComponent.js +++ b/src/core/ReactComponent.js @@ -323,7 +323,7 @@ var ReactComponent = { this.props = nextElement.props; this._owner = nextElement._owner; this._pendingElement = null; - this.updateComponent(transaction, prevElement); + this.updateComponent(transaction, prevElement, nextElement); }, /** @@ -331,11 +331,10 @@ var ReactComponent = { * * @param {ReactReconcileTransaction} transaction * @param {object} prevElement + * @param {object} nextElement * @internal */ - updateComponent: function(transaction, prevElement) { - var nextElement = this._currentElement; - + updateComponent: function(transaction, prevElement, nextElement) { // If either the owner or a `ref` has changed, make sure the newest owner // has stored a reference to `this`, and the previous owner (if different) // has forgotten the reference to `this`. We use the element instead diff --git a/src/core/ReactCompositeComponent.js b/src/core/ReactCompositeComponent.js index 1b063b9064..24dfc9f5af 100644 --- a/src/core/ReactCompositeComponent.js +++ b/src/core/ReactCompositeComponent.js @@ -1006,6 +1006,26 @@ var ReactCompositeComponentMixin = { } }, + receiveComponent: function(nextElement, transaction) { + if (nextElement === this._currentElement && + nextElement._owner != null) { + // Since elements are immutable after the owner is rendered, + // we can do a cheap identity compare here to determine if this is a + // superfluous reconcile. It's possible for state to be mutable but such + // change should trigger an update of the owner which would recreate + // the element. We explicitly check for the existence of an owner since + // it's possible for an element created outside a composite to be + // deeply mutated and reused. + return; + } + + ReactComponent.Mixin.receiveComponent.call( + this, + nextElement, + transaction + ); + }, + /** * If any of `_pendingElement`, `_pendingState`, or `_pendingForceUpdate` * is set, update the component. @@ -1028,64 +1048,107 @@ var ReactCompositeComponentMixin = { return; } - var nextContext = this.context; - var nextProps = this.props; - var nextElement = this._currentElement; + var prevElement = this._currentElement; + var nextElement = prevElement; if (this._pendingElement != null) { nextElement = this._pendingElement; - nextContext = this._processContext(nextElement._context); - nextProps = this._processProps(nextElement.props); this._pendingElement = null; - - this._compositeLifeCycleState = CompositeLifeCycle.RECEIVING_PROPS; - if (this.componentWillReceiveProps) { - this.componentWillReceiveProps(nextProps, nextContext); - } } - this._compositeLifeCycleState = null; + this.updateComponent( + transaction, + prevElement, + nextElement + ); + }, - var nextState = this._pendingState || this.state; - this._pendingState = null; + /** + * Perform an update to a mounted component. The componentWillReceiveProps and + * shouldComponentUpdate methods are called, then (assuming the update isn't + * skipped) the remaining update lifecycle methods are called and the DOM + * representation is updated. + * + * By default, this implements React's rendering and reconciliation algorithm. + * Sophisticated clients may wish to override this. + * + * @param {ReactReconcileTransaction} transaction + * @param {ReactElement} prevParentElement + * @param {ReactElement} nextParentElement + * @internal + * @overridable + */ + updateComponent: ReactPerf.measure( + 'ReactCompositeComponent', + 'updateComponent', + function(transaction, prevParentElement, nextParentElement) { + // Update refs regardless of what shouldComponentUpdate returns + ReactComponent.Mixin.updateComponent.call( + this, + transaction, + prevParentElement, + nextParentElement + ); - var shouldUpdate = - this._pendingForceUpdate || - !this.shouldComponentUpdate || - this.shouldComponentUpdate(nextProps, nextState, nextContext); + var prevContext = this.context; + var prevProps = this.props; + var nextContext = prevContext; + var nextProps = prevProps; + // Distinguish between a props update versus a simple state update + if (prevParentElement !== nextParentElement) { + nextContext = this._processContext(nextParentElement._context); + nextProps = this._processProps(nextParentElement.props); - if (__DEV__) { - if (typeof shouldUpdate === "undefined") { - console.warn( - (this.constructor.displayName || 'ReactCompositeComponent') + - '.shouldComponentUpdate(): Returned undefined instead of a ' + - 'boolean value. Make sure to return true or false.' - ); + this._compositeLifeCycleState = CompositeLifeCycle.RECEIVING_PROPS; + if (this.componentWillReceiveProps) { + this.componentWillReceiveProps(nextProps, nextContext); + } + } + + this._compositeLifeCycleState = null; + + var nextState = this._pendingState || this.state; + this._pendingState = null; + + var shouldUpdate = + this._pendingForceUpdate || + !this.shouldComponentUpdate || + this.shouldComponentUpdate(nextProps, nextState, nextContext); + + if (__DEV__) { + if (typeof shouldUpdate === "undefined") { + console.warn( + (this.constructor.displayName || 'ReactCompositeComponent') + + '.shouldComponentUpdate(): Returned undefined instead of a ' + + 'boolean value. Make sure to return true or false.' + ); + } + } + + if (!shouldUpdate) { + // If it's determined that a component should not update, we still want + // to set props and state but we shortcut the rest of the update. + this._currentElement = nextParentElement; + this.props = nextProps; + this.state = nextState; + this.context = nextContext; + + // Owner cannot change because shouldUpdateReactComponent doesn't allow + // it. TODO: Remove this._owner completely. + this._owner = nextParentElement._owner; + + return; } - } - if (shouldUpdate) { this._pendingForceUpdate = false; // Will set `this.props`, `this.state` and `this.context`. this._performComponentUpdate( - nextElement, + nextParentElement, nextProps, nextState, nextContext, transaction ); - } else { - // If it's determined that a component should not update, we still want - // to set props and state. - this._currentElement = nextElement; - this.props = nextProps; - this.state = nextState; - this.context = nextContext; - - // Owner cannot change because shouldUpdateReactComponent doesn't allow - // it. TODO: Remove this._owner completely. - this._owner = nextElement._owner; - } - }, + }), /** * Merges new props and state, notifies delegate methods of update and @@ -1105,7 +1168,6 @@ var ReactCompositeComponentMixin = { nextContext, transaction ) { - var prevElement = this._currentElement; var prevProps = this.props; var prevState = this.state; var prevContext = this.context; @@ -1123,10 +1185,7 @@ var ReactCompositeComponentMixin = { // it. TODO: Remove this._owner completely. this._owner = nextElement._owner; - this.updateComponent( - transaction, - prevElement - ); + this._updateRenderedComponent(transaction); if (this.componentDidUpdate) { transaction.getReactMountReady().enqueue( @@ -1136,73 +1195,41 @@ var ReactCompositeComponentMixin = { } }, - receiveComponent: function(nextElement, transaction) { - if (nextElement === this._currentElement && - nextElement._owner != null) { - // Since elements are immutable after the owner is rendered, - // we can do a cheap identity compare here to determine if this is a - // superfluous reconcile. It's possible for state to be mutable but such - // change should trigger an update of the owner which would recreate - // the element. We explicitly check for the existence of an owner since - // it's possible for a element created outside a composite to be - // deeply mutated and reused. - return; - } - - ReactComponent.Mixin.receiveComponent.call( - this, - nextElement, - transaction - ); - }, - /** - * Updates the component's currently mounted DOM representation. - * - * By default, this implements React's rendering and reconciliation algorithm. - * Sophisticated clients may wish to override this. + * Call the component's `render` method and update the DOM accordingly. * * @param {ReactReconcileTransaction} transaction - * @param {ReactElement} prevElement * @internal - * @overridable */ - updateComponent: ReactPerf.measure( - 'ReactCompositeComponent', - 'updateComponent', - function(transaction, prevParentElement) { - ReactComponent.Mixin.updateComponent.call( - this, + _updateRenderedComponent: function(transaction) { + var prevComponentInstance = this._renderedComponent; + var prevRenderedElement = prevComponentInstance._currentElement; + var nextRenderedElement = this._renderValidatedComponent(); + if (shouldUpdateReactComponent(prevRenderedElement, nextRenderedElement)) { + prevComponentInstance.receiveComponent( + nextRenderedElement, + transaction + ); + } else { + // These two IDs are actually the same! But nothing should rely on that. + var thisID = this._rootNodeID; + var prevComponentID = prevComponentInstance._rootNodeID; + prevComponentInstance.unmountComponent(); + this._renderedComponent = instantiateReactComponent( + nextRenderedElement, + this._currentElement.type + ); + var nextMarkup = this._renderedComponent.mountComponent( + thisID, transaction, - prevParentElement + this._mountDepth + 1 + ); + ReactComponent.BackendIDOperations.dangerouslyReplaceNodeWithMarkupByID( + prevComponentID, + nextMarkup ); - - var prevComponentInstance = this._renderedComponent; - var prevElement = prevComponentInstance._currentElement; - var nextElement = this._renderValidatedComponent(); - if (shouldUpdateReactComponent(prevElement, nextElement)) { - prevComponentInstance.receiveComponent(nextElement, transaction); - } else { - // These two IDs are actually the same! But nothing should rely on that. - var thisID = this._rootNodeID; - var prevComponentID = prevComponentInstance._rootNodeID; - prevComponentInstance.unmountComponent(); - this._renderedComponent = instantiateReactComponent( - nextElement, - this._currentElement.type - ); - var nextMarkup = this._renderedComponent.mountComponent( - thisID, - transaction, - this._mountDepth + 1 - ); - ReactComponent.BackendIDOperations.dangerouslyReplaceNodeWithMarkupByID( - prevComponentID, - nextMarkup - ); - } } - ), + }, /** * Forces an update. This should only be invoked when it is known with diff --git a/src/core/__tests__/ReactCompositeComponent-test.js b/src/core/__tests__/ReactCompositeComponent-test.js index bfe8e42371..cf253ffc33 100644 --- a/src/core/__tests__/ReactCompositeComponent-test.js +++ b/src/core/__tests__/ReactCompositeComponent-test.js @@ -1483,4 +1483,40 @@ describe('ReactCompositeComponent', function() { ); }); + it('should update refs if shouldComponentUpdate gives false', function() { + var Static = React.createClass({ + shouldComponentUpdate: function() { + return false; + }, + render: function() { + return