Allow this.state assignment on constructor

see jsx-eslint#832
burabure · May 30, 2017 · 36c7ade · 36c7ade
1 parent 7ebcd48
commit 36c7ade
Show file tree

Hide file tree

Showing 3 changed files with 174 additions and 17 deletions.
diff --git a/docs/rules/no-direct-mutation-state.md b/docs/rules/no-direct-mutation-state.md
@@ -3,6 +3,8 @@
 NEVER mutate `this.state` directly, as calling `setState()` afterwards may replace
 the mutation you made. Treat `this.state` as if it were immutable.
 
+The only place that's acceptable to assign this.state is in a ES6 `class` component constructor.
+
 ## Rule Details
 
 This rule is aimed to forbid the use of mutating `this.state` directly.
@@ -18,6 +20,17 @@ var Hello = createReactClass({
     return <div>Hello {this.state.name}</div>;
   }
 });
+
+class Hello extends React.Component {
+  constructor(props) {
+    super(props)
+
+    // Assign at instance creation time, not on a callback
+    doSomethingAsync(() => {
+      this.state = 'bad';
+    });
+  }
+}
 ```
 
 
@@ -34,4 +47,14 @@ var Hello = createReactClass({
     return <div>Hello {this.state.name}</div>;
   }
 });
+
+class Hello extends React.Component {
+  constructor(props) {
+    super(props)
+
+    this.state = {
+      foo: 'bar',
+    }
+  }
+}
 ```
diff --git a/lib/rules/no-direct-mutation-state.js b/lib/rules/no-direct-mutation-state.js
@@ -1,10 +1,10 @@
 /**
  * @fileoverview Prevent direct mutation of this.state
  * @author David Petersen
+ * @author Nicolas Fernandez <@burabure>
  */
 'use strict';
 
-var has = require('has');
 var Components = require('../util/Components');
 
 // ------------------------------------------------------------------------------
@@ -49,41 +49,62 @@ module.exports = {
     // --------------------------------------------------------------------------
     // Public
     // --------------------------------------------------------------------------
+    let inConstructor = false;
+    let inCallExpression = false;
 
     return {
-      AssignmentExpression: function(node) {
-        var item;
-        if (!node.left || !node.left.object || !node.left.object.object) {
+      MethodDefinition(node) {
+        if (node.kind === 'constructor') {
+          inConstructor = true;
+        }
+      },
+
+      CallExpression: function() {
+        inCallExpression = true;
+      },
+
+      AssignmentExpression(node) {
+        let item;
+        if ((inConstructor && !inCallExpression) || !node.left || !node.left.object) {
           return;
         }
-        item = node.left.object;
-        while (item.object.property) {
+        item = node.left;
+        while (item.object && item.object.property) {
           item = item.object;
         }
         if (
           item.object.type === 'ThisExpression' &&
           item.property.name === 'state'
         ) {
-          var component = components.get(utils.getParentComponent());
-          var mutations = component && component.mutations || [];
+          const component = components.get(utils.getParentComponent());
+          const mutations = (component && component.mutations) || [];
           mutations.push(node.left.object);
           components.set(node, {
             mutateSetState: true,
-            mutations: mutations
+            mutations
           });
         }
       },
 
-      'Program:exit': function() {
-        var list = components.list();
-        for (var component in list) {
-          if (!has(list, component) || isValid(list[component])) {
-            continue;
-          }
-          reportMutations(list[component]);
+      'CallExpression:exit': function() {
+        inCallExpression = false;
+      },
+
+      'MethodDefinition:exit': function (node) {
+        if (node.kind === 'constructor') {
+          inConstructor = false;
         }
+      },
+
+      'Program:exit': function () {
+        const list = components.list();
+
+        Object.keys(list).forEach((key) => {
+          if (!isValid(list[key])) {
+            reportMutations(list[key]);
+          }
+        });
       }
     };
-
   })
 };
diff --git a/tests/lib/rules/no-direct-mutation-state.js b/tests/lib/rules/no-direct-mutation-state.js
@@ -61,6 +61,14 @@ ruleTester.run('no-direct-mutation-state', rule, {
       '  }',
       '}'
     ].join('\n')
+  }, {
+    code: [
+      'class Hello extends React.Component {',
+      '  constructor() {',
+      '    this.state.foo = "bar"',
+      '  }',
+      '}'
+    ].join('\n')
   }],
 
   invalid: [{
@@ -118,6 +126,111 @@ ruleTester.run('no-direct-mutation-state', rule, {
       line: 4,
       column: 5
     }]
+  }, {
+    code: [
+      'class Hello extends React.Component {',
+      '  constructor() {',
+      '    someFn()',
+      '  }',
+      '  someFn() {',
+      '    this.state.foo = "bar"',
+      '  }',
+      '}'
+    ].join('\n'),
+    errors: [{
+      message: 'Do not mutate state directly. Use setState().'
+    }]
+  }, {
+    code: [
+      'class Hello extends React.Component {',
+      '  constructor(props) {',
+      '    super(props)',
+      '    doSomethingAsync(() => {',
+      '      this.state = "bad";',
+      '    });',
+      '  }',
+      '}'
+    ].join('\n'),
+    errors: [{
+      message: 'Do not mutate state directly. Use setState().'
+    }]
+  }, {
+    code: [
+      'class Hello extends React.Component {',
+      '  componentWillMount() {',
+      '    this.state.foo = "bar"',
+      '  }',
+      '}'
+    ].join('\n'),
+    errors: [{
+      message: 'Do not mutate state directly. Use setState().'
+    }]
+  }, {
+    code: [
+      'class Hello extends React.Component {',
+      '  componentDidMount() {',
+      '    this.state.foo = "bar"',
+      '  }',
+      '}'
+    ].join('\n'),
+    errors: [{
+      message: 'Do not mutate state directly. Use setState().'
+    }]
+  }, {
+    code: [
+      'class Hello extends React.Component {',
+      '  componentWillReceiveProps() {',
+      '    this.state.foo = "bar"',
+      '  }',
+      '}'
+    ].join('\n'),
+    errors: [{
+      message: 'Do not mutate state directly. Use setState().'
+    }]
+  }, {
+    code: [
+      'class Hello extends React.Component {',
+      '  shouldComponentUpdate() {',
+      '    this.state.foo = "bar"',
+      '  }',
+      '}'
+    ].join('\n'),
+    errors: [{
+      message: 'Do not mutate state directly. Use setState().'
+    }]
+  }, {
+    code: [
+      'class Hello extends React.Component {',
+      '  componentWillUpdate() {',
+      '    this.state.foo = "bar"',
+      '  }',
+      '}'
+    ].join('\n'),
+    errors: [{
+      message: 'Do not mutate state directly. Use setState().'
+    }]
+  }, {
+    code: [
+      'class Hello extends React.Component {',
+      '  componentDidUpdate() {',
+      '    this.state.foo = "bar"',
+      '  }',
+      '}'
+    ].join('\n'),
+    errors: [{
+      message: 'Do not mutate state directly. Use setState().'
+    }]
+  }, {
+    code: [
+      'class Hello extends React.Component {',
+      '  componentWillUnmount() {',
+      '    this.state.foo = "bar"',
+      '  }',
+      '}'
+    ].join('\n'),
+    errors: [{
+      message: 'Do not mutate state directly. Use setState().'
+    }]
   }
   /**
    * Would be nice to prevent this too