New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
"react/no-did-mount-set-state", not considered an valid rule since React 16.3.0 #1754
Comments
Can you elaborate? |
Sorry for not been to clear. "componentWillMount", "componentWillUpdate", "componentWillReceiveProps" will be deprecated life-cycle methods (React 16.3.0, introduced these and deprecation warnings are scheduled to come with 16.4.0), and replacements for those will be accordingly "componentWillMount" => "componentDidMount", "componentWillUpdate" => "componentDidUpdate" and "componentWillReceiveProps" => with new introduced "getDerivedStateFromProps". Therefore it's not relevant to have this rule and existing code bases should be migrated soon enough to match new life-cycles. https://medium.com/@baphemot/whats-new-in-react-16-3-d2c9b7b6193b here's short Medium posting according to changes. |
I'm aware of those deprecations, but I'm not sure why it suddenly means that |
Since From the blogpost:
Whole post with more examples and best practices: https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html |
In my opinion most relevant use cases for this, would be async rendering or if you would like to meddle with refs. I don't see what is issue having |
An extra render is definitely a performance issue; but an extra render with different state causes a user-visible flash. |
This is inaccurate. According to "Update on Async Rendering" on React's blog:
|
I believe this applies to both didMount and didUpdate |
@kentcdodds thanks for the clarification. do you then agree with the OP, that as of v16.3, these two rules no longer have value? |
Yes I do 👍 |
Seems like the actionable thing to do here is, when the React version pragma is >= v16.3, these rules should no-op. |
Mmm, it is exactly for this reason that this rule was created in the first place (but was removed from the recommended configuration since there is some legitimate use cases, see #596). |
Why is this rule not removed? It confused me, but it's perfectly valid for tooltips. |
@steida people still use Reacts older than 16.3. |
We use React >= 16.3, and everywhere we use setState in componentDidUpdate, we do that inside an if where we check against previous values. So the rule does not really apply to us. See e.g. the discussion here: jsx-eslint/eslint-plugin-react#1754
We use React >= 16.3, and everywhere we use setState in componentDidUpdate, we do that inside an if where we check against previous values. So the rule does not really apply to us. See e.g. the discussion here: jsx-eslint/eslint-plugin-react#1754
@yannickcr, @ljharb, has this no-op behavior been implemented? I'm having trouble with ESLint still complaining about this rule, even if I explicitly set |
No, it hasn't yet. |
This comment was marked as spam.
This comment was marked as spam.
… react >= 16.3 Fixes jsx-eslint#1754
Not valid rule on latest React -release, should be removed.
The text was updated successfully, but these errors were encountered: