Skip to content

Commit

Permalink
Merge pull request #1139 from amplitude/no-will-update-set-state
Browse files Browse the repository at this point in the history
[New] Add no-will-update-set-state rule
  • Loading branch information
ljharb committed Apr 9, 2017
2 parents 7111a70 + 4405523 commit 0ae6bb2
Show file tree
Hide file tree
Showing 8 changed files with 413 additions and 118 deletions.
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
61 changes: 2 additions & 59 deletions lib/rules/no-did-mount-set-state.js
Expand Up @@ -4,63 +4,6 @@
*/
'use strict';

// ------------------------------------------------------------------------------
// Rule Definition
// ------------------------------------------------------------------------------
var makeNoMethodSetStateRule = require('../util/makeNoMethodSetStateRule');

module.exports = {
meta: {
docs: {
description: 'Prevent usage of setState in componentDidMount',
category: 'Best Practices',
recommended: false
},

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 !== 'componentDidMount' ||
(mode !== 'disallow-in-func' && depth > 1)
) {
continue;
}
context.report({
node: callee,
message: 'Do not use setState in componentDidMount'
});
break;
}
}
};

}
};
module.exports = makeNoMethodSetStateRule('componentDidMount');
61 changes: 2 additions & 59 deletions lib/rules/no-did-update-set-state.js
Expand Up @@ -4,63 +4,6 @@
*/
'use strict';

// ------------------------------------------------------------------------------
// Rule Definition
// ------------------------------------------------------------------------------
var makeNoMethodSetStateRule = require('../util/makeNoMethodSetStateRule');

module.exports = {
meta: {
docs: {
description: 'Prevent usage of setState in componentDidUpdate',
category: 'Best Practices',
recommended: false
},

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 !== 'componentDidUpdate' ||
(mode !== 'disallow-in-func' && depth > 1)
) {
continue;
}
context.report({
node: callee,
message: 'Do not use setState in componentDidUpdate'
});
break;
}
}
};

}
};
module.exports = makeNoMethodSetStateRule('componentDidUpdate');
9 changes: 9 additions & 0 deletions lib/rules/no-will-update-set-state.js
@@ -0,0 +1,9 @@
/**
* @fileoverview Prevent usage of setState in componentWillUpdate
* @author Yannick Croissant
*/
'use strict';

var makeNoMethodSetStateRule = require('../util/makeNoMethodSetStateRule');

module.exports = makeNoMethodSetStateRule('componentWillUpdate');
70 changes: 70 additions & 0 deletions lib/util/makeNoMethodSetStateRule.js
@@ -0,0 +1,70 @@
/**
* @fileoverview Prevent usage of setState in lifecycle methods
* @author Yannick Croissant
*/
'use strict';

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

function makeNoMethodSetStateRule(methodName) {
return {
meta: {
docs: {
description: 'Prevent usage of setState in ' + methodName,
category: 'Best Practices',
recommended: false
},

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 !== methodName ||
(mode !== 'disallow-in-func' && depth > 1)
) {
continue;
}
context.report({
node: callee,
message: 'Do not use setState in ' + methodName
});
break;
}
}
};

}
};
}

module.exports = makeNoMethodSetStateRule;

0 comments on commit 0ae6bb2

Please sign in to comment.