diff --git a/src/browser/ui/ReactDOMComponent.js b/src/browser/ui/ReactDOMComponent.js index 862e1af6a1..3bdae4cb36 100644 --- a/src/browser/ui/ReactDOMComponent.js +++ b/src/browser/ui/ReactDOMComponent.js @@ -173,8 +173,8 @@ ReactDOMComponent.Mixin = { transaction, mountDepth ); - assertValidProps(this.props); this._previousStyleCopy = null; + assertValidProps(this._currentElement.props); var closeTag = omittedCloseTags[this._tag] ? '' : ''; return ( this._createOpenTagMarkupAndPutListeners(transaction) + @@ -197,7 +197,7 @@ ReactDOMComponent.Mixin = { * @return {string} Markup of opening tag. */ _createOpenTagMarkupAndPutListeners: function(transaction) { - var props = this.props; + var props = this._currentElement.props; var ret = '<' + this._tag; for (var propKey in props) { @@ -253,16 +253,18 @@ ReactDOMComponent.Mixin = { prefix = '\n'; } + var props = this._currentElement.props; + // Intentional use of != to avoid catching zero/false. - var innerHTML = this.props.dangerouslySetInnerHTML; + var innerHTML = props.dangerouslySetInnerHTML; if (innerHTML != null) { if (innerHTML.__html != null) { return prefix + innerHTML.__html; } } else { var contentToUse = - CONTENT_TYPES[typeof this.props.children] ? this.props.children : null; - var childrenToUse = contentToUse != null ? null : this.props.children; + CONTENT_TYPES[typeof props.children] ? props.children : null; + var childrenToUse = contentToUse != null ? null : props.children; if (contentToUse != null) { return prefix + escapeTextForBrowser(contentToUse); } else if (childrenToUse != null) { @@ -338,7 +340,7 @@ ReactDOMComponent.Mixin = { * @param {ReactReconcileTransaction} transaction */ _updateDOMProperties: function(lastProps, transaction) { - var nextProps = this.props; + var nextProps = this._currentElement.props; var propKey; var styleName; var styleUpdates; @@ -427,7 +429,7 @@ ReactDOMComponent.Mixin = { * @param {ReactReconcileTransaction} transaction */ _updateDOMChildren: function(lastProps, transaction) { - var nextProps = this.props; + var nextProps = this._currentElement.props; var lastContent = CONTENT_TYPES[typeof lastProps.children] ? lastProps.children : null; diff --git a/src/browser/ui/__tests__/ReactDOMComponent-test.js b/src/browser/ui/__tests__/ReactDOMComponent-test.js index 3ab38722bf..58216c236e 100644 --- a/src/browser/ui/__tests__/ReactDOMComponent-test.js +++ b/src/browser/ui/__tests__/ReactDOMComponent-test.js @@ -226,7 +226,7 @@ describe('ReactDOMComponent', function() { var ReactReconcileTransaction = require('ReactReconcileTransaction'); var NodeStub = function(initialProps) { - this.props = initialProps || {}; + this._currentElement = { props: initialProps }; this._rootNodeID = 'test'; }; assign(NodeStub.prototype, ReactDOMComponent.Mixin); @@ -276,7 +276,7 @@ describe('ReactDOMComponent', function() { var ReactReconcileTransaction = require('ReactReconcileTransaction'); var NodeStub = function(initialProps) { - this.props = initialProps || {}; + this._currentElement = { props: initialProps }; this._rootNodeID = 'test'; }; assign(NodeStub.prototype, ReactDOMComponent.Mixin); diff --git a/src/core/ReactComponent.js b/src/core/ReactComponent.js index 564c7311cc..39f7381ccb 100644 --- a/src/core/ReactComponent.js +++ b/src/core/ReactComponent.js @@ -20,21 +20,6 @@ var assign = require('Object.assign'); var invariant = require('invariant'); var keyMirror = require('keyMirror'); -/** - * Every React component is in one of these life cycles. - */ -var ComponentLifeCycle = keyMirror({ - /** - * Mounted components have a DOM node representation and are capable of - * receiving new props. - */ - MOUNTED: null, - /** - * Unmounted components are inactive and cannot receive new props. - */ - UNMOUNTED: null -}); - var injected = false; /** @@ -115,11 +100,6 @@ var ReactComponent = { } }, - /** - * @internal - */ - LifeCycle: ComponentLifeCycle, - /** * Injected module that provides ability to mutate individual properties. * Injected into the base class because many different subclasses need access @@ -137,17 +117,6 @@ var ReactComponent = { */ Mixin: { - /** - * Checks whether or not this component is mounted. - * - * @return {boolean} True if mounted, false otherwise. - * @final - * @protected - */ - isMounted: function() { - return this._lifeCycleState === ComponentLifeCycle.MOUNTED; - }, - /** * Sets a subset of the props. * @@ -175,10 +144,6 @@ var ReactComponent = { * @public */ replaceProps: function(props, callback) { - invariant( - this.isMounted(), - 'replaceProps(...): Can only update a mounted component.' - ); invariant( this._mountDepth === 0, 'replaceProps(...): You called `setProps` or `replaceProps` on a ' + @@ -225,19 +190,6 @@ var ReactComponent = { * @internal */ construct: function(element) { - // This is the public exposed props object after it has been processed - // with default props. The element's props represents the true internal - // state of the props. - this.props = element.props; - // Record the component responsible for creating this component. - // This is accessible through the element but we maintain an extra - // field for compatibility with devtools and as a way to make an - // incremental update. TODO: Consider deprecating this field. - this._owner = element._owner; - - // All components start unmounted. - this._lifeCycleState = ComponentLifeCycle.UNMOUNTED; - // See ReactUpdates. this._pendingCallbacks = null; @@ -262,20 +214,12 @@ var ReactComponent = { * @internal */ mountComponent: function(rootID, transaction, mountDepth) { - invariant( - !this.isMounted(), - 'mountComponent(%s, ...): Can only mount an unmounted component. ' + - 'Make sure to avoid storing components between renders or reusing a ' + - 'single component instance in multiple places.', - rootID - ); var ref = this._currentElement.ref; if (ref != null) { var owner = this._currentElement._owner; attachRef(ref, this, owner); } this._rootNodeID = rootID; - this._lifeCycleState = ComponentLifeCycle.MOUNTED; this._mountDepth = mountDepth; // Effectively: return ''; }, @@ -291,17 +235,15 @@ var ReactComponent = { * @internal */ unmountComponent: function() { - invariant( - this.isMounted(), - 'unmountComponent(): Can only unmount a mounted component.' - ); var ref = this._currentElement.ref; if (ref != null) { - detachRef(ref, this, this._owner); + detachRef(ref, this, this._currentElement._owner); } unmountIDFromEnvironment(this._rootNodeID); + // Reset all fields this._rootNodeID = null; - this._lifeCycleState = ComponentLifeCycle.UNMOUNTED; + this._pendingCallbacks = null; + this._pendingElement = null; }, /** @@ -316,10 +258,6 @@ var ReactComponent = { * @internal */ receiveComponent: function(nextElement, transaction) { - invariant( - this.isMounted(), - 'receiveComponent(...): Can only update a mounted component.' - ); this._pendingElement = nextElement; this.performUpdateIfNecessary(transaction); }, @@ -337,8 +275,6 @@ var ReactComponent = { var prevElement = this._currentElement; var nextElement = this._pendingElement; this._currentElement = nextElement; - this.props = nextElement.props; - this._owner = nextElement._owner; this._pendingElement = null; this.updateComponent(transaction, prevElement, nextElement); }, @@ -416,34 +352,6 @@ var ReactComponent = { mountImageIntoNode(markup, container, shouldReuseMarkup); }, - /** - * Checks if this component is owned by the supplied `owner` component. - * - * @param {ReactComponent} owner Component to check. - * @return {boolean} True if `owners` owns this component. - * @final - * @internal - */ - isOwnedBy: function(owner) { - return this._owner === owner; - }, - - /** - * Gets another component, that shares the same owner as this one, by ref. - * - * @param {string} ref of a sibling Component. - * @return {?ReactComponent} the actual sibling Component. - * @final - * @internal - */ - getSiblingByRef: function(ref) { - var owner = this._owner; - if (!owner || !owner.refs) { - return null; - } - return owner.refs[ref]; - }, - /** * Get the publicly accessible representation of this component - i.e. what * is exposed by refs and renderComponent. Can be null for stateless diff --git a/src/core/ReactCompositeComponent.js b/src/core/ReactCompositeComponent.js index 7d5e66e5fe..b0284cc7ae 100644 --- a/src/core/ReactCompositeComponent.js +++ b/src/core/ReactCompositeComponent.js @@ -16,19 +16,19 @@ var ReactContext = require('ReactContext'); var ReactCurrentOwner = require('ReactCurrentOwner'); var ReactElement = require('ReactElement'); var ReactInstanceMap = require('ReactInstanceMap'); -var ReactOwner = require('ReactOwner'); var ReactPerf = require('ReactPerf'); var ReactPropTypeLocations = require('ReactPropTypeLocations'); var ReactUpdates = require('ReactUpdates'); var assign = require('Object.assign'); +var emptyObject = require('emptyObject'); var invariant = require('invariant'); var keyMirror = require('keyMirror'); var shouldUpdateReactComponent = require('shouldUpdateReactComponent'); var warning = require('warning'); function getDeclarationErrorAddendum(component) { - var owner = component._owner || null; + var owner = component._currentElement._owner || null; if (owner) { var constructor = owner._instance.constructor; if (constructor && constructor.displayName) { @@ -56,8 +56,8 @@ function validateLifeCycleOnReplaceState(instance) { * `ReactCompositeComponent` maintains an auxiliary life cycle state in * `this._compositeLifeCycleState` (which can be null). * - * This is different from the life cycle state maintained by `ReactComponent` in - * `this._lifeCycleState`. The following diagram shows how the states overlap in + * This is different from the life cycle state maintained by `ReactComponent`. + * The following diagram shows how the states overlap in * time. There are times when the CompositeLifeCycle is null - at those times it * is only meaningful to look at ComponentLifeCycle alone. * @@ -100,8 +100,7 @@ var CompositeLifeCycle = keyMirror({ * @lends {ReactCompositeComponent.prototype} */ var ReactCompositeComponentMixin = assign({}, - ReactComponent.Mixin, - ReactOwner.Mixin, { + ReactComponent.Mixin, { /** * Base constructor for all composite component. @@ -114,13 +113,13 @@ var ReactCompositeComponentMixin = assign({}, this._instance.props = element.props; this._instance.state = null; this._instance.context = null; + this._instance.refs = emptyObject; this._pendingState = null; this._compositeLifeCycleState = null; // Children can be either an array or more than one argument ReactComponent.Mixin.construct.apply(this, arguments); - ReactOwner.Mixin.construct.apply(this, arguments); }, /** @@ -130,8 +129,7 @@ var ReactCompositeComponentMixin = assign({}, * @final */ isMounted: function() { - return ReactComponent.Mixin.isMounted.call(this) && - this._compositeLifeCycleState !== CompositeLifeCycle.MOUNTING; + return this._compositeLifeCycleState !== CompositeLifeCycle.MOUNTING; }, /** @@ -233,6 +231,9 @@ var ReactCompositeComponentMixin = assign({}, this._renderedComponent.unmountComponent(); this._renderedComponent = null; + // Reset pending fields + this._pendingState = null; + this._pendingForceUpdate = false; ReactComponent.Mixin.unmountComponent.call(this); // Delete the reference from the instance to this internal representation @@ -555,11 +556,6 @@ var ReactCompositeComponentMixin = assign({}, inst.props = nextProps; inst.state = nextState; inst.context = nextContext; - - // Owner cannot change because shouldUpdateReactComponent doesn't allow - // it. TODO: Remove this._owner completely. - this._owner = nextParentElement._owner; - return; } @@ -607,10 +603,6 @@ var ReactCompositeComponentMixin = assign({}, inst.state = nextState; inst.context = nextContext; - // Owner cannot change because shouldUpdateReactComponent doesn't allow - // it. TODO: Remove this._owner completely. - this._owner = nextElement._owner; - this._updateRenderedComponent(transaction); if (inst.componentDidUpdate) { @@ -699,6 +691,32 @@ var ReactCompositeComponentMixin = assign({}, } ), + /** + * Lazily allocates the refs object and stores `component` as `ref`. + * + * @param {string} ref Reference name. + * @param {component} component Component to store as `ref`. + * @final + * @private + */ + attachRef: function(ref, component) { + var inst = this.getPublicInstance(); + var refs = inst.refs === emptyObject ? (inst.refs = {}) : inst.refs; + refs[ref] = component.getPublicInstance(); + }, + + /** + * Detaches a reference name. + * + * @param {string} ref Name to dereference. + * @final + * @private + */ + detachRef: function(ref) { + var refs = this.getPublicInstance().refs; + delete refs[ref]; + }, + /** * Get the publicly accessible representation of this component - i.e. what * is exposed by refs and renderComponent. Can be null for stateless diff --git a/src/core/ReactElementValidator.js b/src/core/ReactElementValidator.js index e93dae340f..51866856dd 100644 --- a/src/core/ReactElementValidator.js +++ b/src/core/ReactElementValidator.js @@ -160,7 +160,7 @@ function monitorUseOfObjectMap() { } /** - * Ensure that every component either is passed in a static location, in an + * Ensure that every element either is passed in a static location, in an * array with an explicit keys property defined, or in an object literal * with valid key property. * diff --git a/src/core/ReactOwner.js b/src/core/ReactOwner.js index 22e493493e..29da2705a9 100644 --- a/src/core/ReactOwner.js +++ b/src/core/ReactOwner.js @@ -11,7 +11,6 @@ "use strict"; -var emptyObject = require('emptyObject'); var invariant = require('invariant'); /** @@ -103,53 +102,6 @@ var ReactOwner = { if (owner.getPublicInstance().refs[ref] === component.getPublicInstance()) { owner.detachRef(ref); } - }, - - /** - * A ReactComponent must mix this in to have refs. - * - * @lends {ReactOwner.prototype} - */ - Mixin: { - - construct: function() { - var inst = this.getPublicInstance(); - inst.refs = emptyObject; - }, - - /** - * Lazily allocates the refs object and stores `component` as `ref`. - * - * @param {string} ref Reference name. - * @param {component} component Component to store as `ref`. - * @final - * @private - */ - attachRef: function(ref, component) { - // TODO: Remove this invariant. This is never exposed and cannot be called - // by user code. The unit test is already removed. - invariant( - component.isOwnedBy(this), - 'attachRef(%s, ...): Only a component\'s owner can store a ref to it.', - ref - ); - var inst = this.getPublicInstance(); - var refs = inst.refs === emptyObject ? (inst.refs = {}) : inst.refs; - refs[ref] = component.getPublicInstance(); - }, - - /** - * Detaches a reference name. - * - * @param {string} ref Name to dereference. - * @final - * @private - */ - detachRef: function(ref) { - var refs = this.getPublicInstance().refs; - delete refs[ref]; - } - } }; diff --git a/src/core/ReactUpdates.js b/src/core/ReactUpdates.js index 7486e3ac67..d9d6f8cc45 100644 --- a/src/core/ReactUpdates.js +++ b/src/core/ReactUpdates.js @@ -136,24 +136,23 @@ function runBatchedUpdates(transaction) { dirtyComponents.sort(mountDepthComparator); for (var i = 0; i < len; 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 + // If a component is unmounted before pending changes apply, it will still + // be here, but we assume that it has cleared its _pendingCallbacks and + // that performUpdateIfNecessary is a noop. var component = dirtyComponents[i]; - if (component.isMounted()) { - // If performUpdateIfNecessary happens to enqueue any new updates, we - // shouldn't execute the callbacks until the next render happens, so - // stash the callbacks first - var callbacks = component._pendingCallbacks; - component._pendingCallbacks = null; - component.performUpdateIfNecessary(transaction.reconcileTransaction); + // If performUpdateIfNecessary happens to enqueue any new updates, we + // shouldn't execute the callbacks until the next render happens, so + // stash the callbacks first + var callbacks = component._pendingCallbacks; + component._pendingCallbacks = null; + component.performUpdateIfNecessary(transaction.reconcileTransaction); - if (callbacks) { - for (var j = 0; j < callbacks.length; j++) { - transaction.callbackQueue.enqueue( - callbacks[j], - component - ); - } + if (callbacks) { + for (var j = 0; j < callbacks.length; j++) { + transaction.callbackQueue.enqueue( + callbacks[j], + component + ); } } } diff --git a/src/core/__tests__/ReactComponentLifeCycle-test.js b/src/core/__tests__/ReactComponentLifeCycle-test.js index 214b18d017..8f145924fe 100644 --- a/src/core/__tests__/ReactComponentLifeCycle-test.js +++ b/src/core/__tests__/ReactComponentLifeCycle-test.js @@ -11,12 +11,13 @@ "use strict"; +var keyMirror = require('keyMirror'); + var React; var ReactTestUtils; var ReactComponent; var ReactCompositeComponent; var ReactInstanceMap; -var ComponentLifeCycle; var CompositeComponentLifeCycle; var getCompositeLifeCycle; @@ -69,6 +70,21 @@ var POST_WILL_UNMOUNT_STATE = { hasWillUnmountCompleted: true }; +/** + * Every React component is in one of these life cycles. + */ +var ComponentLifeCycle = keyMirror({ + /** + * Mounted components have a DOM node representation and are capable of + * receiving new props. + */ + MOUNTED: null, + /** + * Unmounted components are inactive and cannot receive new props. + */ + UNMOUNTED: null +}); + /** * TODO: We should make any setState calls fail in * `getInitialState` and `componentWillMount`. They will usually fail @@ -83,7 +99,6 @@ describe('ReactComponentLifeCycle', function() { ReactTestUtils = require('ReactTestUtils'); ReactComponent = require('ReactComponent'); ReactCompositeComponent = require('ReactCompositeComponent'); - ComponentLifeCycle = ReactComponent.LifeCycle; CompositeComponentLifeCycle = ReactCompositeComponent.LifeCycle; ReactInstanceMap = require('ReactInstanceMap'); @@ -94,13 +109,11 @@ describe('ReactComponentLifeCycle', function() { getLifeCycleState = function(instance) { var internalInstance = ReactInstanceMap.get(instance); - if (!internalInstance) { - // Once a component is fully unmounted, we cannot actually get to the - // internal instance. It's already dereferenced and possibly GC:ed. - // So the unmounted life cycle hook doesn't exist anymore. - return ComponentLifeCycle.UNMOUNTED; - } - return internalInstance._lifeCycleState; + // Once a component gets mounted, it has an internal instance, once it + // gets unmounted, it loses that internal instance. + return internalInstance ? + ComponentLifeCycle.MOUNTED : + ComponentLifeCycle.UNMOUNTED; }; }); diff --git a/src/core/__tests__/ReactElement-test.js b/src/core/__tests__/ReactElement-test.js index 38772a90e9..1a8f530838 100644 --- a/src/core/__tests__/ReactElement-test.js +++ b/src/core/__tests__/ReactElement-test.js @@ -173,6 +173,36 @@ describe('ReactElement', function() { ); }); + it('warns for keys for arrays of elements in rest args', function() { + spyOn(console, 'warn'); + var Component = React.createFactory(ComponentClass); + + var InnerClass = React.createClass({ + displayName: 'InnerClass', + render: function() { + return Component(null, this.props.childSet); + } + }); + + var InnerComponent = React.createFactory(InnerClass); + + var ComponentWrapper = React.createClass({ + displayName: 'ComponentWrapper', + render: function() { + return InnerComponent({ childSet: [ Component(), Component() ] }); + } + }); + + ReactTestUtils.renderIntoDocument(); + + expect(console.warn.argsForCall.length).toBe(1); + expect(console.warn.argsForCall[0][0]).toContain( + 'Each child in an array or iterator should have a unique "key" prop. ' + + 'Check the render method of InnerClass. ' + + 'It was passed a child from ComponentWrapper. ' + ); + }); + it('warns for keys for iterables of elements in rest args', function() { spyOn(console, 'warn'); var Component = React.createFactory(ComponentClass); diff --git a/src/core/__tests__/ReactUpdates-test.js b/src/core/__tests__/ReactUpdates-test.js index 5774560007..816ec2e053 100644 --- a/src/core/__tests__/ReactUpdates-test.js +++ b/src/core/__tests__/ReactUpdates-test.js @@ -815,4 +815,46 @@ describe('ReactUpdates', function() { 'asap-1.2' ]); }); + + it('does not call render after a component as been deleted', function() { + + var renderCount = 0; + var componentB = null; + + var B = React.createClass({ + getInitialState: function() { + return { updates: 0 }; + }, + componentDidMount: function() { + componentB = this; + }, + render: function() { + renderCount++; + return
; + } + }); + + var A = React.createClass({ + getInitialState: function() { + return { showB: true }; + }, + render: function() { + return this.state.showB ? :
; + } + }); + + var container = document.createElement('div'); + + var component = React.render(, container); + + ReactUpdates.batchedUpdates(function() { + // B will have scheduled an update but the batching should ensure that its + // update never fires. + componentB.setState({ updates: 1 }); + component.setState({ showB: false }); + }); + + expect(renderCount).toBe(1); + }); + }); diff --git a/src/test/ReactDefaultPerf.js b/src/test/ReactDefaultPerf.js index ce10bc6a52..d91a61d70e 100644 --- a/src/test/ReactDefaultPerf.js +++ b/src/test/ReactDefaultPerf.js @@ -244,7 +244,8 @@ var ReactDefaultPerf = { entry.displayNames[rootNodeID] = { current: this.constructor.displayName, - owner: this._owner ? this._owner.constructor.displayName : '' + owner: this._currentElement._owner ? + this._currentElement._owner.constructor.displayName : '' }; return rv;