diff --git a/src/classic/class/ReactClass.js b/src/classic/class/ReactClass.js
index f5340e2d08..09dd3452e9 100644
--- a/src/classic/class/ReactClass.js
+++ b/src/classic/class/ReactClass.js
@@ -808,12 +808,22 @@ var ReactClass = {
// Initialize the defaultProps property after all mixins have been merged
if (Constructor.getDefaultProps) {
Constructor.defaultProps = Constructor.getDefaultProps();
- if (__DEV__) {
- // This is a tag to indicate that this use of getDefaultProps is ok,
- // since it's used with createClass. If it's not, then it's likely a
- // mistake so we'll warn you to use the static property instead.
+ }
+
+ if (__DEV__) {
+ // This is a tag to indicate that the use of these method names is ok,
+ // since it's used with createClass. If it's not, then it's likely a
+ // mistake so we'll warn you to use the static property, property
+ // initializer or constructor respectively.
+ if (Constructor.getDefaultProps) {
Constructor.getDefaultProps._isReactClassApproved = true;
}
+ if (Constructor.prototype.getInitialState) {
+ Constructor.prototype.getInitialState._isReactClassApproved = true;
+ }
+ if (Constructor.prototype.componentWillMount) {
+ Constructor.prototype.componentWillMount._isReactClassApproved = true;
+ }
}
invariant(
diff --git a/src/core/ReactCompositeComponent.js b/src/core/ReactCompositeComponent.js
index ab78a4d345..f2f321426a 100644
--- a/src/core/ReactCompositeComponent.js
+++ b/src/core/ReactCompositeComponent.js
@@ -32,8 +32,7 @@ var warning = require('warning');
function getDeclarationErrorAddendum(component) {
var owner = component._currentElement._owner || null;
if (owner) {
- var constructor = owner._instance.constructor;
- var name = constructor && (constructor.displayName || constructor.name);
+ var name = owner.getName();
if (name) {
return ' Check the render method of `' + name + '`.';
}
@@ -200,11 +199,50 @@ var ReactCompositeComponentMixin = assign({},
// deprecating this convenience.
initialState = null;
}
+ // Since plain JS classes are defined without any special initialization
+ // logic, we can not catch common errors early. Therefore, we have to
+ // catch them here, at initialization time, instead.
+ warning(
+ !inst.getInitialState ||
+ inst.getInitialState._isReactClassApproved,
+ 'getInitialState was defined on %s, a plain JavaScript class. ' +
+ 'This is only supported for classes created using React.createClass. ' +
+ 'Did you mean to define a state property instead?',
+ this.getName() || 'a component'
+ );
+ warning(
+ !inst.componentWillMount ||
+ inst.componentWillMount._isReactClassApproved,
+ 'componentWillMount was defined on %s, a plain JavaScript class. ' +
+ 'This is only supported for classes created using React.createClass. ' +
+ 'Did you mean to define a constructor instead?',
+ this.getName() || 'a component'
+ );
+ warning(
+ !inst.propTypes,
+ 'propTypes was defined as an instance property on %s. Use a static ' +
+ 'property to define propTypes instead.',
+ this.getName() || 'a component'
+ );
+ warning(
+ !inst.contextTypes,
+ 'contextTypes was defined as an instance property on %s. Use a ' +
+ 'static property to define contextTypes instead.',
+ this.getName() || 'a component'
+ );
+ warning(
+ typeof inst.componentShouldUpdate !== 'function',
+ '%s has a method called ' +
+ 'componentShouldUpdate(). Did you mean shouldComponentUpdate()? ' +
+ 'The name is phrased as a question because the function is ' +
+ 'expected to return a value.',
+ (this.getName() || 'A component')
+ );
}
invariant(
typeof initialState === 'object' && !Array.isArray(initialState),
'%s.getInitialState(): must return an object or null',
- inst.constructor.displayName || 'ReactCompositeComponent'
+ this.getName() || 'ReactCompositeComponent'
);
inst.state = initialState;
@@ -453,13 +491,12 @@ var ReactCompositeComponentMixin = assign({},
_processChildContext: function(currentContext) {
var inst = this._instance;
var childContext = inst.getChildContext && inst.getChildContext();
- var displayName = inst.constructor.displayName || 'ReactCompositeComponent';
if (childContext) {
invariant(
typeof inst.constructor.childContextTypes === 'object',
'%s.getChildContext(): childContextTypes must be defined in order to ' +
'use getChildContext().',
- displayName
+ this.getName() || 'ReactCompositeComponent'
);
if (__DEV__) {
this._checkPropTypes(
@@ -472,7 +509,7 @@ var ReactCompositeComponentMixin = assign({},
invariant(
name in inst.constructor.childContextTypes,
'%s.getChildContext(): key "%s" is not defined in childContextTypes.',
- displayName,
+ this.getName() || 'ReactCompositeComponent',
name
);
}
@@ -512,8 +549,7 @@ var ReactCompositeComponentMixin = assign({},
_checkPropTypes: function(propTypes, props, location) {
// TODO: Stop validating prop types here and only use the element
// validation.
- var componentName = this._instance.constructor.displayName ||
- this._instance.constructor.name;
+ var componentName = this.getName();
for (var propName in propTypes) {
if (propTypes.hasOwnProperty(propName)) {
var error;
@@ -614,7 +650,7 @@ var ReactCompositeComponentMixin = assign({},
_warnIfContextsDiffer: function(ownerBasedContext, parentBasedContext) {
var ownerKeys = Object.keys(ownerBasedContext).sort();
var parentKeys = Object.keys(parentBasedContext).sort();
- var displayName = this._instance.constructor.displayName || 'ReactCompositeComponent';
+ var displayName = this.getName() || 'ReactCompositeComponent';
if (ownerKeys.length !== parentKeys.length ||
ownerKeys.toString() !== parentKeys.toString()) {
warning(
@@ -706,7 +742,7 @@ var ReactCompositeComponentMixin = assign({},
if (__DEV__) {
if (typeof shouldUpdate === "undefined") {
console.warn(
- (inst.constructor.displayName || 'ReactCompositeComponent') +
+ (this.getName() || 'ReactCompositeComponent') +
'.shouldComponentUpdate(): Returned undefined instead of a ' +
'boolean value. Make sure to return true or false.'
);
@@ -863,7 +899,7 @@ var ReactCompositeComponentMixin = assign({},
ReactElement.isValidElement(renderedComponent),
'%s.render(): A valid ReactComponent must be returned. You may have ' +
'returned undefined, an array or some other invalid object.',
- inst.constructor.displayName || 'ReactCompositeComponent'
+ this.getName() || 'ReactCompositeComponent'
);
return renderedComponent;
},
@@ -893,6 +929,22 @@ var ReactCompositeComponentMixin = assign({},
var refs = this.getPublicInstance().refs;
delete refs[ref];
},
+
+ /**
+ * Get a text description of the component that can be used to identify it
+ * in error messages.
+ * @return {string} The name or null.
+ * @internal
+ */
+ getName: function() {
+ var type = this._currentElement.type;
+ var constructor = this._instance.constructor;
+ return (
+ type.displayName || (constructor && constructor.displayName) ||
+ type.name || (constructor && constructor.name) ||
+ null
+ );
+ },
/**
* Get the publicly accessible representation of this component - i.e. what
@@ -954,7 +1006,7 @@ var ShallowMixin = assign({},
invariant(
typeof initialState === 'object' && !Array.isArray(initialState),
'%s.getInitialState(): must return an object or null',
- inst.constructor.displayName || 'ReactCompositeComponent'
+ this.getName() || 'ReactCompositeComponent'
);
inst.state = initialState;
diff --git a/src/modern/class/__tests__/ReactES6Class-test.js b/src/modern/class/__tests__/ReactES6Class-test.js
index 1a78f397d5..b64e198864 100644
--- a/src/modern/class/__tests__/ReactES6Class-test.js
+++ b/src/modern/class/__tests__/ReactES6Class-test.js
@@ -90,6 +90,67 @@ describe('ReactES6Class', function() {
expect(renderedName).toBe('bar');
});
+ it('warns when classic properties are defined on the instance, ' +
+ 'but does not invoke them.', function() {
+ spyOn(console, 'warn');
+ var getInitialStateWasCalled = false;
+ var componentWillMountWasCalled = false;
+ class Foo extends React.Component {
+ constructor() {
+ this.contextTypes = {};
+ this.propTypes = {};
+ }
+ getInitialState() {
+ getInitialStateWasCalled = true;
+ return {};
+ }
+ componentWillMount() {
+ componentWillMountWasCalled = true;
+ }
+ render() {
+ return ;
+ }
+ }
+ test(, 'SPAN', 'foo');
+ // TODO: expect(getInitialStateWasCalled).toBe(false);
+ // TODO: expect(componentWillMountWasCalled).toBe(false);
+ expect(console.warn.calls.length).toBe(4);
+ expect(console.warn.calls[0].args[0]).toContain(
+ 'getInitialState was defined on Foo, a plain JavaScript class.'
+ );
+ expect(console.warn.calls[1].args[0]).toContain(
+ 'componentWillMount was defined on Foo, a plain JavaScript class.'
+ );
+ expect(console.warn.calls[2].args[0]).toContain(
+ 'propTypes was defined as an instance property on Foo.'
+ );
+ expect(console.warn.calls[3].args[0]).toContain(
+ 'contextTypes was defined as an instance property on Foo.'
+ );
+ });
+
+ it('should warn when mispelling shouldComponentUpdate', function() {
+ spyOn(console, 'warn');
+
+ class NamedComponent {
+ componentShouldUpdate() {
+ return false;
+ }
+ render() {
+ return ;
+ }
+ }
+ test(, 'SPAN', 'foo');
+
+ expect(console.warn.calls.length).toBe(1);
+ expect(console.warn.calls[0].args[0]).toBe(
+ 'Warning: ' +
+ 'NamedComponent has a method called componentShouldUpdate(). Did you ' +
+ 'mean shouldComponentUpdate()? The name is phrased as a question ' +
+ 'because the function is expected to return a value.'
+ );
+ });
+
it('should throw AND warn when trying to access classic APIs', function() {
spyOn(console, 'warn');
var instance = test(, 'DIV', 'foo');