From 1b868cbd71e677165dc1e12a06cf72b6b6fc9faa Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Sat, 17 Dec 2016 11:16:27 -0800 Subject: [PATCH] Don't warn about class component usage of getInitialState if state is also set We warn about getInitialState() usage in class components because users may have accidentally used it instead of state. If they have also specified state though then we should skip this warning. Context: https://twitter.com/soprano/status/810003963286163456 --- scripts/fiber/tests-passing.txt | 3 +++ .../ReactCoffeeScriptClass-test.coffee | 19 +++++++++++++++++++ .../class/__tests__/ReactES6Class-test.js | 15 +++++++++++++++ .../__tests__/ReactTypeScriptClass-test.ts | 18 ++++++++++++++++++ .../shared/fiber/ReactFiberClassComponent.js | 3 ++- .../reconciler/ReactCompositeComponent.js | 3 ++- 6 files changed, 59 insertions(+), 2 deletions(-) diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 387a1fa69c..586dd4f7e3 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -400,6 +400,7 @@ src/isomorphic/modern/class/__tests__/ReactCoffeeScriptClass-test.coffee * renders using forceUpdate even when there is no state * will call all the normal life cycle methods * warns when classic properties are defined on the instance, but does not invoke them. +* does not warn about getInitialState() on class components if state is also defined. * should warn when misspelling shouldComponentUpdate * should warn when misspelling componentWillReceiveProps * should throw AND warn when trying to access classic APIs @@ -422,6 +423,7 @@ src/isomorphic/modern/class/__tests__/ReactES6Class-test.js * renders using forceUpdate even when there is no state * will call all the normal life cycle methods * warns when classic properties are defined on the instance, but does not invoke them. +* does not warn about getInitialState() on class components if state is also defined. * should warn when misspelling shouldComponentUpdate * should warn when misspelling componentWillReceiveProps * should throw AND warn when trying to access classic APIs @@ -449,6 +451,7 @@ src/isomorphic/modern/class/__tests__/ReactTypeScriptClass-test.ts * renders using forceUpdate even when there is no state * will call all the normal life cycle methods * warns when classic properties are defined on the instance, but does not invoke them. +* does not warn about getInitialState() on class components if state is also defined. * should warn when misspelling shouldComponentUpdate * should warn when misspelling componentWillReceiveProps * should throw AND warn when trying to access classic APIs diff --git a/src/isomorphic/modern/class/__tests__/ReactCoffeeScriptClass-test.coffee b/src/isomorphic/modern/class/__tests__/ReactCoffeeScriptClass-test.coffee index cf71005350..de03d875b0 100644 --- a/src/isomorphic/modern/class/__tests__/ReactCoffeeScriptClass-test.coffee +++ b/src/isomorphic/modern/class/__tests__/ReactCoffeeScriptClass-test.coffee @@ -319,6 +319,25 @@ describe 'ReactCoffeeScriptClass', -> ) undefined + it 'does not warn about getInitialState() on class components + if state is also defined.', -> + spyOn console, 'error' + class Foo extends React.Component + constructor: (props) -> + super props + @state = bar: @props.initialValue + + getInitialState: -> + {} + + render: -> + span + className: 'foo' + + test React.createElement(Foo), 'SPAN', 'foo' + expect(console.error.calls.count()).toBe 0 + undefined + it 'should warn when misspelling shouldComponentUpdate', -> spyOn console, 'error' class NamedComponent extends React.Component diff --git a/src/isomorphic/modern/class/__tests__/ReactES6Class-test.js b/src/isomorphic/modern/class/__tests__/ReactES6Class-test.js index 2ff9091c1a..ca34089157 100644 --- a/src/isomorphic/modern/class/__tests__/ReactES6Class-test.js +++ b/src/isomorphic/modern/class/__tests__/ReactES6Class-test.js @@ -354,6 +354,21 @@ describe('ReactES6Class', () => { ); }); + it('does not warn about getInitialState() on class components if state is also defined.', () => { + spyOn(console, 'error'); + class Foo extends React.Component { + state = this.getInitialState(); + getInitialState() { + return {}; + } + render() { + return ; + } + } + test(, 'SPAN', 'foo'); + expect(console.error.calls.count()).toBe(0); + }); + it('should warn when misspelling shouldComponentUpdate', () => { spyOn(console, 'error'); diff --git a/src/isomorphic/modern/class/__tests__/ReactTypeScriptClass-test.ts b/src/isomorphic/modern/class/__tests__/ReactTypeScriptClass-test.ts index 0e90be788a..6ef95ccaa3 100644 --- a/src/isomorphic/modern/class/__tests__/ReactTypeScriptClass-test.ts +++ b/src/isomorphic/modern/class/__tests__/ReactTypeScriptClass-test.ts @@ -454,6 +454,24 @@ describe('ReactTypeScriptClass', function() { ); }); + it('does not warn about getInitialState() on class components ' + + 'if state is also defined.', () => { + spyOn(console, 'error'); + + class Example extends React.Component { + state = {}; + getInitialState() { + return {}; + } + render() { + return React.createElement('span', {className: 'foo'}); + } + } + + test(React.createElement(Example), 'SPAN', 'foo'); + expect((console.error).calls.count()).toBe(0); + }); + it('should warn when misspelling shouldComponentUpdate', function() { spyOn(console, 'error'); diff --git a/src/renderers/shared/fiber/ReactFiberClassComponent.js b/src/renderers/shared/fiber/ReactFiberClassComponent.js index 3772571f8d..b8813947b0 100644 --- a/src/renderers/shared/fiber/ReactFiberClassComponent.js +++ b/src/renderers/shared/fiber/ReactFiberClassComponent.js @@ -104,7 +104,8 @@ module.exports = function( ); const noGetInitialStateOnES6 = ( !instance.getInitialState || - instance.getInitialState.isReactClassApproved + instance.getInitialState.isReactClassApproved || + instance.state ); warning( noGetInitialStateOnES6, diff --git a/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js b/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js index a4c3ee9c09..8fdaec8c1b 100644 --- a/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js +++ b/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js @@ -271,7 +271,8 @@ var ReactCompositeComponent = { // catch them here, at initialization time, instead. warning( !inst.getInitialState || - inst.getInitialState.isReactClassApproved, + inst.getInitialState.isReactClassApproved || + inst.state, '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?',