fix: 🐛 use constructor() instead of UNSAFE_componentWillMount() #63
Conversation
|
This change should hopefully remove console warnings like below.
|
|
I am very interested on this PR too. @gaearon, can you see it, please? |
|
Hey y'all, thanks for the PR! I'm concerned that this change may create a bug regarding side effect ordering. The tests don't currently cover the behavior, but please refer to this discussion for more context. Could you please add some tests that cover nesting order to demonstrate that changing from componentWillMount to the constructor won't break anything? I know that's asking a lot more, but it would really help. |
| @@ -67,7 +67,8 @@ export default function withSideEffect( | |||
| return recordedState; | |||
| } | |||
|
|
|||
| UNSAFE_componentWillMount() { | |||
| constructor(props, context) { | |||
There was a problem hiding this comment.
Are you sure this should be in the constructor instead of componentDidMount? Ideally, constructors should not have side-effects
|
Thanks for the PR, but this doesn’t fix anything. It just silences the warning. Side effects in constructor are exactly as bad as in componentWillMount. |
Closes #62