-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Add no-will-update-set-state rule #1139
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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>; | ||
} | ||
}); | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
/** | ||
* @fileoverview Prevent usage of setState in componentWillUpdate | ||
* @author Yannick Croissant | ||
*/ | ||
'use strict'; | ||
|
||
// ------------------------------------------------------------------------------ | ||
// Rule Definition | ||
// ------------------------------------------------------------------------------ | ||
|
||
module.exports = { | ||
meta: { | ||
docs: { | ||
description: 'Prevent usage of setState in componentWillUpdate', | ||
category: 'Best Practices', | ||
recommended: false | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
} | ||
} | ||
}; | ||
|
||
} | ||
}; |
There was a problem hiding this comment.
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.