Skip to content

Commit

Permalink
Add warning for getDefaultProps on ES6 classes
Browse files Browse the repository at this point in the history
  • Loading branch information
sophiebits committed Apr 16, 2015
1 parent be03fa7 commit 85b0edc
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 6 deletions.
8 changes: 8 additions & 0 deletions src/core/ReactCompositeComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,14 @@ var ReactCompositeComponentMixin = {
'Did you mean to define a state property instead?',
this.getName() || 'a component'
);
warning(
!inst.getDefaultProps ||
inst.getDefaultProps.isReactClassApproved,
'getDefaultProps was defined on %s, a plain JavaScript class. ' +
'This is only supported for classes created using React.createClass. ' +
'Use a static property to define defaultProps instead.',
this.getName() || 'a component'
);
warning(
!inst.propTypes,
'propTypes was defined as an instance property on %s. Use a static ' +
Expand Down
13 changes: 11 additions & 2 deletions src/modern/class/__tests__/ReactCoffeeScriptClass-test.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,7 @@ describe 'ReactCoffeeScriptClass', ->
but does not invoke them.', ->
spyOn console, 'error'
getInitialStateWasCalled = false
getDefaultPropsWasCalled = false
class Foo extends React.Component
constructor: ->
@contextTypes = {}
Expand All @@ -275,20 +276,28 @@ describe 'ReactCoffeeScriptClass', ->
getInitialStateWasCalled = true
{}

getDefaultProps: ->
getDefaultPropsWasCalled = true
{}

render: ->
span
className: 'foo'

test React.createElement(Foo), 'SPAN', 'foo'
expect(getInitialStateWasCalled).toBe false
expect(console.error.calls.length).toBe 3
expect(getDefaultPropsWasCalled).toBe false
expect(console.error.calls.length).toBe 4
expect(console.error.calls[0].args[0]).toContain(
'getInitialState was defined on Foo, a plain JavaScript class.'
)
expect(console.error.calls[1].args[0]).toContain(
'propTypes was defined as an instance property on Foo.'
'getDefaultProps was defined on Foo, a plain JavaScript class.'
)
expect(console.error.calls[2].args[0]).toContain(
'propTypes was defined as an instance property on Foo.'
)
expect(console.error.calls[3].args[0]).toContain(
'contextTypes was defined as an instance property on Foo.'
)

Expand Down
13 changes: 11 additions & 2 deletions src/modern/class/__tests__/ReactES6Class-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,7 @@ describe('ReactES6Class', function() {
it('warns when classic properties are defined on the instance, ' +
'but does not invoke them.', function() {
spyOn(console, 'error');
var getDefaultPropsWasCalled = false;
var getInitialStateWasCalled = false;
class Foo extends React.Component {
constructor() {
Expand All @@ -307,20 +308,28 @@ describe('ReactES6Class', function() {
getInitialStateWasCalled = true;
return {};
}
getDefaultProps() {
getDefaultPropsWasCalled = true;
return {};
}
render() {
return <span className="foo" />;
}
}
test(<Foo />, 'SPAN', 'foo');
expect(getInitialStateWasCalled).toBe(false);
expect(console.error.calls.length).toBe(3);
expect(getDefaultPropsWasCalled).toBe(false);
expect(console.error.calls.length).toBe(4);
expect(console.error.calls[0].args[0]).toContain(
'getInitialState was defined on Foo, a plain JavaScript class.'
);
expect(console.error.calls[1].args[0]).toContain(
'propTypes was defined as an instance property on Foo.'
'getDefaultProps was defined on Foo, a plain JavaScript class.'
);
expect(console.error.calls[2].args[0]).toContain(
'propTypes was defined as an instance property on Foo.'
);
expect(console.error.calls[3].args[0]).toContain(
'contextTypes was defined as an instance property on Foo.'
);
});
Expand Down
15 changes: 13 additions & 2 deletions src/modern/class/__tests__/ReactTypeScriptClass-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -235,9 +235,14 @@ class NormalLifeCycles {
// warns when classic properties are defined on the instance,
// but does not invoke them.
var getInitialStateWasCalled = false;
var getDefaultPropsWasCalled = false;
class ClassicProperties extends React.Component {
contextTypes = {};
propTypes = {};
getDefaultProps() {
getDefaultPropsWasCalled = true;
return {};
}
getInitialState() {
getInitialStateWasCalled = true;
return {};
Expand Down Expand Up @@ -410,17 +415,23 @@ describe('ReactTypeScriptClass', function() {
var warn = jest.genMockFn();
console.error = warn;
getInitialStateWasCalled = false;
getDefaultPropsWasCalled = false;
test(React.createElement(ClassicProperties), 'SPAN', 'foo');
expect(getInitialStateWasCalled).toBe(false);
expect(warn.mock.calls.length).toBe(3);
expect(getDefaultPropsWasCalled).toBe(false);
expect(warn.mock.calls.length).toBe(4);
expect(warn.mock.calls[0][0]).toContain(
'getInitialState was defined on ClassicProperties, ' +
'a plain JavaScript class.'
);
expect(warn.mock.calls[1][0]).toContain(
'propTypes was defined as an instance property on ClassicProperties.'
'getDefaultProps was defined on ClassicProperties, ' +
'a plain JavaScript class.'
);
expect(warn.mock.calls[2][0]).toContain(
'propTypes was defined as an instance property on ClassicProperties.'
);
expect(warn.mock.calls[3][0]).toContain(
'contextTypes was defined as an instance property on ClassicProperties.'
);
});
Expand Down

0 comments on commit 85b0edc

Please sign in to comment.