Skip to content

Commit 70fef71

Browse files
yannickcryannickcr
committed
Add allow-in-func option to no-did-mount-set-state (fixes #56)
1 parent bb96d3c commit 70fef71

File tree

3 files changed

+190
-22
lines changed

3 files changed

+190
-22
lines changed

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

Lines changed: 64 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,32 @@ Updating the state after a component mount will trigger a second `render()` call
44

55
## Rule Details
66

7+
This rule is aimed to forbids the use of `this.setState` in `componentDidMount`.
8+
79
The following patterns are considered warnings:
810

911
```js
1012
var Hello = React.createClass({
1113
componentDidMount: function() {
12-
this.setState({
13-
name: this.props.name.toUpperCase()
14+
this.setState({
15+
name: this.props.name.toUpperCase()
16+
});
17+
},
18+
render: function() {
19+
return <div>Hello {this.state.name}</div>;
20+
}
21+
});
22+
```
23+
24+
```js
25+
var Hello = React.createClass({
26+
componentDidMount: function() {
27+
this.onMount(function callback(newName) {
28+
this.setState({
29+
name: newName
1430
});
15-
},
31+
});
32+
},
1633
render: function() {
1734
return <div>Hello {this.state.name}</div>;
1835
}
@@ -31,3 +48,47 @@ var Hello = React.createClass({
3148
}
3249
});
3350
```
51+
52+
## Rule Options
53+
54+
```js
55+
...
56+
"no-did-mount-set-state": [<enabled>, <mode>]
57+
...
58+
```
59+
60+
### `allow-in-func` mode
61+
62+
By default this rule forbids any call to `this.setState` in `componentDidMount`. But since `componentDidMount` is a common place to set some event listeners, you may end up with calls to `this.setState` in some callbacks. The `allow-in-func` mode allows you to use `this.setState` in `componentDidMount` as long as they are called within a function.
63+
64+
The following patterns are considered warnings:
65+
66+
```js
67+
var Hello = React.createClass({
68+
componentDidMount: function() {
69+
this.setState({
70+
name: this.props.name.toUpperCase()
71+
});
72+
},
73+
render: function() {
74+
return <div>Hello {this.state.name}</div>;
75+
}
76+
});
77+
```
78+
79+
The following patterns are not considered warnings:
80+
81+
```js
82+
var Hello = React.createClass({
83+
componentDidMount: function() {
84+
this.onMount(function callback(newName) {
85+
this.setState({
86+
name: newName
87+
});
88+
});
89+
},
90+
render: function() {
91+
return <div>Hello {this.state.name}</div>;
92+
}
93+
});
94+
```

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

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010

1111
module.exports = function(context) {
1212

13+
var mode = context.options[0] || 'never';
14+
1315
// --------------------------------------------------------------------------
1416
// Public
1517
// --------------------------------------------------------------------------
@@ -18,18 +20,28 @@ module.exports = function(context) {
1820

1921
CallExpression: function(node) {
2022
var callee = node.callee;
21-
if (callee.type !== 'MemberExpression') {
22-
return;
23-
}
24-
if (callee.object.type !== 'ThisExpression' || callee.property.name !== 'setState') {
23+
if (
24+
callee.type !== 'MemberExpression' ||
25+
callee.object.type !== 'ThisExpression' ||
26+
callee.property.name !== 'setState'
27+
) {
2528
return;
2629
}
27-
var ancestors = context.getAncestors(callee);
30+
var ancestors = context.getAncestors(callee).reverse();
31+
var depth = 0;
2832
for (var i = 0, j = ancestors.length; i < j; i++) {
29-
if (ancestors[i].type !== 'Property' || ancestors[i].key.name !== 'componentDidMount') {
33+
if (/Function(Expression|Declaration)$/.test(ancestors[i].type)) {
34+
depth++;
35+
}
36+
if (
37+
ancestors[i].type !== 'Property' ||
38+
ancestors[i].key.name !== 'componentDidMount' ||
39+
(mode === 'allow-in-func' && depth > 1)
40+
) {
3041
continue;
3142
}
3243
context.report(callee, 'Do not use setState in componentDidMount');
44+
break;
3345
}
3446
}
3547
};

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

Lines changed: 108 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111
var eslint = require('eslint').linter;
1212
var ESLintTester = require('eslint-tester');
1313

14+
require('babel-eslint');
15+
1416
// ------------------------------------------------------------------------------
1517
// Tests
1618
// ------------------------------------------------------------------------------
@@ -32,10 +34,7 @@ eslintTester.addRuleTest('lib/rules/no-did-mount-set-state', {
3234
}, {
3335
code: [
3436
'var Hello = React.createClass({',
35-
'componentDidMount: function() {},',
36-
' render: function() {',
37-
' return <div>Hello {this.props.name}</div>;',
38-
' }',
37+
' componentDidMount: function() {}',
3938
'});'
4039
].join('\n'),
4140
ecmaFeatures: {
@@ -47,30 +46,126 @@ eslintTester.addRuleTest('lib/rules/no-did-mount-set-state', {
4746
' componentDidMount: function() {',
4847
' someNonMemberFunction(arg);',
4948
' this.someHandler = this.setState;',
50-
' },',
51-
' render: function() {',
52-
' return <div>Hello {this.props.name}</div>;',
5349
' }',
5450
'});'
5551
].join('\n'),
5652
ecmaFeatures: {
5753
jsx: true
5854
}
55+
}, {
56+
code: [
57+
'var Hello = React.createClass({',
58+
' componentDidMount: function() {',
59+
' someClass.onSomeEvent(function(data) {',
60+
' this.setState({',
61+
' data: data',
62+
' });',
63+
' })',
64+
' }',
65+
'});'
66+
].join('\n'),
67+
args: [1, 'allow-in-func'],
68+
ecmaFeatures: {
69+
jsx: true
70+
}
71+
}, {
72+
code: [
73+
'var Hello = React.createClass({',
74+
' componentDidMount: function() {',
75+
' function handleEvent(data) {',
76+
' this.setState({',
77+
' data: data',
78+
' });',
79+
' }',
80+
' someClass.onSomeEvent(handleEvent)',
81+
' }',
82+
'});'
83+
].join('\n'),
84+
parser: 'babel-eslint',
85+
args: [1, 'allow-in-func'],
86+
ecmaFeatures: {
87+
jsx: true
88+
}
5989
}],
6090

6191
invalid: [{
6292
code: [
6393
'var Hello = React.createClass({',
6494
' componentDidMount: function() {',
65-
' this.setState({',
66-
' name: this.props.name.toUpperCase()',
67-
' });',
68-
' },',
69-
' render: function() {',
70-
' return <div>Hello {this.state.name}</div>;',
95+
' this.setState({',
96+
' data: data',
97+
' });',
98+
' }',
99+
'});'
100+
].join('\n'),
101+
ecmaFeatures: {
102+
jsx: true
103+
},
104+
errors: [{
105+
message: 'Do not use setState in componentDidMount'
106+
}]
107+
}, {
108+
code: [
109+
'var Hello = React.createClass({',
110+
' componentDidMount: function() {',
111+
' this.setState({',
112+
' data: data',
113+
' });',
114+
' }',
115+
'});'
116+
].join('\n'),
117+
args: [1, 'allow-in-func'],
118+
ecmaFeatures: {
119+
jsx: true
120+
},
121+
errors: [{
122+
message: 'Do not use setState in componentDidMount'
123+
}]
124+
}, {
125+
code: [
126+
'var Hello = React.createClass({',
127+
' componentDidMount: function() {',
128+
' someClass.onSomeEvent(function(data) {',
129+
' this.setState({',
130+
' data: data',
131+
' });',
132+
' })',
133+
' }',
134+
'});'
135+
].join('\n'),
136+
ecmaFeatures: {
137+
jsx: true
138+
},
139+
errors: [{
140+
message: 'Do not use setState in componentDidMount'
141+
}]
142+
}, {
143+
code: [
144+
'var Hello = React.createClass({',
145+
' componentDidMount: function() {',
146+
' if (true) {',
147+
' this.setState({',
148+
' data: data',
149+
' });',
150+
' }',
151+
' }',
152+
'});'
153+
].join('\n'),
154+
ecmaFeatures: {
155+
jsx: true
156+
},
157+
errors: [{
158+
message: 'Do not use setState in componentDidMount'
159+
}]
160+
}, {
161+
code: [
162+
'var Hello = React.createClass({',
163+
' componentDidMount: function() {',
164+
' someClass.onSomeEvent((data) => this.setState({data: data}));',
71165
' }',
72166
'});'
73167
].join('\n'),
168+
parser: 'babel-eslint',
74169
ecmaFeatures: {
75170
jsx: true
76171
},

0 commit comments

Comments
 (0)