Handle UpdateExpressions in no-direct-mutation-state #1387
Conversation
| code: [ | ||
| 'class Hello extends React.Component {', | ||
| ' constructor() {', | ||
| ' this.state.foo = 1;', |
There was a problem hiding this comment.
Could we also add a test that makes ++ invalid inside the constructor?
There was a problem hiding this comment.
Oh shoot, I must have missed adding that line checking another thing… Sorry.
I had this.state.foo++ right here as a valid case - we're in the constructor and while I think it's awkward, it's legal.
There was a problem hiding this comment.
I don't see why it should be legal; it's mutating this.state after it's assigned.
There was a problem hiding this comment.
Assignment doesn't actually matter until we're out of the constructor method. That's when React takes over.
But that's also a tangential change to what I wanted to make here - I'm bringing ++ to parity with = (you have a test case immediately above this one showing that). If you want to change how you treat assignment as well, that's fine but I think you probably want to do that distinctly.
There was a problem hiding this comment.
This has been my favorite find in our codebase where we were assigning to state.
this.setState({
foo: this.state.foo++
});
There was a problem hiding this comment.
@TheSavior (cc: @zpao )
I actually have a PR proposing a rule catching all references to this.state used in at this.setState in #1374. Does this rule cover that as well? If not, I would be really happy for feedback.
This rule should also catch increment & decrement operators.
zpao
force-pushed
the
update-expression
branch
from
fb6b216
to
aafafc4
Compare
|
Do you need anything else from me here? |
This rule should also catch increment & decrement operators.
Fixes #1386