Skip to content

Commit 59ac136

Browse files
committed
Add no-did-mount-set-state and no-did-update-set-state rules
1 parent 7fef61b commit 59ac136

File tree

8 files changed

+278
-3
lines changed

8 files changed

+278
-3
lines changed

README.md

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,9 @@ Finally, enable all of the rules that you would like to use.
4646
"react/prop-types": 1,
4747
"react/display-name": 1,
4848
"react/wrap-multilines": 1,
49-
"react/self-closing-comp": 1
49+
"react/self-closing-comp": 1,
50+
"react/no-did-mount-set-state": 1,
51+
"react/no-did-update-set-state": 1
5052
}
5153
}
5254
```
@@ -58,13 +60,16 @@ Finally, enable all of the rules that you would like to use.
5860
* [display-name](docs/rules/display-name.md): Prevent missing displayName in a React component definition
5961
* [wrap-multilines](docs/rules/wrap-multilines.md): Prevent missing parentheses around multilines JSX
6062
* [self-closing-comp](docs/rules/self-closing-comp.md): Prevent extra closing tags for components without children
63+
* [no-did-mount-set-state](docs/rules/no-did-mount-set-state.md): Prevent usage of setState in componentDidMount
64+
* [no-did-update-set-state](docs/rules/no-did-update-set-state.md): Prevent usage of setState in componentDidUpdate
6165

6266
## To Do
6367

6468
* no-deprecated: Prevent usage of deprecated methods ([React 0.12 Updated API](http://facebook.github.io/react/blog/2014/10/28/react-v0.12.html#new-terminology-amp-updated-apis))
6569
* no-classic: Prevent usage of "classic" methods ([#2700](https://github.com/facebook/react/pull/2700))
6670
* [Implement relevant rules from David Chang's React Style Guide](https://reactjsnews.com/react-style-guide-patterns-i-like)
6771
* [Implement relevant rules from John Cobb's best practices and conventions](http://web-design-weekly.com/2015/01/29/opinionated-guide-react-js-best-practices-conventions/)
72+
* [Implement relevant rules from Alexander Early's tips and best practices](http://aeflash.com/2015-02/react-tips-and-best-practices.html)
6873

6974
[Any rule idea is welcome !](https://github.com/yannickcr/eslint-plugin-react/issues)
7075

docs/rules/no-did-mount-set-state.md

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
# Prevent usage of setState in componentDidMount (no-did-mount-set-state)
2+
3+
Updating the state after a component mount will trigger a second `render()` call and can lead to property/layout thrashing.
4+
5+
## Rule Details
6+
7+
The following patterns are considered warnings:
8+
9+
```js
10+
var Hello = React.createClass({
11+
componentDidMount: function() {
12+
this.setState({
13+
name: this.props.name.toUpperCase()
14+
});
15+
},
16+
render: function() {
17+
return <div>Hello {this.state.name}</div>;
18+
}
19+
});
20+
```
21+
22+
The following patterns are not considered warnings:
23+
24+
```js
25+
var Hello = React.createClass({
26+
componentDidMount: function() {
27+
this.props.onMount();
28+
},
29+
render: function() {
30+
return <div>Hello {this.props.name}</div>;
31+
}
32+
});
33+
```

docs/rules/no-did-update-set-state.md

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
# Prevent usage of setState in componentDidMount (no-did-update-set-state)
2+
3+
Updating the state after a component update will trigger a second `render()` call and can lead to property/layout thrashing.
4+
5+
## Rule Details
6+
7+
The following patterns are considered warnings:
8+
9+
```js
10+
var Hello = React.createClass({
11+
componentDidUpdate: function() {
12+
this.setState({
13+
name: this.props.name.toUpperCase()
14+
});
15+
},
16+
render: function() {
17+
return <div>Hello {this.state.name}</div>;
18+
}
19+
});
20+
```
21+
22+
The following patterns are not considered warnings:
23+
24+
```js
25+
var Hello = React.createClass({
26+
componentDidUpdate: function() {
27+
this.props.onUpdate();
28+
},
29+
render: function() {
30+
return <div>Hello {this.props.name}</div>;
31+
}
32+
});
33+
```

index.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,17 @@ module.exports = {
66
'prop-types': require('./lib/rules/prop-types'),
77
'display-name': require('./lib/rules/display-name'),
88
'wrap-multilines': require('./lib/rules/wrap-multilines'),
9-
'self-closing-comp': require('./lib/rules/self-closing-comp')
9+
'self-closing-comp': require('./lib/rules/self-closing-comp'),
10+
'no-did-mount-set-state': require('./lib/rules/no-did-mount-set-state'),
11+
'no-did-update-set-state': require('./lib/rules/no-did-update-set-state')
1012
},
1113
rulesConfig: {
1214
'no-multi-comp': 0,
1315
'prop-types': 0,
1416
'display-name': 0,
1517
'wrap-multilines': 0,
16-
'self-closing-comp': 0
18+
'self-closing-comp': 0,
19+
'no-did-mount-set-state': 0,
20+
'no-did-update-set-state': 0
1721
}
1822
};

lib/rules/no-did-mount-set-state.js

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/**
2+
* @fileoverview Prevent usage of setState in componentDidMount
3+
* @author Yannick Croissant
4+
*/
5+
'use strict';
6+
7+
// ------------------------------------------------------------------------------
8+
// Rule Definition
9+
// ------------------------------------------------------------------------------
10+
11+
module.exports = function(context) {
12+
13+
// --------------------------------------------------------------------------
14+
// Public
15+
// --------------------------------------------------------------------------
16+
17+
return {
18+
19+
'MemberExpression': function(node) {
20+
if (node.object.type !== 'ThisExpression' || node.property.name !== 'setState') {
21+
return;
22+
}
23+
var ancestors = context.getAncestors(node);
24+
for (var i = 0, j = ancestors.length; i < j; i++) {
25+
if (ancestors[i].type !== 'Property' || ancestors[i].key.name !== 'componentDidMount') {
26+
continue;
27+
}
28+
context.report(node, 'Do not use setState in componentDidMount');
29+
}
30+
}
31+
};
32+
33+
};

lib/rules/no-did-update-set-state.js

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/**
2+
* @fileoverview Prevent usage of setState in componentDidUpdate
3+
* @author Yannick Croissant
4+
*/
5+
'use strict';
6+
7+
// ------------------------------------------------------------------------------
8+
// Rule Definition
9+
// ------------------------------------------------------------------------------
10+
11+
module.exports = function(context) {
12+
13+
// --------------------------------------------------------------------------
14+
// Public
15+
// --------------------------------------------------------------------------
16+
17+
return {
18+
19+
'MemberExpression': function(node) {
20+
if (node.object.type !== 'ThisExpression' || node.property.name !== 'setState') {
21+
return;
22+
}
23+
var ancestors = context.getAncestors(node);
24+
for (var i = 0, j = ancestors.length; i < j; i++) {
25+
if (ancestors[i].type !== 'Property' || ancestors[i].key.name !== 'componentDidUpdate') {
26+
continue;
27+
}
28+
context.report(node, 'Do not use setState in componentDidUpdate');
29+
}
30+
}
31+
};
32+
33+
};
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/**
2+
* @fileoverview Prevent usage of setState in componentDidMount
3+
* @author Yannick Croissant
4+
*/
5+
'use strict';
6+
7+
// ------------------------------------------------------------------------------
8+
// Requirements
9+
// ------------------------------------------------------------------------------
10+
11+
var eslint = require('eslint').linter;
12+
var ESLintTester = require('eslint-tester');
13+
14+
// ------------------------------------------------------------------------------
15+
// Tests
16+
// ------------------------------------------------------------------------------
17+
18+
var eslintTester = new ESLintTester(eslint);
19+
eslintTester.addRuleTest('lib/rules/no-did-mount-set-state', {
20+
21+
valid: [
22+
{
23+
code: '\
24+
var Hello = React.createClass({\
25+
render: function() {\
26+
return <div>Hello {this.props.name}</div>;\
27+
}\
28+
});',
29+
ecmaFeatures: {
30+
jsx: true
31+
}
32+
}, {
33+
code: '\
34+
var Hello = React.createClass({\
35+
componentDidMount: function() {},\
36+
render: function() {\
37+
return <div>Hello {this.props.name}</div>;\
38+
}\
39+
});',
40+
ecmaFeatures: {
41+
jsx: true
42+
}
43+
}
44+
],
45+
46+
invalid: [
47+
{
48+
code: '\
49+
var Hello = React.createClass({\
50+
componentDidMount: function() {\
51+
this.setState({\
52+
name: this.props.name.toUpperCase()\
53+
});\
54+
},\
55+
render: function() {\
56+
return <div>Hello {this.state.name}</div>;\
57+
}\
58+
});',
59+
ecmaFeatures: {
60+
jsx: true
61+
},
62+
errors: [{
63+
message: 'Do not use setState in componentDidMount'
64+
}]
65+
}
66+
]
67+
});
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/**
2+
* @fileoverview Prevent usage of setState in componentDidUpdate
3+
* @author Yannick Croissant
4+
*/
5+
'use strict';
6+
7+
// ------------------------------------------------------------------------------
8+
// Requirements
9+
// ------------------------------------------------------------------------------
10+
11+
var eslint = require('eslint').linter;
12+
var ESLintTester = require('eslint-tester');
13+
14+
// ------------------------------------------------------------------------------
15+
// Tests
16+
// ------------------------------------------------------------------------------
17+
18+
var eslintTester = new ESLintTester(eslint);
19+
eslintTester.addRuleTest('lib/rules/no-did-update-set-state', {
20+
21+
valid: [
22+
{
23+
code: '\
24+
var Hello = React.createClass({\
25+
render: function() {\
26+
return <div>Hello {this.props.name}</div>;\
27+
}\
28+
});',
29+
ecmaFeatures: {
30+
jsx: true
31+
}
32+
}, {
33+
code: '\
34+
var Hello = React.createClass({\
35+
componentDidUpdate: function() {},\
36+
render: function() {\
37+
return <div>Hello {this.props.name}</div>;\
38+
}\
39+
});',
40+
ecmaFeatures: {
41+
jsx: true
42+
}
43+
}
44+
],
45+
46+
invalid: [
47+
{
48+
code: '\
49+
var Hello = React.createClass({\
50+
componentDidUpdate: function() {\
51+
this.setState({\
52+
name: this.props.name.toUpperCase()\
53+
});\
54+
},\
55+
render: function() {\
56+
return <div>Hello {this.state.name}</div>;\
57+
}\
58+
});',
59+
ecmaFeatures: {
60+
jsx: true
61+
},
62+
errors: [{
63+
message: 'Do not use setState in componentDidUpdate'
64+
}]
65+
}
66+
]
67+
});

0 commit comments

Comments
 (0)