Eslint-plugin-react: "react/no-did-mount-set-state", not considered an valid rule since React 16.3.0

Created on 1 Apr 2018  ·  17Comments  ·  Source: yannickcr/eslint-plugin-react

Not valid rule on latest React -release, should be removed.

help wanted react 16

Most helpful comment

Yes I do :+1:

All 17 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 componentDidMount - which is NOT deprecated - would be able to have a setState call in it.

Since componentWillMount will disappear the state change done needs to go somewhere else. For this case, users can almost always use getDerivedStateFromProps to achieve this, but there will be some other cases where users need to call setState in componentDidMount or componentDidUpdate.

From the blogpost:

{...} In general, it is better to avoid cascading updates like this, but in some cases they are necessary (for example, if you need to position a tooltip after measuring the rendered DOM element).

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 setState in componentDidMount, except if you consider extra render as performance issue.

An extra render is definitely a performance issue; but an extra render with different state causes a user-visible flash.

an extra render with different state causes a user-visible flash.

This is inaccurate. According to "Update on Async Rendering" on React's blog:

Sometimes people use componentWillUpdate out of a misplaced fear that by the time componentDidUpdate fires, it is “too late” to update the state of other components. This is not the case. React ensures that any setState calls that happen during componentDidMount and componentDidUpdate are flushed before the user sees the updated UI. In general, it is better to avoid cascading updates like this, but in some cases they are necessary (for example, if you need to position a tooltip after measuring the rendered DOM element).

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 :+1:

Seems like the actionable thing to do here is, when the React version pragma is >= v16.3, these rules should no-op.

In general, it is better to avoid cascading updates like this, but in some cases they are necessary (for example, if you need to position a tooltip after measuring the rendered DOM element).

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 https://github.com/yannickcr/eslint-plugin-react/issues/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.

Seems like the actionable thing to do here is, when the React version pragma is >= v16.3, these rules should no-op.

@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 settings.react.version to 16.8 in .eslintrc.json.

No, it hasn't yet.

hi
any update on this?
thanks

Was this page helpful?
0 / 5 - 0 ratings