Skip to content

Commit b9b1525

Browse files
ljharbljharb
authored
Merge pull request #1374 from jaaberg/no-access-state-in-setstate
Rule proposal: no-access-state-in-setstate
2 parents c84b879 + df92a49 commit b9b1525

File tree

5 files changed

+302
-0
lines changed

5 files changed

+302
-0
lines changed

‎README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ Finally, enable all of the rules that you would like to use. Use [our preset](#
9090
* [react/forbid-elements](docs/rules/forbid-elements.md): Forbid certain elements
9191
* [react/forbid-prop-types](docs/rules/forbid-prop-types.md): Forbid certain propTypes
9292
* [react/forbid-foreign-prop-types](docs/rules/forbid-foreign-prop-types.md): Forbid foreign propTypes
93+
* [react/no-access-state-in-setstate](docs/rules/no-access-state-in-setstate.md): Prevent using this.state inside this.setState
9394
* [react/no-array-index-key](docs/rules/no-array-index-key.md): Prevent using Array index in `key` props
9495
* [react/no-children-prop](docs/rules/no-children-prop.md): Prevent passing children as props
9596
* [react/no-danger](docs/rules/no-danger.md): Prevent usage of dangerous JSX properties

‎docs/rules/no-access-state-in-setstate.md

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
# Prevent using this.state within a this.setState (no-access-state-in-setstate)
2+
3+
This rule should prevent usage of `this.state` inside `setState` calls.
4+
Such usage of `this.state` might result in errors when two state calls are
5+
called in batch and thus referencing old state and not the current
6+
state. An example can be an increment function:
7+
8+
```
9+
function increment() {
10+
this.setState({value: this.state.value + 1});
11+
}
12+
```
13+
14+
If these two `setState` operations is grouped together in a batch it will
15+
look be something like the following, given that value is 1:
16+
17+
```
18+
setState({value: 1 + 1})
19+
setState({value: 1 + 1})
20+
```
21+
22+
This can be avoided with using callbacks which takes the previous state
23+
as first argument:
24+
25+
```
26+
function increment() {
27+
this.setState(prevState => ({value: prevState.value + 1}));
28+
}
29+
```
30+
31+
Then react will call the argument with the correct and updated state,
32+
even when things happen in batches. And the example above will be
33+
something like:
34+
35+
36+
```
37+
setState({value: 1 + 1})
38+
setState({value: 2 + 1})
39+
```

‎index.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ const allRules = {
3838
'jsx-uses-react': require('./lib/rules/jsx-uses-react'),
3939
'jsx-uses-vars': require('./lib/rules/jsx-uses-vars'),
4040
'jsx-wrap-multilines': require('./lib/rules/jsx-wrap-multilines'),
41+
'no-access-state-in-setstate': require('./lib/rules/no-access-state-in-setstate'),
4142
'no-array-index-key': require('./lib/rules/no-array-index-key'),
4243
'no-children-prop': require('./lib/rules/no-children-prop'),
4344
'no-danger': require('./lib/rules/no-danger'),

‎lib/rules/no-access-state-in-setstate.js

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
/**
2+
* @fileoverview Prevent usage of this.state within setState
3+
* @author Rolf Erik Lekang, Jørgen Aaberg
4+
*/
5+
6+
'use strict';
7+
8+
// ------------------------------------------------------------------------------
9+
// Rule Definition
10+
// ------------------------------------------------------------------------------
11+
12+
module.exports = {
13+
meta: {
14+
docs: {
15+
description: 'Reports when this.state is accessed within setState',
16+
category: 'Possible Errors',
17+
recommended: false
18+
}
19+
},
20+
21+
create: function(context) {
22+
function isSetStateCall(node) {
23+
return node.type === 'CallExpression' &&
24+
node.callee.property &&
25+
node.callee.property.name === 'setState' &&
26+
node.callee.object.type === 'ThisExpression';
27+
}
28+
29+
// The methods array contains all methods or functions that are using this.state
30+
// or that are calling another method or function using this.state
31+
const methods = [];
32+
// The vars array contains all variables that contains this.state
33+
const vars = [];
34+
return {
35+
CallExpression(node) {
36+
// Appends all the methods that are calling another
37+
// method containg this.state to the methods array
38+
methods.map(method => {
39+
if (node.callee.name === method.methodName) {
40+
let current = node.parent;
41+
while (current.type !== 'Program') {
42+
if (current.type === 'MethodDefinition') {
43+
methods.push({
44+
methodName: current.key.name,
45+
node: method.node
46+
});
47+
break;
48+
}
49+
current = current.parent;
50+
}
51+
}
52+
});
53+
54+
// Finding all CallExpressions that is inside a setState
55+
// to further check if they contains this.state
56+
let current = node.parent;
57+
while (current.type !== 'Program') {
58+
if (isSetStateCall(current)) {
59+
const methodName = node.callee.name;
60+
methods.map(method => {
61+
if (method.methodName === methodName) {
62+
context.report(
63+
method.node,
64+
'Use callback in setState when referencing the previous state.'
65+
);
66+
}
67+
});
68+
69+
break;
70+
}
71+
current = current.parent;
72+
}
73+
},
74+
75+
MemberExpression(node) {
76+
if (
77+
node.property.name === 'state' &&
78+
node.object.type === 'ThisExpression'
79+
) {
80+
let current = node;
81+
while (current.type !== 'Program') {
82+
// Reporting if this.state is directly within this.setState
83+
if (isSetStateCall(current)) {
84+
context.report(
85+
node,
86+
'Use callback in setState when referencing the previous state.'
87+
);
88+
break;
89+
}
90+
91+
// Storing all functions and methods that contains this.state
92+
if (current.type === 'MethodDefinition') {
93+
methods.push({
94+
methodName: current.key.name,
95+
node: node
96+
});
97+
break;
98+
} else if (current.type === 'FunctionExpression') {
99+
methods.push({
100+
methodName: current.parent.key.name,
101+
node: node
102+
});
103+
break;
104+
}
105+
106+
// Storing all variables containg this.state
107+
if (current.type === 'VariableDeclarator') {
108+
vars.push({
109+
node: node,
110+
scope: context.getScope()
111+
});
112+
break;
113+
}
114+
115+
current = current.parent;
116+
}
117+
}
118+
},
119+
120+
Identifier(node) {
121+
// Checks if the identifier is a variable within an object
122+
let current = node;
123+
while (current.parent.type === 'BinaryExpression') {
124+
current = current.parent;
125+
}
126+
if (current.parent.value === current) {
127+
while (current.type !== 'Program') {
128+
if (isSetStateCall(current)) {
129+
vars
130+
.filter(v => v.scope === context.getScope())
131+
.map(v => context.report(
132+
v.node,
133+
'Use callback in setState when referencing the previous state.'
134+
));
135+
}
136+
current = current.parent;
137+
}
138+
}
139+
}
140+
};
141+
}
142+
};

‎tests/lib/rules/no-access-state-in-setstate.js

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
/**
2+
* @fileoverview Prevent usage of this.state within setState
3+
* @author Rolf Erik Lekang, Jørgen Aaberg
4+
*/
5+
'use strict';
6+
7+
// ------------------------------------------------------------------------------
8+
// Requirements
9+
// ------------------------------------------------------------------------------
10+
11+
const rule = require('../../../lib/rules/no-access-state-in-setstate');
12+
const RuleTester = require('eslint').RuleTester;
13+
14+
const parserOptions = {
15+
ecmaVersion: 6,
16+
ecmaFeatures: {
17+
jsx: true
18+
}
19+
};
20+
21+
// ------------------------------------------------------------------------------
22+
// Tests
23+
// ------------------------------------------------------------------------------
24+
25+
const ruleTester = new RuleTester();
26+
ruleTester.run('no-access-state-in-setstate', rule, {
27+
valid: [{
28+
code: [
29+
'var Hello = React.createClass({',
30+
' onClick: function() {',
31+
' this.setState(state => ({value: state.value + 1}))',
32+
' }',
33+
'});'
34+
].join('\n'),
35+
parserOptions: parserOptions
36+
}, {
37+
code: [
38+
'var Hello = React.createClass({',
39+
' multiplyValue: function(obj) {',
40+
' return obj.value*2',
41+
' },',
42+
' onClick: function() {',
43+
' var value = this.state.value',
44+
' this.multiplyValue({ value: value })',
45+
' }',
46+
'});'
47+
].join('\n'),
48+
parserOptions: parserOptions
49+
}],
50+
51+
invalid: [{
52+
code: [
53+
'var Hello = React.createClass({',
54+
' onClick: function() {',
55+
' this.setState({value: this.state.value + 1})',
56+
' }',
57+
'});'
58+
].join('\n'),
59+
parserOptions: parserOptions,
60+
errors: [{
61+
message: 'Use callback in setState when referencing the previous state.'
62+
}]
63+
}, {
64+
code: [
65+
'var Hello = React.createClass({',
66+
' onClick: function() {',
67+
' this.setState(() => ({value: this.state.value + 1}))',
68+
' }',
69+
'});'
70+
].join('\n'),
71+
parserOptions: parserOptions,
72+
errors: [{
73+
message: 'Use callback in setState when referencing the previous state.'
74+
}]
75+
}, {
76+
code: [
77+
'var Hello = React.createClass({',
78+
' onClick: function() {',
79+
' var nextValue = this.state.value + 1',
80+
' this.setState({value: nextValue})',
81+
' }',
82+
'});'
83+
].join('\n'),
84+
parserOptions: parserOptions,
85+
errors: [{
86+
message: 'Use callback in setState when referencing the previous state.'
87+
}]
88+
}, {
89+
code: [
90+
'function nextState(state) {',
91+
' return {value: state.value + 1}',
92+
'}',
93+
'var Hello = React.createClass({',
94+
' onClick: function() {',
95+
' this.setState(nextState(this.state))',
96+
' }',
97+
'});'
98+
].join('\n'),
99+
parserOptions: parserOptions,
100+
errors: [{
101+
message: 'Use callback in setState when referencing the previous state.'
102+
}]
103+
}, {
104+
code: [
105+
'var Hello = React.createClass({',
106+
' nextState: function() {',
107+
' return {value: this.state.value + 1}',
108+
' },',
109+
' onClick: function() {',
110+
' this.setState(nextState())',
111+
' }',
112+
'});'
113+
].join('\n'),
114+
parserOptions: parserOptions,
115+
errors: [{
116+
message: 'Use callback in setState when referencing the previous state.'
117+
}]
118+
}]
119+
});

0 commit comments

Comments
 (0)