From 61abc645e50314187850d4e04ed6d257652f1a59 Mon Sep 17 00:00:00 2001 From: Ben Alpert Date: Wed, 20 Nov 2013 00:30:33 -0800 Subject: [PATCH] Move __owner__ off of props --- src/core/ReactComponent.js | 36 ++++++++++++++++---------- src/core/ReactCompositeComponent.js | 30 ++++++++++++++++++--- src/core/ReactDOMComponent.js | 9 +++++-- src/core/ReactPropTransferer.js | 2 +- src/core/shouldUpdateReactComponent.js | 2 +- src/dom/DOMPropertyOperations.js | 1 - 6 files changed, 58 insertions(+), 22 deletions(-) diff --git a/src/core/ReactComponent.js b/src/core/ReactComponent.js index f85c5cce251..3aaf1d355fb 100644 --- a/src/core/ReactComponent.js +++ b/src/core/ReactComponent.js @@ -81,8 +81,8 @@ function validateExplicitKey(component) { if (!component.isOwnedBy(ReactCurrentOwner.current)) { // Name of the component that originally created this child. var childOwnerName = - component.props.__owner__ && - component.props.__owner__.constructor.displayName; + component._owner && + component._owner.constructor.displayName; // Usually the current owner is the offender, but if it accepts // children as a property, it may be the creator of the child that's @@ -260,7 +260,7 @@ var ReactComponent = { */ replaceProps: function(props, callback) { invariant( - !this.props.__owner__, + !this._owner, 'replaceProps(...): You called `setProps` or `replaceProps` on a ' + 'component with an owner. This is an anti-pattern since props will ' + 'get reactively updated when rendered. Instead, change the owner\'s ' + @@ -288,13 +288,19 @@ var ReactComponent = { construct: function(initialProps, children) { this.props = initialProps || {}; // Record the component responsible for creating this component. - this.props.__owner__ = ReactCurrentOwner.current; + this._owner = ReactCurrentOwner.current; // All components start unmounted. this._lifeCycleState = ComponentLifeCycle.UNMOUNTED; this._pendingProps = null; this._pendingCallbacks = null; + // Unlike _pendingProps and _pendingCallbacks, we won't use null to + // indicate that nothing is pending because it's possible for a component + // to have a null owner. Instead, an owner change is pending when + // this._owner !== this._pendingOwner. + this._pendingOwner = this._owner; + // Children can be more than one argument var childrenLength = arguments.length - 1; if (childrenLength === 1) { @@ -336,7 +342,7 @@ var ReactComponent = { ); var props = this.props; if (props.ref != null) { - ReactOwner.addComponentAsRefTo(this, props.ref, props.__owner__); + ReactOwner.addComponentAsRefTo(this, props.ref, this._owner); } this._rootNodeID = rootID; this._lifeCycleState = ComponentLifeCycle.MOUNTED; @@ -361,7 +367,7 @@ var ReactComponent = { ); var props = this.props; if (props.ref != null) { - ReactOwner.removeComponentAsRefFrom(this, props.ref, props.__owner__); + ReactOwner.removeComponentAsRefFrom(this, props.ref, this._owner); } ReactComponent.unmountIDFromEnvironment(this._rootNodeID); this._rootNodeID = null; @@ -384,6 +390,7 @@ var ReactComponent = { this.isMounted(), 'receiveComponent(...): Can only update a mounted component.' ); + this._pendingOwner = nextComponent._owner; this._pendingProps = nextComponent.props; this._performUpdateIfNecessary(transaction); }, @@ -411,9 +418,11 @@ var ReactComponent = { return; } var prevProps = this.props; + var prevOwner = this._owner; this.props = this._pendingProps; + this._owner = this._pendingOwner; this._pendingProps = null; - this.updateComponent(transaction, prevProps); + this.updateComponent(transaction, prevProps, prevOwner); }, /** @@ -423,21 +432,20 @@ var ReactComponent = { * @param {object} prevProps * @internal */ - updateComponent: function(transaction, prevProps) { + updateComponent: function(transaction, prevProps, prevOwner) { var props = this.props; // 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`. - if (props.__owner__ !== prevProps.__owner__ || - props.ref !== prevProps.ref) { + if (this._owner !== prevOwner || props.ref !== prevProps.ref) { if (prevProps.ref != null) { ReactOwner.removeComponentAsRefFrom( - this, prevProps.ref, prevProps.__owner__ + this, prevProps.ref, prevOwner ); } // Correct, even if the owner is the same, and only the ref has changed. if (props.ref != null) { - ReactOwner.addComponentAsRefTo(this, props.ref, props.__owner__); + ReactOwner.addComponentAsRefTo(this, props.ref, this._owner); } } }, @@ -491,7 +499,7 @@ var ReactComponent = { * @internal */ isOwnedBy: function(owner) { - return this.props.__owner__ === owner; + return this._owner === owner; }, /** @@ -503,7 +511,7 @@ var ReactComponent = { * @internal */ getSiblingByRef: function(ref) { - var owner = this.props.__owner__; + var owner = this._owner; if (!owner || !owner.refs) { return null; } diff --git a/src/core/ReactCompositeComponent.js b/src/core/ReactCompositeComponent.js index 6c61396d47f..3960e93312b 100644 --- a/src/core/ReactCompositeComponent.js +++ b/src/core/ReactCompositeComponent.js @@ -833,6 +833,12 @@ var ReactCompositeComponentMixin = { this._compositeLifeCycleState = CompositeLifeCycle.RECEIVING_STATE; + // Unlike props, state, and context, we specifically don't want to set + // _pendingOwner to null here because it's possible for a component to have + // a null owner, so we instead make `this._owner === this._pendingOwner` + // mean that there's no owner change pending. + var nextOwner = this._pendingOwner; + var nextState = this._pendingState || this.state; this._pendingState = null; @@ -846,6 +852,7 @@ var ReactCompositeComponentMixin = { // Will set `this.props`, `this.state` and `this.context`. this._performComponentUpdate( nextProps, + nextOwner, nextState, nextContext, transaction @@ -854,6 +861,7 @@ var ReactCompositeComponentMixin = { // If it's determined that a component should not update, we still want // to set props and state. this.props = nextProps; + this._owner = nextOwner; this.state = nextState; this.context = nextContext; } @@ -866,6 +874,7 @@ var ReactCompositeComponentMixin = { * performs update. * * @param {object} nextProps Next object to set as properties. + * @param {?ReactComponent} nextOwner Next component to set as owner * @param {?object} nextState Next object to set as state. * @param {?object} nextContext Next object to set as context. * @param {ReactReconcileTransaction} transaction @@ -873,11 +882,13 @@ var ReactCompositeComponentMixin = { */ _performComponentUpdate: function( nextProps, + nextOwner, nextState, nextContext, transaction ) { var prevProps = this.props; + var prevOwner = this._owner; var prevState = this.state; var prevContext = this.context; @@ -886,12 +897,19 @@ var ReactCompositeComponentMixin = { } this.props = nextProps; + this._owner = nextOwner; this.state = nextState; this._currentContext = nextContext; this.context = this._processContext(nextContext); - this.updateComponent(transaction, prevProps, prevState, prevContext); + this.updateComponent( + transaction, + prevProps, + prevOwner, + prevState, + prevContext + ); if (this.componentDidUpdate) { transaction.getReactMountReady().enqueue( @@ -918,6 +936,7 @@ var ReactCompositeComponentMixin = { * * @param {ReactReconcileTransaction} transaction * @param {object} prevProps + * @param {?ReactComponent} prevOwner * @param {?object} prevState * @param {?object} prevContext * @internal @@ -926,8 +945,13 @@ var ReactCompositeComponentMixin = { updateComponent: ReactPerf.measure( 'ReactCompositeComponent', 'updateComponent', - function(transaction, prevProps, prevState, prevContext) { - ReactComponent.Mixin.updateComponent.call(this, transaction, prevProps); + function(transaction, prevProps, prevOwner, prevState, prevContext) { + ReactComponent.Mixin.updateComponent.call( + this, + transaction, + prevProps, + prevOwner + ); var prevComponent = this._renderedComponent; var nextComponent = this._renderValidatedComponent(); if (shouldUpdateReactComponent(prevComponent, nextComponent)) { diff --git a/src/core/ReactDOMComponent.js b/src/core/ReactDOMComponent.js index dcbc7911036..6a744b087a6 100644 --- a/src/core/ReactDOMComponent.js +++ b/src/core/ReactDOMComponent.js @@ -200,8 +200,13 @@ ReactDOMComponent.Mixin = { updateComponent: ReactPerf.measure( 'ReactDOMComponent', 'updateComponent', - function(transaction, prevProps) { - ReactComponent.Mixin.updateComponent.call(this, transaction, prevProps); + function(transaction, prevProps, prevOwner) { + ReactComponent.Mixin.updateComponent.call( + this, + transaction, + prevProps, + prevOwner + ); this._updateDOMProperties(prevProps); this._updateDOMChildren(prevProps, transaction); } diff --git a/src/core/ReactPropTransferer.js b/src/core/ReactPropTransferer.js index 44306526893..003213ff188 100644 --- a/src/core/ReactPropTransferer.js +++ b/src/core/ReactPropTransferer.js @@ -92,7 +92,7 @@ var ReactPropTransferer = { */ transferPropsTo: function(component) { invariant( - component.props.__owner__ === this, + component._owner === this, '%s: You can\'t call transferPropsTo() on a component that you ' + 'don\'t own, %s. This usually means you are calling ' + 'transferPropsTo() on a component passed in as props or children.', diff --git a/src/core/shouldUpdateReactComponent.js b/src/core/shouldUpdateReactComponent.js index 3ef1e7c3e72..e9b76c74e17 100644 --- a/src/core/shouldUpdateReactComponent.js +++ b/src/core/shouldUpdateReactComponent.js @@ -32,7 +32,7 @@ function shouldUpdateReactComponent(prevComponent, nextComponent) { // TODO: Remove warning after a release. if (prevComponent && nextComponent && prevComponent.constructor === nextComponent.constructor) { - if (prevComponent.props.__owner__ === nextComponent.props.__owner__) { + if (prevComponent._owner === nextComponent._owner) { return true; } else { if (__DEV__) { diff --git a/src/dom/DOMPropertyOperations.js b/src/dom/DOMPropertyOperations.js index 6f629d40838..2898b411442 100644 --- a/src/dom/DOMPropertyOperations.js +++ b/src/dom/DOMPropertyOperations.js @@ -30,7 +30,6 @@ var processAttributeNameAndPrefix = memoizeStringOnly(function(name) { if (__DEV__) { var reactProps = { - __owner__: true, children: true, dangerouslySetInnerHTML: true, key: true,