Repository: yannickcr/eslint-plugin-react
Pull request #1411 (2017-08-31 22:36:06)

[no-unused-state] Add ability to handle arrow function class method
For https://github.com/yannickcr/eslint-plugin-react/issues/1363, so that state property usage by object destructuring assignment in a class method defined with an arrow function gets recorded. Examples:

```js
class ArrowFunctionClassMethodTest extends React.Component {
  constructor() {
    this.state = { foo: 0 };
  }

  doSomething = () => {
    const { state: { foo } } = this;

    return foo;
  }

  render() {
    return <SomeComponent />;
  }
}
```

should not be warned.

Let me know if there is anything I can do to improve this PR. Also, just to confirm, this rule doesn't care about whether the class method is used or not, right?

jackyho112: @ljharb 

This list will only be added to if the property name gets assigned to a different variable name.  

For instance,

```js
class App extends React.Component {
        state = { foo: 0 };

        doSomething = () => {
          const { state: { foo: bar }} = this;

          return bar;
        }

        render() {
          return <SomeComponent />;
        }
      }
```

will add `'bar'` to the list. I have added one more test regarding this.

I also refactored how the code deals with `classInfo.aliases` according to your feedback. We still need to clear the set after each method definition though because each alias added is local to the class method. Let me know if this looks better. 
jackyho112: @ljharb 

I have restored the implementation back to using `classInfo.aliases = new Set()` which is a part of the original code. However, we still need to do `classInfo.aliases = new Set()` for every class method defintion because the aliases in `classInfo.aliases` are local to each class method. Let me know if that's ok.
jackyho112: @ljharb 

Thanks for the review! After this is merged, I can take a look at documenting no-unused-state as I saw there is an issue open for it.
jackyho112: [](url)
jackyho112: One test for the `no-typos` rule is failing in the builds. Not sure why. Don't think it has anything to do with node versions.
jackyho112: @ljharb 

I think this [test](https://github.com/jackyho112/eslint-plugin-react/commit/fa890cbad6d34658fda810288c4ab7765ec9a4f0#diff-a36b384ace30edac8eacb00c170eed54R247) is failing on master. Any clue? Maybe this test should have errors in the first place?
jseminck: Thanks @jackyho112 

This is because of the new ESLint release. They changed some internal process and a case that we didn't handle before will now work.

I can update the case to be valid, but it would only work with the latest ESLint. With older cases it will still fail. Even more: We had to write some work-around to make sure ESLint did not crash but I can't remove that code unless we update the peerDependency for ESLint.

I also commented here: https://github.com/yannickcr/eslint-plugin-react/pull/1375#issuecomment-329899585

But I think I can start a new PR and we can start/continue a discussion there.
jseminck: Created a PR to move that test: https://github.com/yannickcr/eslint-plugin-react/pull/1437

Patch:
diff --git a/lib/rules/no-unused-state.js b/lib/rules/no-unused-state.js
index d4bef8c2..4040d80d 100644
--- a/lib/rules/no-unused-state.js
+++ b/lib/rules/no-unused-state.js
@@ -237,6 +237,27 @@ module.exports = {
         ) {
           addStateFields(node.value);
         }
+
+        if (
+          !node.static &&
+          node.value &&
+          node.value.type === 'ArrowFunctionExpression'
+        ) {
+          // Create a new set for this.state aliases local to this method.
+          classInfo.aliases = new Set();
+        }
+      },
+
+      'ClassProperty:exit'(node) {
+        if (
+          classInfo &&
+          !node.static &&
+          node.value &&
+          node.value.type === 'ArrowFunctionExpression'
+        ) {
+          // Forget our set of local aliases.
+          classInfo.aliases = null;
+        }
       },
 
       MethodDefinition() {


Test patch:
diff --git a/tests/lib/rules/no-unused-state.js b/tests/lib/rules/no-unused-state.js
index 4fbb8a4f..62f9599e 100644
--- a/tests/lib/rules/no-unused-state.js
+++ b/tests/lib/rules/no-unused-state.js
@@ -387,6 +387,22 @@ eslintTester.run('no-unused-state', rule, {
           return <SomeComponent foo={foo} />;
         }
       }`,
+    `class NonRenderClassMethodFalseNegativeTest extends React.Component {
+        constructor() {
+          this.state = { foo: 0, bar: 0 };
+        }
+        doSomething() {
+          const { foo } = this.state;
+          return this.state.foo;
+        }
+        doSomethingElse() {
+          const { state: { bar }} = this;
+          return bar;
+        }
+        render() {
+          return <SomeComponent />;
+        }
+      }`,
     {
       code: `class TypeCastExpressionSpreadFalseNegativeTest extends React.Component {
         constructor() {
@@ -397,6 +413,72 @@ eslintTester.run('no-unused-state', rule, {
         }
       }`,
       parser: 'babel-eslint'
+    },
+    {
+      code: `class ArrowFunctionClassMethodDestructuringFalseNegativeTest extends React.Component {
+        constructor() {
+          this.state = { foo: 0 };
+        }
+
+        doSomething = () => {
+          const { state: { foo } } = this;
+
+          return foo;
+        }
+
+        render() {
+          return <SomeComponent />;
+        }
+      }`,
+      parser: 'babel-eslint'
+    },
+    {
+      code: `class ArrowFunctionClassMethodWithClassPropertyTransformFalseNegativeTest extends React.Component {
+        state = { foo: 0 };
+
+        doSomething = () => {
+          const { state:{ foo } } = this;
+
+          return foo;
+        }
+
+        render() {
+          return <SomeComponent />;
+        }
+      }`,
+      parser: 'babel-eslint'
+    },
+    {
+      code: `class ArrowFunctionClassMethodDeepDestructuringFalseNegativeTest extends React.Component {
+        state = { foo: { bar: 0 } };
+
+        doSomething = () => {
+          const { state: { foo: { bar }}} = this;
+
+          return bar;
+        }
+
+        render() {
+          return <SomeComponent />;
+        }
+      }`,
+      parser: 'babel-eslint'
+    },
+    {
+      code: `class ArrowFunctionClassMethodDestructuringAssignmentFalseNegativeTest extends React.Component {
+        state = { foo: 0 };
+
+        doSomething = () => {
+          const { state: { foo: bar }} = this;
+
+          return bar;
+        }
+
+        render() {
+          return <SomeComponent />;
+        }
+      }`,
+      parser: 'babel-eslint'
     }
   ],
 
@@ -642,6 +724,21 @@ eslintTester.run('no-unused-state', rule, {
         }`,
       errors: getErrorMessages(['foo'])
     },
+    {
+      code: `class UnusedStateArrowFunctionMethodTest extends React.Component {
+          constructor() {
+            this.state = { foo: 0 };
+          }
+          doSomething = () => {
+            return null;
+          }
+          render() {
+            return <SomeComponent />;
+          }
+        }`,
+      errors: getErrorMessages(['foo']),
+      parser: 'babel-eslint'
+    },
     {
       code: `class TypeCastExpressionTest extends React.Component {
           constructor() {