Skip to content

Move Component Class Instantiation into ReactCompositeComponent #2918

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 23, 2015
Merged

Move Component Class Instantiation into ReactCompositeComponent #2918

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
766a79c
Move component class instantiation into ReactCompositeComponent
sebmarkbage Jan 23, 2015
9abd113
Pass context to the constructor
sebmarkbage Jan 23, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/classic/class/ReactClass.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand All @@ -815,6 +815,7 @@ var ReactClass = {
}

this.props = props;
this.context = context;
this.state = null;

// ReactClasses doesn't have constructors. Instead, they use the
Expand Down
30 changes: 30 additions & 0 deletions src/classic/class/__tests__/ReactClass-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 <span className={this.state.className} />;
}
});

var Outer = React.createClass({
childContextTypes: {
className: React.PropTypes.string
},
getChildContext() {
return { className: 'foo' };
},
render() {
return <Foo />;
}
});

var container = document.createElement('div');
React.render(<Outer />, 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({
Expand Down
22 changes: 15 additions & 7 deletions src/classic/element/ReactElementValidator.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How so? Isn't componentClass.propTypes dependent on what ReactNativeComponent has injected?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but it's problematic if there are multiple output environments. E.g. if an ART subtree was defined in terms of string based target nodes. Then we don't know which of the multiple environments this particular string will end up at.

Another case is HTML/SVG where the <title /> element means different things depending on which parent it ends up in. So they should have different propTypes depending on the parent. We don't really have a good way to solve this issue.

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.'
);
Expand Down
176 changes: 56 additions & 120 deletions src/core/ReactCompositeComponent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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, 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;
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
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
},
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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) ||
Expand All @@ -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',
Expand All @@ -1111,9 +1049,7 @@ var ReactCompositeComponent = {

LifeCycle: CompositeLifeCycle,

Mixin: ReactCompositeComponentMixin,

ShallowMixin: ShallowMixin
Mixin: ReactCompositeComponentMixin

};

Expand Down
Loading