Warning message updated if defaultProps were defined as an instance property #9493
Conversation
abhaynikam
commented
Apr 22, 2017
abhaynikam
commented
- Updated the warning message as per @gaearon asked for in PR Warning added if defaultProps were defined as an instance property #9433
- Updated the warning message in the testcases as well.
- Issue related to Warn in dev if shouldComponentUpdate is defined on PureComponent #9239
|
Can you change this to say:
|
|
@gaearon : Thanks for the comment. I have update the warning message. Please have a look |
| @@ -504,8 +504,8 @@ describe('ReactCompositeComponent', () => { | |||
|
|
|||
| 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.', | |||
| 'Warning: setting defaultProps as an instance property on Component is not supported ' + | |||
There was a problem hiding this comment.
Not that it matters too much, but for future reference, we start warnings with a capital letter. It should be Setting defaultProps [...]. We don't consider Warning: to be part of the sentence.
There was a problem hiding this comment.
Thanks for clarifying! Your comment above had it as lowercase so I thought it was fine, but I'll make note of this from now on.
There was a problem hiding this comment.
Yea, I only really noticed it in the code.
I thought it's worth changing as it might come up in custom warning dialog (I don't remember if it appends Warning: or not).
