Eslint-plugin-react: False positive no-access-state-in-setstate

Created on 13 Dec 2017  路  8Comments  路  Source: yannickcr/eslint-plugin-react

Rule no-access-state-in-setstate is false positive if this.state is used in callback (setState's second argument).

Example:

  handleChange = (value) => {
    this.setState(prevState => someLogicToMutateState(prevState, value), () => {
      this.props.onChange(this.state); // accessing this.state here is save but it's reported
    });
  };

React documentation says:

鈥se componentDidUpdate or a setState callback (setState(updater, callback)), either of which are guaranteed to fire after the update has been applied.

bug help wanted

Most helpful comment

@nbyarnell Nah, in your case it reports correctly. Your code is same as this:

  handleEvent = (thing) => {
    this.setState({
      "a": this.state.a.doSomething(),
      "b": this.state.b.doSomethingElse(thing)
    }, () => {
      $(element).show();
      $(otherElement).hide();
    });
  };

The correct version would be:

  handleEvent = (thing) => {
    this.setState(prevState => ({
        "a": prevState.a.doSomething(),
        "b": prevState.b.doSomethingElse(thing),
      }),
      () => {
        $(element).show();
        $(otherElement).hide();
      });
  };

All 8 comments

It's a bad antipattern to pass around the props or state objects; in addition to making it impossible for a linter to check on the usage of props or state, it also makes it harder to reason about the contents of those objects.

Separately, accessing this.state should definitely be allowed inside the callback argument to this.setState.

Yep, should be something like this.props.onChange({...this.state});. It's just example, not real code ;)

I'm also experiencing a false positive, but not even with this.state being used in a setState callback:

handleEvent = (thing) => {
  const {
      a,
      b
    } = this.state, // this line reports 'Use callback in setState when referencing the previous state.'
    c = a.doSomething(),
    d = b.doSomethingElse(thing);

  this.setState({
    "a": c,
    "b": d
  }, () => {
    $(element).show();
    $(otherElement).hide();
  });
};

@nbyarnell Nah, in your case it reports correctly. Your code is same as this:

  handleEvent = (thing) => {
    this.setState({
      "a": this.state.a.doSomething(),
      "b": this.state.b.doSomethingElse(thing)
    }, () => {
      $(element).show();
      $(otherElement).hide();
    });
  };

The correct version would be:

  handleEvent = (thing) => {
    this.setState(prevState => ({
        "a": prevState.a.doSomething(),
        "b": prevState.b.doSomethingElse(thing),
      }),
      () => {
        $(element).show();
        $(otherElement).hide();
      });
  };

Another great catch, thanks for reporting!
So many ways to set state, so many things to handle!

1614 Should fix it :)

edit: Wops, seems like I was late to the party. Consider #1610 istead of #1614 :)

Please check in v7.6.0 (and v7.6.1 when released, if you're using object spread) and we can reopen if it's still occurring.

1610 should fix this.

I seem to be experiencing this bug in v7.10.0.

this.setState(
    (prevState) => ({
        foo: prevState.foo.filter(...)
    }),
    () => {
        this.doThing(this.state.foo[0]) // linter unhappy here
    }
)

Edit: Hmm, maybe some sort of cache issue. After refactoring my code, the linter stopped complaining for some reason. Oh well!

Was this page helpful?
0 / 5 - 0 ratings