Repository: yannickcr/eslint-plugin-react
Pull request #1139 (2017-04-05 21:23:27)

Add no-will-update-set-state rule
Basically just a copy and paste of the `no-did-update-set-state` rule for `componentWillUpdate`.  This rule would have saved me a fair bit of time tracking down an issue where this pattern had been used by accident so I thought it was worthwhile adding support for.


Patch:
diff --git a/README.md b/README.md
index 06f97b7b..0bbbed59 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 00000000..e3dddee2
--- /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 <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>;
+  }
+});
+```
diff --git a/index.js b/index.js
index 914e4433..9fd15e15 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-did-mount-set-state.js b/lib/rules/no-did-mount-set-state.js
index 648f9ec3..dd6422a6 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/makeNoMethodSetStateRule');
 
-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 53f096e3..75f42e52 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/makeNoMethodSetStateRule');
 
-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
new file mode 100644
index 00000000..8f73df75
--- /dev/null
+++ b/lib/rules/no-will-update-set-state.js
@@ -0,0 +1,9 @@
+/**
+ * @fileoverview Prevent usage of setState in componentWillUpdate
+ * @author Yannick Croissant
+ */
+'use strict';
+
+var makeNoMethodSetStateRule = require('../util/makeNoMethodSetStateRule');
+
+module.exports = makeNoMethodSetStateRule('componentWillUpdate');
diff --git a/lib/util/makeNoMethodSetStateRule.js b/lib/util/makeNoMethodSetStateRule.js
new file mode 100644
index 00000000..dc4c2c19
--- /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;


Test patch:
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 00000000..292f3224
--- /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 <div>Hello {this.props.name}</div>;',
+      '  }',
+      '});'
+    ].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'
+    }]
+  }]
+});