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

Add no-will-update-set-state rule #1139

Merged
merged 3 commits into from Apr 9, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Expand Up @@ -102,6 +102,7 @@ Finally, enable all of the rules that you would like to use. Use [our preset](#
* [react/no-unescaped-entities](docs/rules/no-unescaped-entities.md): Prevent invalid characters from appearing in markup
* [react/no-unknown-property](docs/rules/no-unknown-property.md): Prevent usage of unknown DOM property (fixable)
* [react/no-unused-prop-types](docs/rules/no-unused-prop-types.md): Prevent definitions of unused prop types
* [react/no-will-update-set-state](docs/rules/no-will-update-set-state.md): Prevent usage of `setState` in `componentWillUpdate`
* [react/prefer-es6-class](docs/rules/prefer-es6-class.md): Enforce ES5 or ES6 class for React Components
* [react/prefer-stateless-function](docs/rules/prefer-stateless-function.md): Enforce stateless React Components to be written as a pure function
* [react/prop-types](docs/rules/prop-types.md): Prevent missing props validation in a React component definition
Expand Down
90 changes: 90 additions & 0 deletions docs/rules/no-will-update-set-state.md
@@ -0,0 +1,90 @@
# Prevent usage of setState in componentWillUpdate (no-will-update-set-state)

Updating the state during the componentWillUpdate step can lead to indeterminate component state and is not allowed.

## Rule Details

The following patterns are considered warnings:

```jsx
var Hello = React.createClass({
componentWillUpdate: function() {
this.setState({
name: this.props.name.toUpperCase()
});
},
render: function() {
return <div>Hello {this.state.name}</div>;
}
});
```

The following patterns are not considered warnings:

```jsx
var Hello = React.createClass({
componentWillUpdate: function() {
this.props.prepareHandler();
},
render: function() {
return <div>Hello {this.props.name}</div>;
}
});
```

```jsx
var Hello = React.createClass({
componentWillUpdate: function() {
this.prepareHandler(function callback(newName) {
this.setState({
name: newName
});
});
},
render: function() {
return <div>Hello {this.props.name}</div>;
}
});
```

## Rule Options

```js
...
"no-will-update-set-state": [<enabled>, <mode>]
...
```

### `disallow-in-func` mode

By default this rule forbids any call to `this.setState` in `componentWillUpdate` outside of functions. The `disallow-in-func` mode makes this rule more strict by disallowing calls to `this.setState` even within functions.

The following patterns are considered warnings:

```jsx
var Hello = React.createClass({
componentDidUpdate: function() {
this.setState({
name: this.props.name.toUpperCase()
});
},
render: function() {
return <div>Hello {this.state.name}</div>;
}
});
```

```jsx
var Hello = React.createClass({
componentDidUpdate: function() {
this.prepareHandler(function callback(newName) {
this.setState({
name: newName
});
});
},
render: function() {
return <div>Hello {this.state.name}</div>;
}
});
```
1 change: 1 addition & 0 deletions index.js
Expand Up @@ -20,6 +20,7 @@ var allRules = {
'no-did-update-set-state': require('./lib/rules/no-did-update-set-state'),
'no-render-return-value': require('./lib/rules/no-render-return-value'),
'no-unescaped-entities': require('./lib/rules/no-unescaped-entities'),
'no-will-update-set-state': require('./lib/rules/no-will-update-set-state'),
'react-in-jsx-scope': require('./lib/rules/react-in-jsx-scope'),
'jsx-uses-vars': require('./lib/rules/jsx-uses-vars'),
'jsx-handler-names': require('./lib/rules/jsx-handler-names'),
Expand Down
66 changes: 66 additions & 0 deletions lib/rules/no-will-update-set-state.js
@@ -0,0 +1,66 @@
/**
* @fileoverview Prevent usage of setState in componentWillUpdate
* @author Yannick Croissant
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't seem right changing the author when I didn't write any of the code.

*/
'use strict';

// ------------------------------------------------------------------------------
// Rule Definition
// ------------------------------------------------------------------------------

module.exports = {
meta: {
docs: {
description: 'Prevent usage of setState in componentWillUpdate',
category: 'Best Practices',
recommended: false
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this should actually be in the recommended rules because there isn't a valid use-case?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding things to the recommended rules is a breaking change, so it should be in a separate PR regardless.

},

schema: [{
enum: ['disallow-in-func']
}]
},

create: function(context) {

var mode = context.options[0] || 'allow-in-func';

// --------------------------------------------------------------------------
// Public
// --------------------------------------------------------------------------

return {

CallExpression: function(node) {
var callee = node.callee;
if (
callee.type !== 'MemberExpression' ||
callee.object.type !== 'ThisExpression' ||
callee.property.name !== 'setState'
) {
return;
}
var ancestors = context.getAncestors(callee).reverse();
var depth = 0;
for (var i = 0, j = ancestors.length; i < j; i++) {
if (/Function(Expression|Declaration)$/.test(ancestors[i].type)) {
depth++;
}
if (
(ancestors[i].type !== 'Property' && ancestors[i].type !== 'MethodDefinition') ||
ancestors[i].key.name !== 'componentWillUpdate' ||
(mode !== 'disallow-in-func' && depth > 1)
) {
continue;
}
context.report({
node: callee,
message: 'Do not use setState in componentWillUpdate'
});
break;
}
}
};

}
};