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-no-set-state-in-update-lifecycle #71

Closed
adidahiya opened this issue Feb 24, 2017 · 3 comments
Closed

New rule: react-no-set-state-in-update-lifecycle #71

adidahiya opened this issue Feb 24, 2017 · 3 comments

Comments

@adidahiya
Copy link
Contributor

adidahiya commented Feb 24, 2017

There are a couple places I'd like to ban this.setState in component lifecycle methods:

1. in componentWillUpdate

Doing this can lead to infinite recursion.

2. in componentDidUpdate

Same as eslint rule https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/no-did-update-set-state.md

3. in componentWillMount

componentWillMount() is invoked immediately before mounting occurs. It is called before render(), therefore setting state in this method will not trigger a re-rendering. Avoid introducing any side-effects or subscriptions in this method.

@adidahiya adidahiya changed the title react-no-set-state-in-update-hooks New Rule: react-no-set-state-in-update-hooks Feb 24, 2017
@adidahiya adidahiya changed the title New Rule: react-no-set-state-in-update-hooks New rule: react-no-set-state-in-update-hooks Feb 24, 2017
@adidahiya adidahiya changed the title New rule: react-no-set-state-in-update-hooks New rule: react-no-set-state-in-update-lifecycle Mar 4, 2017
@brsanthu
Copy link

@adidahiya Related to state, every component which has state, should override componentWillReceiveProps. Otherwise, component will work on old state, which would have initialized based on initial props. So is it possible to add a rule to detect that and warn? (should I create a new issue for this?)

@adidahiya
Copy link
Contributor Author

@brsanthu yeah, that should be filed as a separate issue.

It sounds like you're referring to this pattern?

constructor(props) {
    // initialize based on initial props
    this.state = { ... };
}

This seems to be the only situation where a missing componentWillRecieveProps implementation is likely wrong. Otherwise, you can happily use stateful components without implementing this lifecycle method.

AWare added a commit to guardian/dotcom-rendering that referenced this issue Sep 7, 2018
@adidahiya
Copy link
Contributor Author

Closing due to deprecation timeline, see #210

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

2 participants