From b8b10001f3501b807c74781428824d5f6d9917a0 Mon Sep 17 00:00:00 2001 From: Ben Alpert Date: Wed, 15 Apr 2015 17:32:09 -0700 Subject: [PATCH] Add warning for getDefaultProps on ES6 classes Fixes #3655. --- src/core/ReactCompositeComponent.js | 8 ++++++++ .../__tests__/ReactCoffeeScriptClass-test.coffee | 13 +++++++++++-- src/modern/class/__tests__/ReactES6Class-test.js | 13 +++++++++++-- .../class/__tests__/ReactTypeScriptClass-test.ts | 15 +++++++++++++-- 4 files changed, 43 insertions(+), 6 deletions(-) diff --git a/src/core/ReactCompositeComponent.js b/src/core/ReactCompositeComponent.js index e1f63ffa6a0df..788a16bd87c65 100644 --- a/src/core/ReactCompositeComponent.js +++ b/src/core/ReactCompositeComponent.js @@ -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 ' + diff --git a/src/modern/class/__tests__/ReactCoffeeScriptClass-test.coffee b/src/modern/class/__tests__/ReactCoffeeScriptClass-test.coffee index fb41a04734eb5..eaa2a433d92a3 100644 --- a/src/modern/class/__tests__/ReactCoffeeScriptClass-test.coffee +++ b/src/modern/class/__tests__/ReactCoffeeScriptClass-test.coffee @@ -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 = {} @@ -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.' ) diff --git a/src/modern/class/__tests__/ReactES6Class-test.js b/src/modern/class/__tests__/ReactES6Class-test.js index eb66f819a744b..84711158a51a7 100644 --- a/src/modern/class/__tests__/ReactES6Class-test.js +++ b/src/modern/class/__tests__/ReactES6Class-test.js @@ -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() { @@ -307,20 +308,28 @@ describe('ReactES6Class', function() { getInitialStateWasCalled = true; return {}; } + getDefaultProps() { + getDefaultPropsWasCalled = true; + return {}; + } render() { return ; } } test(, '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.' ); }); diff --git a/src/modern/class/__tests__/ReactTypeScriptClass-test.ts b/src/modern/class/__tests__/ReactTypeScriptClass-test.ts index 70423d8c0756b..772316ef4a6f2 100644 --- a/src/modern/class/__tests__/ReactTypeScriptClass-test.ts +++ b/src/modern/class/__tests__/ReactTypeScriptClass-test.ts @@ -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 {}; @@ -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.' ); });