Add no-will-update-set-state rule #1139
Conversation
| docs: { | ||
| description: 'Prevent usage of setState in componentWillUpdate', | ||
| category: 'Best Practices', | ||
| recommended: false |
There was a problem hiding this comment.
Maybe this should actually be in the recommended rules because there isn't a valid use-case?
There was a problem hiding this comment.
Adding things to the recommended rules is a breaking change, so it should be in a separate PR regardless.
| @@ -0,0 +1,66 @@ | |||
| /** | |||
| * @fileoverview Prevent usage of setState in componentWillUpdate | |||
| * @author Yannick Croissant | |||
There was a problem hiding this comment.
Didn't seem right changing the author when I didn't write any of the code.
|
@ljharb Thanks for the feedback, see the new commits for my attempt to factor out the duplicate code. |
|
I'm not sure I completely agree with that. To me there is a distinction between the 2 existing rules and the rule I'm adding. The existing rules are something that is normally a bad pattern but can work (and in the rarest of circumstances might even be correct) whereas the new rule catches code that should never exist and is likely buggy. So putting them all under the same rule with an argument might give the wrong impression about the varying levels of strictness with which the rules should be applied. Perhaps the distinction between between the rules isn't as great as I'm interpreting or the reduction in code is worthwhile enough to do this regardless though. I'm open to being swayed but right now I think I'd advocate to keep them separate. |
|
That seems like a reasonable argument. I'm good with just adding this rule. |
|
Awesome. Thanks very much for the reviews and feedback. |
Basically just a copy and paste of the
no-did-update-set-staterule forcomponentWillUpdate. This rule would have saved me a fair bit of time tracking down an issue where this pattern had been used by accident so I thought it was worthwhile adding support for.