@samitouri / QOS-React / commits / 1b868cbd71

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

Brian Vaughn committed Dec 17, 2016 at 11:16 UTC 1b868cbd71e677165dc1e12a06cf72b6b6fc9faa
6 files changed +59 -2
scripts/fiber/tests-passing.txt
+3
@@ -400,6 +400,7 @@ src/isomorphic/modern/class/__tests__/ReactCoffeeScriptClass-test.coffee
400 * renders using forceUpdate even when there is no state
401 * will call all the normal life cycle methods
402 * warns when classic properties are defined on the instance, but does not invoke them.
403 +* does not warn about getInitialState() on class components if state is also defined.
404 * should warn when misspelling shouldComponentUpdate
405 * should warn when misspelling componentWillReceiveProps
406 * should throw AND warn when trying to access classic APIs
@@ -422,6 +423,7 @@ src/isomorphic/modern/class/__tests__/ReactES6Class-test.js
423 * renders using forceUpdate even when there is no state
424 * will call all the normal life cycle methods
425 * warns when classic properties are defined on the instance, but does not invoke them.
426 +* does not warn about getInitialState() on class components if state is also defined.
427 * should warn when misspelling shouldComponentUpdate
428 * should warn when misspelling componentWillReceiveProps
429 * should throw AND warn when trying to access classic APIs
@@ -449,6 +451,7 @@ src/isomorphic/modern/class/__tests__/ReactTypeScriptClass-test.ts
451 * renders using forceUpdate even when there is no state
452 * will call all the normal life cycle methods
453 * warns when classic properties are defined on the instance, but does not invoke them.
454 +* does not warn about getInitialState() on class components if state is also defined.
455 * should warn when misspelling shouldComponentUpdate
456 * should warn when misspelling componentWillReceiveProps
457 * should throw AND warn when trying to access classic APIs
src/isomorphic/modern/class/__tests__/ReactCoffeeScriptClass-test.coffee
+19
@@ -319,6 +319,25 @@ describe 'ReactCoffeeScriptClass', ->
319 )
320 undefined
321
322 + it 'does not warn about getInitialState() on class components
323 + if state is also defined.', ->
324 + spyOn console, 'error'
325 + class Foo extends React.Component
326 + constructor: (props) ->
327 + super props
328 + @state = bar: @props.initialValue
329 +
330 + getInitialState: ->
331 + {}
332 +
333 + render: ->
334 + span
335 + className: 'foo'
336 +
337 + test React.createElement(Foo), 'SPAN', 'foo'
338 + expect(console.error.calls.count()).toBe 0
339 + undefined
340 +
341 it 'should warn when misspelling shouldComponentUpdate', ->
342 spyOn console, 'error'
343 class NamedComponent extends React.Component
src/isomorphic/modern/class/__tests__/ReactES6Class-test.js
+15
@@ -354,6 +354,21 @@ describe('ReactES6Class', () => {
354 );
355 });
356
357 + it('does not warn about getInitialState() on class components if state is also defined.', () => {
358 + spyOn(console, 'error');
359 + class Foo extends React.Component {
360 + state = this.getInitialState();
361 + getInitialState() {
362 + return {};
363 + }
364 + render() {
365 + return <span className="foo" />;
366 + }
367 + }
368 + test(<Foo />, 'SPAN', 'foo');
369 + expect(console.error.calls.count()).toBe(0);
370 + });
371 +
372 it('should warn when misspelling shouldComponentUpdate', () => {
373 spyOn(console, 'error');
374
src/isomorphic/modern/class/__tests__/ReactTypeScriptClass-test.ts
+18
@@ -454,6 +454,24 @@ describe('ReactTypeScriptClass', function() {
454 );
455 });
456
457 + it('does not warn about getInitialState() on class components ' +
458 + 'if state is also defined.', () => {
459 + spyOn(console, 'error');
460 +
461 + class Example extends React.Component {
462 + state = {};
463 + getInitialState() {
464 + return {};
465 + }
466 + render() {
467 + return React.createElement('span', {className: 'foo'});
468 + }
469 + }
470 +
471 + test(React.createElement(Example), 'SPAN', 'foo');
472 + expect((<any>console.error).calls.count()).toBe(0);
473 + });
474 +
475 it('should warn when misspelling shouldComponentUpdate', function() {
476 spyOn(console, 'error');
477
src/renderers/shared/fiber/ReactFiberClassComponent.js
+2 -1
@@ -104,7 +104,8 @@ module.exports = function(
104 );
105 const noGetInitialStateOnES6 = (
106 !instance.getInitialState ||
107 - instance.getInitialState.isReactClassApproved
107 + instance.getInitialState.isReactClassApproved ||
108 + instance.state
109 );
110 warning(
111 noGetInitialStateOnES6,
src/renderers/shared/stack/reconciler/ReactCompositeComponent.js
+2 -1
@@ -271,7 +271,8 @@ var ReactCompositeComponent = {
271 // catch them here, at initialization time, instead.
272 warning(
273 !inst.getInitialState ||
274 - inst.getInitialState.isReactClassApproved,
274 + inst.getInitialState.isReactClassApproved ||
275 + inst.state,
276 'getInitialState was defined on %s, a plain JavaScript class. ' +
277 'This is only supported for classes created using React.createClass. ' +
278 'Did you mean to define a state property instead?',