From 5518bd44a9bdd573ec33f9f2af9d91ea90dedc90 Mon Sep 17 00:00:00 2001 From: Abhay Nikam Date: Fri, 21 Apr 2017 20:16:19 +0530 Subject: [PATCH] Warning added if defaultProps were defined as an instance property (#9433) * Added warning when defaultProps was defined as an instance property * Added testcases to check warning message for defaultProps * Update fiber tests --- scripts/fiber/tests-passing.txt | 1 + .../__tests__/ReactCompositeComponent-test.js | 23 +++++++++++++++++++ .../shared/fiber/ReactFiberClassComponent.js | 7 ++++++ .../reconciler/ReactCompositeComponent.js | 6 +++++ 4 files changed, 37 insertions(+) diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 21ec8fa92a..80f88c729f 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -461,6 +461,7 @@ src/renderers/__tests__/ReactCompositeComponent-test.js * should call componentWillUnmount before unmounting * should warn when shouldComponentUpdate() returns undefined * should warn when componentDidUnmount method is defined +* should warn when defaultProps was defined as an instance property * should pass context to children when not owner * should skip update when rerendering element in container * should pass context when re-rendered for static child diff --git a/src/renderers/__tests__/ReactCompositeComponent-test.js b/src/renderers/__tests__/ReactCompositeComponent-test.js index 520319ee28..f266130887 100644 --- a/src/renderers/__tests__/ReactCompositeComponent-test.js +++ b/src/renderers/__tests__/ReactCompositeComponent-test.js @@ -488,6 +488,29 @@ describe('ReactCompositeComponent', () => { ); }); + it('should warn when defaultProps was defined as an instance property', () => { + spyOn(console, 'error'); + + class Component extends React.Component { + constructor(props) { + super(props); + this.defaultProps = {name: 'Abhay'}; + } + + render() { + return
; + } + } + + ReactTestUtils.renderIntoDocument(); + + expectDev(console.error.calls.count()).toBe(1); + expectDev(console.error.calls.argsFor(0)[0]).toBe( + 'Warning: defaultProps was defined as an instance property ' + + 'on Component. Use a static property to define defaultProps instead.', + ); + }); + it('should pass context to children when not owner', () => { class Parent extends React.Component { render() { diff --git a/src/renderers/shared/fiber/ReactFiberClassComponent.js b/src/renderers/shared/fiber/ReactFiberClassComponent.js index 617aab1fdb..24a44e94c3 100644 --- a/src/renderers/shared/fiber/ReactFiberClassComponent.js +++ b/src/renderers/shared/fiber/ReactFiberClassComponent.js @@ -228,6 +228,13 @@ module.exports = function( name, name, ); + const noInstanceDefaultProps = !instance.defaultProps; + warning( + noInstanceDefaultProps, + 'defaultProps was defined as an instance property on %s. Use a static ' + + 'property to define defaultProps instead.', + name, + ); } const state = instance.state; diff --git a/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js b/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js index 096b5240a0..c58a8fcedb 100644 --- a/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js +++ b/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js @@ -299,6 +299,12 @@ var ReactCompositeComponent = { 'componentWillRecieveProps(). Did you mean componentWillReceiveProps()?', this.getName() || 'A component', ); + warning( + !inst.defaultProps, + 'defaultProps was defined as an instance property on %s. Use a static ' + + 'property to define defaultProps instead.', + this.getName() || 'a component', + ); } var initialState = inst.state;