From 766a79c6957ddc3b7040fbbb748df48ac0fcb625 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Thu, 22 Jan 2015 18:16:32 -0800 Subject: [PATCH 1/2] Move component class instantiation into ReactCompositeComponent We need to move instantiation into the mount phase for context purposes. To do this I moved the shallow rendering stuff into ReactTestUtils and reused more of the existing code for it by instantiating a noop child. Everywhere we refer to the "type" we should pass it to ReactNativeComponent to resolve any string value into the underlying composite. --- src/classic/element/ReactElementValidator.js | 22 ++- src/core/ReactCompositeComponent.js | 176 ++++++------------- src/core/ReactNativeComponent.js | 42 +++-- src/core/instantiateReactComponent.js | 27 +-- src/test/ReactTestUtils.js | 39 +++- 5 files changed, 139 insertions(+), 167 deletions(-) diff --git a/src/classic/element/ReactElementValidator.js b/src/classic/element/ReactElementValidator.js index 150f595e118..7e3426633d1 100644 --- a/src/classic/element/ReactElementValidator.js +++ b/src/classic/element/ReactElementValidator.js @@ -22,6 +22,7 @@ var ReactElement = require('ReactElement'); var ReactPropTypeLocations = require('ReactPropTypeLocations'); var ReactPropTypeLocationNames = require('ReactPropTypeLocationNames'); var ReactCurrentOwner = require('ReactCurrentOwner'); +var ReactNativeComponent = require('ReactNativeComponent'); var getIteratorFn = require('getIteratorFn'); var monitorCodeUse = require('monitorCodeUse'); @@ -361,26 +362,33 @@ var ReactElementValidator = { } if (type) { - var name = type.displayName || type.name; - if (type.propTypes) { + // Extract the component class from the element. Converts string types + // to a composite class which may have propTypes. + // TODO: Validating a string's propTypes is not decoupled from the + // rendering target which is problematic. + var componentClass = ReactNativeComponent.getComponentClassForElement( + element + ); + var name = componentClass.displayName || componentClass.name; + if (componentClass.propTypes) { checkPropTypes( name, - type.propTypes, + componentClass.propTypes, element.props, ReactPropTypeLocations.prop ); } - if (type.contextTypes) { + if (componentClass.contextTypes) { checkPropTypes( name, - type.contextTypes, + componentClass.contextTypes, element._context, ReactPropTypeLocations.context ); } - if (typeof type.getDefaultProps === 'function') { + if (typeof componentClass.getDefaultProps === 'function') { warning( - type.getDefaultProps.isReactClassApproved, + componentClass.getDefaultProps.isReactClassApproved, 'getDefaultProps is only used on classic React.createClass ' + 'definitions. Use a static property named `defaultProps` instead.' ); diff --git a/src/core/ReactCompositeComponent.js b/src/core/ReactCompositeComponent.js index 7747b800e7d..aa0af8c092c 100644 --- a/src/core/ReactCompositeComponent.js +++ b/src/core/ReactCompositeComponent.js @@ -17,6 +17,7 @@ var ReactContext = require('ReactContext'); var ReactCurrentOwner = require('ReactCurrentOwner'); var ReactElement = require('ReactElement'); var ReactInstanceMap = require('ReactInstanceMap'); +var ReactNativeComponent = require('ReactNativeComponent'); var ReactPerf = require('ReactPerf'); var ReactPropTypeLocations = require('ReactPropTypeLocations'); var ReactPropTypeLocationNames = require('ReactPropTypeLocationNames'); @@ -108,12 +109,7 @@ var ReactCompositeComponentMixin = assign({}, */ construct: function(element) { this._rootNodeID = null; - - this._instance.props = element.props; - // instance.state get set up to its proper initial value in mount - // which may be null. - this._instance.context = null; - this._instance.refs = emptyObject; + this._instance = null; this._pendingElement = null; this._pendingContext = null; @@ -165,18 +161,31 @@ var ReactCompositeComponentMixin = assign({}, this._mountOrder = nextMountID++; this._rootNodeID = rootID; - var inst = this._instance; + var publicProps = this._processProps(this._currentElement.props); + var publicContext = this._processContext(this._currentElement._context); + + var Component = ReactNativeComponent.getComponentClassForElement( + this._currentElement + ); + + // Initialize the public class + var inst = new Component(publicProps); + // These should be set up in the constructor, but as a convenience for + // simpler class abstractions, we set them up after the fact. + inst.props = publicProps; + inst.context = publicContext; + inst.refs = emptyObject; + + this._instance = inst; // Store a reference from the instance back to the internal representation ReactInstanceMap.set(inst, this); this._compositeLifeCycleState = CompositeLifeCycle.MOUNTING; - inst.context = this._processContext(this._currentElement._context); if (__DEV__) { this._warnIfContextsDiffer(this._currentElement._context, context); } - inst.props = this._processProps(this._currentElement.props); if (__DEV__) { // Since plain JS classes are defined without any special initialization @@ -485,7 +494,12 @@ var ReactCompositeComponentMixin = assign({}, */ _maskContext: function(context) { var maskedContext = null; - var contextTypes = this._instance.constructor.contextTypes; + // This really should be getting the component class for the element, + // but we know that we're not going to need it for built-ins. + if (typeof this._currentElement.type === 'string') { + return emptyObject; + } + var contextTypes = this._currentElement.type.contextTypes; if (!contextTypes) { return emptyObject; } @@ -506,16 +520,17 @@ var ReactCompositeComponentMixin = assign({}, */ _processContext: function(context) { var maskedContext = this._maskContext(context); - var contextTypes = this._instance.constructor.contextTypes; - if (!contextTypes) { - return emptyObject; - } if (__DEV__) { - this._checkPropTypes( - contextTypes, - maskedContext, - ReactPropTypeLocations.context + var Component = ReactNativeComponent.getComponentClassForElement( + this._currentElement ); + if (Component.contextTypes) { + this._checkPropTypes( + Component.contextTypes, + maskedContext, + ReactPropTypeLocations.context + ); + } } return maskedContext; }, @@ -566,10 +581,15 @@ var ReactCompositeComponentMixin = assign({}, */ _processProps: function(newProps) { if (__DEV__) { - var inst = this._instance; - var propTypes = inst.constructor.propTypes; - if (propTypes) { - this._checkPropTypes(propTypes, newProps, ReactPropTypeLocations.prop); + var Component = ReactNativeComponent.getComponentClassForElement( + this._currentElement + ); + if (Component.propTypes) { + this._checkPropTypes( + Component.propTypes, + newProps, + ReactPropTypeLocations.prop + ); } } return newProps; @@ -894,15 +914,22 @@ var ReactCompositeComponentMixin = assign({}, transaction, context ); - ReactComponentEnvironment.replaceNodeWithMarkupByID( - prevComponentID, - nextMarkup - ); + this._replaceNodeWithMarkupByID(prevComponentID, nextMarkup); } }, /** - * @private + * @protected + */ + _replaceNodeWithMarkupByID: function(prevComponentID, nextMarkup) { + ReactComponentEnvironment.replaceNodeWithMarkupByID( + prevComponentID, + nextMarkup + ); + }, + + /** + * @protected */ _renderValidatedComponentWithoutOwnerOrContext: function() { var inst = this._instance; @@ -983,7 +1010,7 @@ var ReactCompositeComponentMixin = assign({}, */ getName: function() { var type = this._currentElement.type; - var constructor = this._instance.constructor; + var constructor = this._instance && this._instance.constructor; return ( type.displayName || (constructor && constructor.displayName) || type.name || (constructor && constructor.name) || @@ -1008,95 +1035,6 @@ var ReactCompositeComponentMixin = assign({}, }); -var ShallowMixin = assign({}, - ReactCompositeComponentMixin, { - - /** - * Initializes the component, renders markup, and registers event listeners. - * - * @param {string} rootID DOM ID of the root node. - * @param {ReactReconcileTransaction|ReactServerRenderingTransaction} transaction - * @return {ReactElement} Shallow rendering of the component. - * @final - * @internal - */ - mountComponent: function(rootID, transaction, context) { - ReactComponent.Mixin.mountComponent.call( - this, - rootID, - transaction, - context - ); - - var inst = this._instance; - - // Store a reference from the instance back to the internal representation - ReactInstanceMap.set(inst, this); - - this._compositeLifeCycleState = CompositeLifeCycle.MOUNTING; - - // No context for shallow-mounted components. - inst.props = this._processProps(this._currentElement.props); - - var initialState = inst.state; - if (initialState === undefined) { - inst.state = initialState = null; - } - invariant( - typeof initialState === 'object' && !Array.isArray(initialState), - '%s.state: must be set to an object or null', - this.getName() || 'ReactCompositeComponent' - ); - - this._pendingState = null; - this._pendingForceUpdate = false; - - if (inst.componentWillMount) { - inst.componentWillMount(); - // When mounting, calls to `setState` by `componentWillMount` will set - // `this._pendingState` without triggering a re-render. - if (this._pendingState) { - inst.state = this._pendingState; - this._pendingState = null; - } - } - - // No recursive call to instantiateReactComponent for shallow rendering. - this._renderedComponent = - this._renderValidatedComponentWithoutOwnerOrContext(); - - // Done with mounting, `setState` will now trigger UI changes. - this._compositeLifeCycleState = null; - - // No call to this._renderedComponent.mountComponent for shallow - // rendering. - - if (inst.componentDidMount) { - transaction.getReactMountReady().enqueue(inst.componentDidMount, inst); - } - - return this._renderedComponent; - }, - - /** - * Call the component's `render` method and update the DOM accordingly. - * - * @param {ReactReconcileTransaction} transaction - * @internal - */ - _updateRenderedComponent: function(transaction) { - var prevComponentInstance = this._renderedComponent; - var prevRenderedElement = prevComponentInstance._currentElement; - // Use the without-owner-or-context variant of _rVC below: - var nextRenderedElement = - this._renderValidatedComponentWithoutOwnerOrContext(); - // This is a noop in shallow render - shouldUpdateReactComponent(prevRenderedElement, nextRenderedElement); - this._renderedComponent = nextRenderedElement; - } - -}); - ReactPerf.measureMethods( ReactCompositeComponentMixin, 'ReactCompositeComponent', @@ -1111,9 +1049,7 @@ var ReactCompositeComponent = { LifeCycle: CompositeLifeCycle, - Mixin: ReactCompositeComponentMixin, - - ShallowMixin: ShallowMixin + Mixin: ReactCompositeComponentMixin }; diff --git a/src/core/ReactNativeComponent.js b/src/core/ReactNativeComponent.js index fb2cde26446..2448d2b420b 100644 --- a/src/core/ReactNativeComponent.js +++ b/src/core/ReactNativeComponent.js @@ -57,27 +57,36 @@ function autoGenerateWrapperClass(type) { } /** - * Create an internal class for a specific tag. + * Get a composite component wrapper class for a specific tag. * - * @param {string} tag The tag for which to create an internal instance. - * @param {any} props The props passed to the instance constructor. - * @return {ReactComponent} component The injected empty component. + * @param {ReactElement} element The tag for which to get the class. + * @return {function} The React class constructor function. */ -function createInstanceForTag(tag, props, parentType) { +function getComponentClassForElement(element) { + if (typeof element.type === 'function') { + return element.type; + } + var tag = element.type; var componentClass = tagToComponentClass[tag]; if (componentClass == null) { tagToComponentClass[tag] = componentClass = autoGenerateWrapperClass(tag); } - if (parentType === tag) { - // Avoid recursion - invariant( - genericComponentClass, - 'There is no registered component for the tag %s', - tag - ); - return new genericComponentClass(tag, props); - } - return new componentClass(props); + return componentClass; +} + +/** + * Get a native internal component class for a specific tag. + * + * @param {ReactElement} element The element to create. + * @return {function} The internal class constructor function. + */ +function createInternalComponent(element) { + invariant( + genericComponentClass, + 'There is no registered component for the tag %s', + element.type + ); + return new genericComponentClass(element.type, element.props); } /** @@ -97,7 +106,8 @@ function isTextComponent(component) { } var ReactNativeComponent = { - createInstanceForTag: createInstanceForTag, + getComponentClassForElement: getComponentClassForElement, + createInternalComponent: createInternalComponent, createInstanceForText: createInstanceForText, isTextComponent: isTextComponent, injection: ReactNativeComponentInjection diff --git a/src/core/instantiateReactComponent.js b/src/core/instantiateReactComponent.js index bc524836b9a..da6fc46c2db 100644 --- a/src/core/instantiateReactComponent.js +++ b/src/core/instantiateReactComponent.js @@ -21,9 +21,7 @@ var invariant = require('invariant'); var warning = require('warning'); // To avoid a cyclic dependency, we create the final class in this module -var ReactCompositeComponentWrapper = function(inst) { - this._instance = inst; -}; +var ReactCompositeComponentWrapper = function() { }; assign( ReactCompositeComponentWrapper.prototype, ReactCompositeComponent.Mixin, @@ -73,28 +71,19 @@ function instantiateReactComponent(node, parentCompositeType) { } // Special case string values - if (typeof element.type === 'string') { - instance = ReactNativeComponent.createInstanceForTag( - element.type, - element.props, - parentCompositeType - ); - // If the injected special class is not an internal class, but another - // composite, then we must wrap it. - // TODO: Move this resolution around to something cleaner. - if (typeof instance.mountComponent !== 'function') { - instance = new ReactCompositeComponentWrapper(instance); - } + if (parentCompositeType === element.type && + typeof element.type === 'string') { + // Avoid recursion if the wrapper renders itself. + instance = ReactNativeComponent.createInternalComponent(element); + // All native components are currently wrapped in a composite so we're + // safe to assume that this is what we should instantiate. } else if (isInternalComponentType(element.type)) { // This is temporarily available for custom components that are not string // represenations. I.e. ART. Once those are updated to use the string // representation, we can drop this code path. instance = new element.type(element); } else { - // TODO: Update to follow new ES6 initialization. Ideally, we can use - // props in property initializers. - var inst = new element.type(element.props); - instance = new ReactCompositeComponentWrapper(inst); + instance = new ReactCompositeComponentWrapper(); } } else if (typeof node === 'string' || typeof node === 'number') { instance = ReactNativeComponent.createInstanceForText(node); diff --git a/src/test/ReactTestUtils.js b/src/test/ReactTestUtils.js index b2e6eb0b1fe..4be012dd7bd 100644 --- a/src/test/ReactTestUtils.js +++ b/src/test/ReactTestUtils.js @@ -321,15 +321,44 @@ var ReactShallowRenderer = function() { }; ReactShallowRenderer.prototype.getRenderOutput = function() { - return (this._instance && this._instance._renderedComponent) || null; + return ( + (this._instance && this._instance._renderedComponent && + this._instance._renderedComponent._currentElement) + || null + ); }; -var ShallowComponentWrapper = function(inst) { - this._instance = inst; +var NoopInternalComponent = function(element) { + this._currentElement = element; +} + +NoopInternalComponent.prototype = { + + mountComponent: function() { + }, + + receiveComponent: function(element) { + this._currentElement = element; + }, + + unmountComponent: function() { + + } + }; + +var ShallowComponentWrapper = function() { }; assign( ShallowComponentWrapper.prototype, - ReactCompositeComponent.ShallowMixin + ReactCompositeComponent.Mixin, { + _instantiateReactComponent: function(element) { + return new NoopInternalComponent(element); + }, + _replaceNodeWithMarkupByID: function() {}, + _renderValidatedComponent: + ReactCompositeComponent.Mixin. + _renderValidatedComponentWithoutOwnerOrContext + } ); ReactShallowRenderer.prototype.render = function(element, context) { @@ -341,7 +370,7 @@ ReactShallowRenderer.prototype.render = function(element, context) { ReactShallowRenderer.prototype._render = function(element, transaction, context) { if (!this._instance) { var rootID = ReactInstanceHandles.createReactRootID(); - var instance = new ShallowComponentWrapper(new element.type(element.props)); + var instance = new ShallowComponentWrapper(element.type); instance.construct(element); instance.mountComponent(rootID, transaction, context); From 9abd1133c94288c09512f7cf0b8a0b8136df6da7 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 23 Jan 2015 10:08:40 -0800 Subject: [PATCH 2/2] Pass context to the constructor This should reenable reading this.context from getInitialState. Added a bunch of tests for this too. --- src/classic/class/ReactClass.js | 3 +- .../class/__tests__/ReactClass-test.js | 30 ++++++++++++++++++ src/core/ReactCompositeComponent.js | 2 +- src/modern/class/ReactComponentBase.js | 3 +- .../class/__tests__/ReactES6Class-test.js | 31 +++++++++++++++++++ 5 files changed, 66 insertions(+), 3 deletions(-) diff --git a/src/classic/class/ReactClass.js b/src/classic/class/ReactClass.js index cd1f2fb403f..b5359d076bb 100644 --- a/src/classic/class/ReactClass.js +++ b/src/classic/class/ReactClass.js @@ -805,7 +805,7 @@ var ReactClass = { * @public */ createClass: function(spec) { - var Constructor = function(props) { + var Constructor = function(props, context) { // This constructor is overridden by mocks. The argument is used // by mocks to assert on what gets mounted. @@ -815,6 +815,7 @@ var ReactClass = { } this.props = props; + this.context = context; this.state = null; // ReactClasses doesn't have constructors. Instead, they use the diff --git a/src/classic/class/__tests__/ReactClass-test.js b/src/classic/class/__tests__/ReactClass-test.js index e2aeb41b489..b0d3b45b5af 100644 --- a/src/classic/class/__tests__/ReactClass-test.js +++ b/src/classic/class/__tests__/ReactClass-test.js @@ -278,6 +278,36 @@ describe('ReactClass-spec', function() { expect(instance.state.occupation).toEqual('clown'); }); + it('renders based on context getInitialState', function() { + var Foo = React.createClass({ + contextTypes: { + className: React.PropTypes.string + }, + getInitialState() { + return { className: this.context.className }; + }, + render() { + return ; + } + }); + + var Outer = React.createClass({ + childContextTypes: { + className: React.PropTypes.string + }, + getChildContext() { + return { className: 'foo' }; + }, + render() { + return ; + } + }); + + var container = document.createElement('div'); + React.render(, container); + expect(container.firstChild.className).toBe('foo'); + }); + it('should throw with non-object getInitialState() return values', function() { [['an array'], 'a string', 1234].forEach(function(state) { var Component = React.createClass({ diff --git a/src/core/ReactCompositeComponent.js b/src/core/ReactCompositeComponent.js index aa0af8c092c..fd9eed50a0c 100644 --- a/src/core/ReactCompositeComponent.js +++ b/src/core/ReactCompositeComponent.js @@ -169,7 +169,7 @@ var ReactCompositeComponentMixin = assign({}, ); // Initialize the public class - var inst = new Component(publicProps); + var inst = new Component(publicProps, publicContext); // These should be set up in the constructor, but as a convenience for // simpler class abstractions, we set them up after the fact. inst.props = publicProps; diff --git a/src/modern/class/ReactComponentBase.js b/src/modern/class/ReactComponentBase.js index a3308f64ff8..fcdc0e2b79d 100644 --- a/src/modern/class/ReactComponentBase.js +++ b/src/modern/class/ReactComponentBase.js @@ -19,8 +19,9 @@ var warning = require('warning'); /** * Base class helpers for the updating state of a component. */ -function ReactComponentBase(props) { +function ReactComponentBase(props, context) { this.props = props; + this.context = context; } /** diff --git a/src/modern/class/__tests__/ReactES6Class-test.js b/src/modern/class/__tests__/ReactES6Class-test.js index 527916b7a56..ffeddabda3b 100644 --- a/src/modern/class/__tests__/ReactES6Class-test.js +++ b/src/modern/class/__tests__/ReactES6Class-test.js @@ -98,6 +98,37 @@ describe('ReactES6Class', function() { test(, 'SPAN', 'bar'); }); + it('renders based on context in the constructor', function() { + class Foo extends React.Component { + constructor(props, context) { + super(props, context); + this.state = { tag: context.tag, className: this.context.className }; + } + render() { + var Tag = this.state.tag; + return ; + } + } + Foo.contextTypes = { + tag: React.PropTypes.string, + className: React.PropTypes.string + }; + + class Outer extends React.Component { + getChildContext() { + return { tag: 'span', className: 'foo' }; + } + render() { + return ; + } + } + Outer.childContextTypes = { + tag: React.PropTypes.string, + className: React.PropTypes.string + }; + test(, 'SPAN', 'foo'); + }); + it('renders only once when setting state in componentWillMount', function() { var renderCount = 0; class Foo extends React.Component {