From 877e6dce413ea9c54fa3cec96a791bb14a2b7f48 Mon Sep 17 00:00:00 2001 From: William Holloway Date: Wed, 5 Apr 2017 14:17:48 -0700 Subject: [PATCH 1/3] Add no-will-update-set-state rule --- README.md | 1 + docs/rules/no-will-update-set-state.md | 90 ++++++++ index.js | 1 + lib/rules/no-will-update-set-state.js | 66 ++++++ tests/lib/rules/no-will-update-set-state.js | 238 ++++++++++++++++++++ 5 files changed, 396 insertions(+) create mode 100644 docs/rules/no-will-update-set-state.md create mode 100644 lib/rules/no-will-update-set-state.js create mode 100644 tests/lib/rules/no-will-update-set-state.js diff --git a/README.md b/README.md index 06f97b7b75..0bbbed59e2 100644 --- a/README.md +++ b/README.md @@ -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 diff --git a/docs/rules/no-will-update-set-state.md b/docs/rules/no-will-update-set-state.md new file mode 100644 index 0000000000..e3dddee237 --- /dev/null +++ b/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
Hello {this.state.name}
; + } +}); +``` + +The following patterns are not considered warnings: + +```jsx +var Hello = React.createClass({ + componentWillUpdate: function() { + this.props.prepareHandler(); + }, + render: function() { + return
Hello {this.props.name}
; + } +}); +``` + +```jsx +var Hello = React.createClass({ + componentWillUpdate: function() { + this.prepareHandler(function callback(newName) { + this.setState({ + name: newName + }); + }); + }, + render: function() { + return
Hello {this.props.name}
; + } +}); +``` + +## Rule Options + +```js +... +"no-will-update-set-state": [, ] +... +``` + +### `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
Hello {this.state.name}
; + } +}); +``` + +```jsx +var Hello = React.createClass({ + componentDidUpdate: function() { + this.prepareHandler(function callback(newName) { + this.setState({ + name: newName + }); + }); + }, + render: function() { + return
Hello {this.state.name}
; + } +}); +``` diff --git a/index.js b/index.js index 914e4433f3..9fd15e15bf 100644 --- a/index.js +++ b/index.js @@ -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'), diff --git a/lib/rules/no-will-update-set-state.js b/lib/rules/no-will-update-set-state.js new file mode 100644 index 0000000000..e0da8aa7ef --- /dev/null +++ b/lib/rules/no-will-update-set-state.js @@ -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 + }, + + 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; + } + } + }; + + } +}; diff --git a/tests/lib/rules/no-will-update-set-state.js b/tests/lib/rules/no-will-update-set-state.js new file mode 100644 index 0000000000..292f32249c --- /dev/null +++ b/tests/lib/rules/no-will-update-set-state.js @@ -0,0 +1,238 @@ +/** + * @fileoverview Prevent usage of setState in componentWillUpdate + * @author Yannick Croissant + */ +'use strict'; + +// ------------------------------------------------------------------------------ +// Requirements +// ------------------------------------------------------------------------------ + +var rule = require('../../../lib/rules/no-will-update-set-state'); +var RuleTester = require('eslint').RuleTester; + +var parserOptions = { + ecmaVersion: 6, + ecmaFeatures: { + jsx: true + } +}; + +require('babel-eslint'); + +// ------------------------------------------------------------------------------ +// Tests +// ------------------------------------------------------------------------------ + +var ruleTester = new RuleTester(); +ruleTester.run('no-will-update-set-state', rule, { + + valid: [{ + code: [ + 'var Hello = React.createClass({', + ' render: function() {', + ' return
Hello {this.props.name}
;', + ' }', + '});' + ].join('\n'), + parserOptions: parserOptions + }, { + code: [ + 'var Hello = React.createClass({', + ' componentWillUpdate: function() {}', + '});' + ].join('\n'), + parserOptions: parserOptions + }, { + code: [ + 'var Hello = React.createClass({', + ' componentWillUpdate: function() {', + ' someNonMemberFunction(arg);', + ' this.someHandler = this.setState;', + ' }', + '});' + ].join('\n'), + parserOptions: parserOptions + }, { + code: [ + 'var Hello = React.createClass({', + ' componentWillUpdate: function() {', + ' someClass.onSomeEvent(function(data) {', + ' this.setState({', + ' data: data', + ' });', + ' })', + ' }', + '});' + ].join('\n'), + parserOptions: parserOptions + }, { + code: [ + 'var Hello = React.createClass({', + ' componentWillUpdate: function() {', + ' function handleEvent(data) {', + ' this.setState({', + ' data: data', + ' });', + ' }', + ' someClass.onSomeEvent(handleEvent)', + ' }', + '});' + ].join('\n'), + parser: 'babel-eslint', + parserOptions: parserOptions + }], + + invalid: [{ + code: [ + 'var Hello = React.createClass({', + ' componentWillUpdate: function() {', + ' this.setState({', + ' data: data', + ' });', + ' }', + '});' + ].join('\n'), + parserOptions: parserOptions, + errors: [{ + message: 'Do not use setState in componentWillUpdate' + }] + }, { + code: [ + 'class Hello extends React.Component {', + ' componentWillUpdate() {', + ' this.setState({', + ' data: data', + ' });', + ' }', + '}' + ].join('\n'), + parser: 'babel-eslint', + errors: [{ + message: 'Do not use setState in componentWillUpdate' + }] + }, { + code: [ + 'var Hello = React.createClass({', + ' componentWillUpdate: function() {', + ' this.setState({', + ' data: data', + ' });', + ' }', + '});' + ].join('\n'), + options: ['disallow-in-func'], + parserOptions: parserOptions, + errors: [{ + message: 'Do not use setState in componentWillUpdate' + }] + }, { + code: [ + 'class Hello extends React.Component {', + ' componentWillUpdate() {', + ' this.setState({', + ' data: data', + ' });', + ' }', + '}' + ].join('\n'), + parser: 'babel-eslint', + options: ['disallow-in-func'], + errors: [{ + message: 'Do not use setState in componentWillUpdate' + }] + }, { + code: [ + 'var Hello = React.createClass({', + ' componentWillUpdate: function() {', + ' someClass.onSomeEvent(function(data) {', + ' this.setState({', + ' data: data', + ' });', + ' })', + ' }', + '});' + ].join('\n'), + parserOptions: parserOptions, + options: ['disallow-in-func'], + errors: [{ + message: 'Do not use setState in componentWillUpdate' + }] + }, { + code: [ + 'class Hello extends React.Component {', + ' componentWillUpdate() {', + ' someClass.onSomeEvent(function(data) {', + ' this.setState({', + ' data: data', + ' });', + ' })', + ' }', + '}' + ].join('\n'), + parser: 'babel-eslint', + options: ['disallow-in-func'], + errors: [{ + message: 'Do not use setState in componentWillUpdate' + }] + }, { + code: [ + 'var Hello = React.createClass({', + ' componentWillUpdate: function() {', + ' if (true) {', + ' this.setState({', + ' data: data', + ' });', + ' }', + ' }', + '});' + ].join('\n'), + parserOptions: parserOptions, + errors: [{ + message: 'Do not use setState in componentWillUpdate' + }] + }, { + code: [ + 'class Hello extends React.Component {', + ' componentWillUpdate() {', + ' if (true) {', + ' this.setState({', + ' data: data', + ' });', + ' }', + ' }', + '}' + ].join('\n'), + parser: 'babel-eslint', + errors: [{ + message: 'Do not use setState in componentWillUpdate' + }] + }, { + code: [ + 'var Hello = React.createClass({', + ' componentWillUpdate: function() {', + ' someClass.onSomeEvent((data) => this.setState({data: data}));', + ' }', + '});' + ].join('\n'), + parser: 'babel-eslint', + parserOptions: parserOptions, + options: ['disallow-in-func'], + errors: [{ + message: 'Do not use setState in componentWillUpdate' + }] + }, { + code: [ + 'class Hello extends React.Component {', + ' componentWillUpdate() {', + ' someClass.onSomeEvent((data) => this.setState({data: data}));', + ' }', + '}' + ].join('\n'), + parser: 'babel-eslint', + options: ['disallow-in-func'], + errors: [{ + message: 'Do not use setState in componentWillUpdate' + }] + }] +}); From 58519aef5cce8d1c96af3d795dca59d661de0ae3 Mon Sep 17 00:00:00 2001 From: William Holloway Date: Thu, 6 Apr 2017 14:42:18 -0700 Subject: [PATCH 2/3] Factor out no--set-state rule code to a utility --- lib/rules/no-did-mount-set-state.js | 61 +---------------------- lib/rules/no-did-update-set-state.js | 61 +---------------------- lib/rules/no-will-update-set-state.js | 61 +---------------------- lib/util/makeNoMethodSetStateRule.js | 70 +++++++++++++++++++++++++++ 4 files changed, 76 insertions(+), 177 deletions(-) create mode 100644 lib/util/makeNoMethodSetStateRule.js diff --git a/lib/rules/no-did-mount-set-state.js b/lib/rules/no-did-mount-set-state.js index 648f9ec32e..353d26febf 100644 --- a/lib/rules/no-did-mount-set-state.js +++ b/lib/rules/no-did-mount-set-state.js @@ -4,63 +4,6 @@ */ 'use strict'; -// ------------------------------------------------------------------------------ -// Rule Definition -// ------------------------------------------------------------------------------ +var makeNoMethodSetStateRule = require('../util/makeNoSetStateRule'); -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'); diff --git a/lib/rules/no-did-update-set-state.js b/lib/rules/no-did-update-set-state.js index 53f096e3f1..9bee29677c 100644 --- a/lib/rules/no-did-update-set-state.js +++ b/lib/rules/no-did-update-set-state.js @@ -4,63 +4,6 @@ */ 'use strict'; -// ------------------------------------------------------------------------------ -// Rule Definition -// ------------------------------------------------------------------------------ +var makeNoMethodSetStateRule = require('../util/makeNoSetStateRule'); -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'); diff --git a/lib/rules/no-will-update-set-state.js b/lib/rules/no-will-update-set-state.js index e0da8aa7ef..7ca3edac41 100644 --- a/lib/rules/no-will-update-set-state.js +++ b/lib/rules/no-will-update-set-state.js @@ -4,63 +4,6 @@ */ 'use strict'; -// ------------------------------------------------------------------------------ -// Rule Definition -// ------------------------------------------------------------------------------ +var makeNoMethodSetStateRule = require('../util/makeNoSetStateRule'); -module.exports = { - meta: { - docs: { - description: 'Prevent usage of setState in componentWillUpdate', - 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 !== 'componentWillUpdate' || - (mode !== 'disallow-in-func' && depth > 1) - ) { - continue; - } - context.report({ - node: callee, - message: 'Do not use setState in componentWillUpdate' - }); - break; - } - } - }; - - } -}; +module.exports = makeNoMethodSetStateRule('componentWillUpdate'); diff --git a/lib/util/makeNoMethodSetStateRule.js b/lib/util/makeNoMethodSetStateRule.js new file mode 100644 index 0000000000..dc4c2c191f --- /dev/null +++ b/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; From 4405523fe1b8cfe51cb67e41f5587852562f0588 Mon Sep 17 00:00:00 2001 From: William Holloway Date: Thu, 6 Apr 2017 14:43:48 -0700 Subject: [PATCH 3/3] Fix util filename in require --- lib/rules/no-did-mount-set-state.js | 2 +- lib/rules/no-did-update-set-state.js | 2 +- lib/rules/no-will-update-set-state.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/rules/no-did-mount-set-state.js b/lib/rules/no-did-mount-set-state.js index 353d26febf..dd6422a602 100644 --- a/lib/rules/no-did-mount-set-state.js +++ b/lib/rules/no-did-mount-set-state.js @@ -4,6 +4,6 @@ */ 'use strict'; -var makeNoMethodSetStateRule = require('../util/makeNoSetStateRule'); +var makeNoMethodSetStateRule = require('../util/makeNoMethodSetStateRule'); module.exports = makeNoMethodSetStateRule('componentDidMount'); diff --git a/lib/rules/no-did-update-set-state.js b/lib/rules/no-did-update-set-state.js index 9bee29677c..75f42e5270 100644 --- a/lib/rules/no-did-update-set-state.js +++ b/lib/rules/no-did-update-set-state.js @@ -4,6 +4,6 @@ */ 'use strict'; -var makeNoMethodSetStateRule = require('../util/makeNoSetStateRule'); +var makeNoMethodSetStateRule = require('../util/makeNoMethodSetStateRule'); module.exports = makeNoMethodSetStateRule('componentDidUpdate'); diff --git a/lib/rules/no-will-update-set-state.js b/lib/rules/no-will-update-set-state.js index 7ca3edac41..8f73df7525 100644 --- a/lib/rules/no-will-update-set-state.js +++ b/lib/rules/no-will-update-set-state.js @@ -4,6 +4,6 @@ */ 'use strict'; -var makeNoMethodSetStateRule = require('../util/makeNoSetStateRule'); +var makeNoMethodSetStateRule = require('../util/makeNoMethodSetStateRule'); module.exports = makeNoMethodSetStateRule('componentWillUpdate');