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 387a1fa69c7ea..586dd4f7e35ee 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 cf71005350f9b..de03d875b0ecc 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 2ff9091c1a94a..ca34089157cb4 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 0e90be788a43c..6ef95ccaa3aba 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 3772571f8dd86..b8813947b04d7 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 a4c3ee9c0925a..8fdaec8c1baf7 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?',