Add allow-in-func option to no-did-mount-set-state (fixes #56)

jsx-eslint · May 14, 2015 · 4c167eb · 4c167eb
1 parent 0c3d542
commit 4c167eb
Show file tree

Hide file tree

Showing 3 changed files with 190 additions and 22 deletions.
diff --git a/docs/rules/no-did-mount-set-state.md b/docs/rules/no-did-mount-set-state.md
@@ -4,15 +4,32 @@ Updating the state after a component mount will trigger a second `render()` call
 
 ## Rule Details
 
+This rule is aimed to forbids the use of `this.setState` in `componentDidMount`.
+
 The following patterns are considered warnings:
 
 ```js
 var Hello = React.createClass({
   componentDidMount: function() {
-     this.setState({
-        name: this.props.name.toUpperCase()
+    this.setState({
+      name: this.props.name.toUpperCase()
+    });
+  },
+  render: function() {
+    return <div>Hello {this.state.name}</div>;
+  }
+});
+```
+
+```js
+var Hello = React.createClass({
+  componentDidMount: function() {
+    this.onMount(function callback(newName) {
+      this.setState({
+        name: newName
       });
-    },
+    });
+  },
   render: function() {
     return <div>Hello {this.state.name}</div>;
   }
@@ -31,3 +48,47 @@ var Hello = React.createClass({
   }
 });
 ```
+
+## Rule Options
+
+```js
+...
+"no-did-mount-set-state": [<enabled>, <mode>]
+...
+```
+
+### `allow-in-func` mode
+
+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.
+
+The following patterns are considered warnings:
+
+```js
+var Hello = React.createClass({
+  componentDidMount: function() {
+    this.setState({
+      name: this.props.name.toUpperCase()
+    });
+  },
+  render: function() {
+    return <div>Hello {this.state.name}</div>;
+  }
+});
+```
+
+The following patterns are not considered warnings:
+
+```js
+var Hello = React.createClass({
+  componentDidMount: function() {
+    this.onMount(function callback(newName) {
+      this.setState({
+        name: newName
+      });
+    });
+  },
+  render: function() {
+    return <div>Hello {this.state.name}</div>;
+  }
+});
+```
diff --git a/lib/rules/no-did-mount-set-state.js b/lib/rules/no-did-mount-set-state.js
@@ -10,6 +10,8 @@
 
 module.exports = function(context) {
 
+  var mode = context.options[0] || 'never';
+
   // --------------------------------------------------------------------------
   // Public
   // --------------------------------------------------------------------------
@@ -18,18 +20,28 @@ module.exports = function(context) {
 
     CallExpression: function(node) {
       var callee = node.callee;
-      if (callee.type !== 'MemberExpression') {
-        return;
-      }
-      if (callee.object.type !== 'ThisExpression' || callee.property.name !== 'setState') {
+      if (
+        callee.type !== 'MemberExpression' ||
+        callee.object.type !== 'ThisExpression' ||
+        callee.property.name !== 'setState'
+      ) {
         return;
       }
-      var ancestors = context.getAncestors(callee);
+      var ancestors = context.getAncestors(callee).reverse();
+      var depth = 0;
       for (var i = 0, j = ancestors.length; i < j; i++) {
-        if (ancestors[i].type !== 'Property' || ancestors[i].key.name !== 'componentDidMount') {
+        if (/Function(Expression|Declaration)$/.test(ancestors[i].type)) {
+          depth++;
+        }
+        if (
+          ancestors[i].type !== 'Property' ||
+          ancestors[i].key.name !== 'componentDidMount' ||
+          (mode === 'allow-in-func' && depth > 1)
+        ) {
           continue;
         }
         context.report(callee, 'Do not use setState in componentDidMount');
+        break;
       }
     }
   };

diff --git a/tests/lib/rules/no-did-mount-set-state.js b/tests/lib/rules/no-did-mount-set-state.js
@@ -11,6 +11,8 @@
 var eslint = require('eslint').linter;
 var ESLintTester = require('eslint-tester');
 
+require('babel-eslint');
+
 // ------------------------------------------------------------------------------
 // Tests
 // ------------------------------------------------------------------------------
@@ -32,10 +34,7 @@ eslintTester.addRuleTest('lib/rules/no-did-mount-set-state', {
   }, {
     code: [
       'var Hello = React.createClass({',
-      'componentDidMount: function() {},',
-      '  render: function() {',
-      '    return <div>Hello {this.props.name}</div>;',
-      '  }',
+      '  componentDidMount: function() {}',
       '});'
     ].join('\n'),
     ecmaFeatures: {
@@ -47,30 +46,126 @@ eslintTester.addRuleTest('lib/rules/no-did-mount-set-state', {
       '  componentDidMount: function() {',
       '    someNonMemberFunction(arg);',
       '    this.someHandler = this.setState;',
-      '  },',
-      '  render: function() {',
-      '    return <div>Hello {this.props.name}</div>;',
       '  }',
       '});'
     ].join('\n'),
     ecmaFeatures: {
       jsx: true
     }
+  }, {
+    code: [
+      'var Hello = React.createClass({',
+      '  componentDidMount: function() {',
+      '    someClass.onSomeEvent(function(data) {',
+      '      this.setState({',
+      '        data: data',
+      '      });',
+      '    })',
+      '  }',
+      '});'
+    ].join('\n'),
+    args: [1, 'allow-in-func'],
+    ecmaFeatures: {
+      jsx: true
+    }
+  }, {
+    code: [
+      'var Hello = React.createClass({',
+      '  componentDidMount: function() {',
+      '    function handleEvent(data) {',
+      '      this.setState({',
+      '        data: data',
+      '      });',
+      '    }',
+      '    someClass.onSomeEvent(handleEvent)',
+      '  }',
+      '});'
+    ].join('\n'),
+    parser: 'babel-eslint',
+    args: [1, 'allow-in-func'],
+    ecmaFeatures: {
+      jsx: true
+    }
   }],
 
   invalid: [{
     code: [
       'var Hello = React.createClass({',
       '  componentDidMount: function() {',
-      '    this.setState({',
-      '      name: this.props.name.toUpperCase()',
-      '    });',
-      '  },',
-      '  render: function() {',
-      '    return <div>Hello {this.state.name}</div>;',
+    '      this.setState({',
+    '        data: data',
+    '      });',
+      '  }',
+      '});'
+    ].join('\n'),
+    ecmaFeatures: {
+      jsx: true
+    },
+    errors: [{
+      message: 'Do not use setState in componentDidMount'
+    }]
+  }, {
+    code: [
+      'var Hello = React.createClass({',
+      '  componentDidMount: function() {',
+    '      this.setState({',
+    '        data: data',
+    '      });',
+      '  }',
+      '});'
+    ].join('\n'),
+    args: [1, 'allow-in-func'],
+    ecmaFeatures: {
+      jsx: true
+    },
+    errors: [{
+      message: 'Do not use setState in componentDidMount'
+    }]
+  }, {
+    code: [
+      'var Hello = React.createClass({',
+      '  componentDidMount: function() {',
+      '    someClass.onSomeEvent(function(data) {',
+      '      this.setState({',
+      '        data: data',
+      '      });',
+      '    })',
+      '  }',
+      '});'
+    ].join('\n'),
+    ecmaFeatures: {
+      jsx: true
+    },
+    errors: [{
+      message: 'Do not use setState in componentDidMount'
+    }]
+  }, {
+    code: [
+      'var Hello = React.createClass({',
+      '  componentDidMount: function() {',
+      '    if (true) {',
+      '      this.setState({',
+      '        data: data',
+      '      });',
+      '    }',
+      '  }',
+      '});'
+    ].join('\n'),
+    ecmaFeatures: {
+      jsx: true
+    },
+    errors: [{
+      message: 'Do not use setState in componentDidMount'
+    }]
+  }, {
+    code: [
+      'var Hello = React.createClass({',
+      '  componentDidMount: function() {',
+      '    someClass.onSomeEvent((data) => this.setState({data: data}));',
       '  }',
       '});'
     ].join('\n'),
+    parser: 'babel-eslint',
     ecmaFeatures: {
       jsx: true
     },