chore(AutoControlledComponent): Remove unsafe lifecycles #3245
Conversation
Remove unsafe lifecycles from `AutoControlledComponent` and affected components: - `componentWillMount` - `componentWillRecieveProps` - `componentWillUpdate` Relates to Semantic-Org#2732
Codecov Report
@@ Coverage Diff @@
## master #3245 +/- ##
==========================================
+ Coverage 99.92% 99.96% +0.03%
==========================================
Files 169 169
Lines 2802 2802
==========================================
+ Hits 2800 2801 +1
+ Misses 2 1 -1
Continue to review full report at Codecov.
|
|
Many changes where introduced by the formater in defined in the hooks. Sorry for the extra work. This PR cannot be breaked further as inherited components are affected |
| 'State will not be set.', | ||
| 'Only props in static autoControlledProps will be set on state.', | ||
| ].join(' '), | ||
| ) |
There was a problem hiding this comment.
We cannot simply rename componentWillReceiveProps() to componentDidUpdate() there because we will have an additional render() after setState(), just compare
componentWillReceiveProps()
render()
// vs
render()
componentDidUpdate
render()
componentDidUpdate
Example: https://codesandbox.io/s/4x35y87rl4
The right way is to use there getDerivedStateFromProps(). This will be more complicated refactor :(
There was a problem hiding this comment.
I've left a comment about the changes, we need to make more work there.
Thank you for the start 👍
However, we can safely make changes in MountNode for example: https://github.com/Semantic-Org/Semantic-UI-React/blob/v0.83.0/src/addons/MountNode/MountNode.js#L38
|
Sure, I'll try to work on define the same logic with
|
We can update |
|
Hey @kohakukun I recently merged #3303, it has a very cool simplification of |
|
Sorry for the slow reply |
Remove unsafe lifecycles from
AutoControlledComponentand affectedcomponents:
componentWillMountcomponentWillRecievePropscomponentWillUpdateRelates to #2732