Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
adidahiya opened this issue Mar 29, 2017 · 5 comments
Closed

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

adidahiya opened this issue Mar 29, 2017 · 5 comments

Comments

@adidahiya
Copy link
Contributor

Enforce usage of the callback form of this.setState

// bad
this.setState({ ... });
// good
this.setState((prevState, props) => ...);
@karfau
Copy link

karfau commented Apr 4, 2017

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
Copy link

vhfmag commented May 5, 2017

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
Copy link

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

@karfau
Copy link

karfau commented Aug 12, 2018

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
Copy link
Contributor Author

closing in favor of #172

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants