gaearon / react-side-effect

Create components whose nested prop changes map to a global side effect
MIT License
1.22k stars 75 forks source link

fix: 🐛 use constructor() instead of UNSAFE_componentWillMount() #63

Closed streamich closed 4 years ago

streamich commented 4 years ago

Closes https://github.com/gaearon/react-side-effect/issues/62

streamich commented 4 years ago

This change should hopefully remove console warnings like below.

image

andreyluiz commented 4 years ago

I am very interested on this PR too. @gaearon, can you see it, please?

lourd commented 4 years ago

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.

gaearon commented 4 years ago

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.