Rule proposal: no-access-state-in-setstate

[jaaberg]

This rule prevents the use of this.state within this.setState. It is further described in docs/rules/no-access-state-in-setstate.md in the PR, but to summarize: this.setState is async, and referring to this.state within it may lead to race conditions.

This is my first eslint-plugin PR, so I am really happy to get any kind of feedback.

edit:
This is also the recommended way doing it in the react docs:

If the next state depends on the previous state, we recommend using the updater function form, instead

this.setState((prevState) => {
 return {counter: prevState.quantity + 1};
}); 

[ljharb]

This is good advice when the semantic is "increment", but referencing this.state inside a setState call doesn't necessarily mean that the dev doesn't want s "snapshot".

What happens with:

const { value } = this.state;
this.setState({ value: value + 1 });

?

This is conceptually the same code, but it might be a useful way for the author to indicate that this is an intentional ordering.

[jaaberg]

Right now, I will report this as a warning. The same with

getstate() {
  return { value: this.state.counter++ };
}

increase() {
  this.setState(getState());
}

I could easily make it only report this.state directly inside this.setState, but I don't see why you would use this.state and not the prevState-solution which is guaranteed to be up-to-date.

Do you have an example when you actually would want to do that?

This is from the react docs:

If the next state depends on the previous state, we recommend using the updater function form, instead

this.setState((prevState) => {
 return {counter: prevState.quantity + 1};
}); 

@ljharb

[okonet]

OMG can this please be merged soon?

[FezVrasta]

I want this 🙏

[FezVrasta]

`this.state` maybe? (with code formatting)

[jaaberg]

Yes, I can probably do that for all this.state and setState mentions in the docs. Thanks, I will fix that first thing to the morning tomorrow.

[jaaberg]

For anyone that needs this rule, I've just released it as a separate plugin for eslint here since this hasn't been going anywhere for a few months.

I really hope it can get merged to this project some time, so if the maintainers has some questions, please reach out.

[ljharb]

I think the code is good, but i'm still not convinced this is an actual problem, despite React's docs' warning.

[fohrloop]

What do you guys think about the case, where this.state is saved in a temporary variable? For example, react/destructuring-assignment enforces that. Should/could no-access-state-in-setstate trigger in that case?

const { value } = this.state;
this.setState({ value: value + 1 });

[FezVrasta]

it should, because you are still referencing a possibly outdated value

[fohrloop]

I was thinking that, too. For some reason I could get it triggered only by using value: this.state.value +1 explicitly. Using eslint-plugin-react v. 7.5.1 & ESLint v.3.19.0.

[ljharb]

@np-8 can you file that as a new issue?

[fohrloop]

Sure, here you are: #1597