palantir / tslint-react

:orange_book: Lint rules related to React & JSX for TSLint.
Apache License 2.0
749 stars 76 forks source link

New rule: react-set-state-with-function #80

Closed adidahiya closed 5 years ago

adidahiya commented 7 years ago

Enforce usage of the callback form of this.setState

// bad
this.setState({ ... });
// good
this.setState((prevState, props) => ...);
karfau commented 7 years ago

From reading these docs the first approach it fine, as long as there are no occurences of this.state or this.props inside of it, right? Would you still enforce the second approach to make sure code is more consistent, even though it is more typing all the time?

vhfmag commented 7 years ago

I don't know much about tslint and tsutils API, so that may be not feasible, but I think the ideal would be that the rule allows objects if props and state aren't used and enforce callback usage otherwise. Does that sound feasible?

benbankes commented 6 years ago

It seems like disallowing this.setState({...}); might be a little too restrictive. It seems like a better approach might be #172

karfau commented 6 years ago

From what I understand, the rule suggested in #172 is more flexible and covers more cases then this one. They have some overlap though and I can imagine a team deciding for this rule that might be simpler to implement and thus be more performent. So not sure if those two are duplicates or not. Maybe the behavior suggested here could be provided by an option in the other rule to stop ading overlapping rules that might be confusing to choose from and configure.

adidahiya commented 5 years ago

closing in favor of #172