Eslint-plugin-react: no-direct-mutation-state do not fail in constructor

[TrustNik]

Hello,
I guess following code should not be considered as a problem (now it is):

export default class SomeComponent extends Component {
   constructor (props) {
           super(props);
           this.state = {a : 1};
           this.state.b = 2;
   }
...

What do you think?

[ljharb]

Yes, that should warn.

[TrustNik]

@ljharb could you please explain why? If I understand correctly state is initialized in the constructor, so it can't be mutated.

[ljharb]

@TrustNik it doesn't cause the same problems as mutating this.state elsewhere - but mutation is never a good thing.

I can see the argument that the original spirit of this rule doesn't apply to "within the constructor", but allowing mutation isn't worth making an exception imo.

[TrustNik]

@ljharb I see. Thanks for clarification.

[ivosabev]

You should never call setState from the constructor as it brings a lot of function calls that you don't need for a component that hasn't rendered yet. It may even potentially cause double rendering.

The correct linting rule would be to remove the warning for this.state mutation and add a warning for using setState only when used inside the constructor.

[ljharb]

It is correct to warn about any mutation.

[ivosabev]

Except for setting state in the constructor. This behavior is against the official docs and some people think they should use the setState inside the constructor, when they shouldn't.

https://facebook.github.io/react/docs/state-and-lifecycle.html#do-not-modify-state-directly

[ljharb]

@ivosabev Assigning to this.state once in the constructor is perfectly fine. Mutating it immediately afterwards is not.

[burabure]

I have this rule Implemented here https://github.com/burabure/eslint-plugin-burabure/blob/master/lib/rules/no-direct-mutation-state.js @ljharb would you like this merged?

[ljharb]

@burabure if you have a pr with tests that can fix any broken behavior, that'd be great.

[ryndshn]

was this ever fixed? i'm still receiving a warning when modifying this.state in the constructor.

[ljharb]

@ryndshn assigning to it once is fine; modifying it afterwards is intentionally disallowed.

[d07RiV]

Why is it disallowed? I don't see any reason not to, and "never a good thing" is not a good explanation.

How do you modify state in constructor then, if it was initially assigned to in class properties?

[ljharb]

You don’t. You assign your entire state all at once, in one place. If you can’t do that in a class property, you shouldn’t have a “state” class property at all - do it all in the constructor.