eslint-plugin-use-encapsulation icon indicating copy to clipboard operation
eslint-plugin-use-encapsulation copied to clipboard

Doesn't catch wrapped React components (e.g., styled-components wrapped)

Open ngokevin opened this issue 3 years ago • 1 comments
trafficstars

Great post and lint!

Here's a diff that seems to work to catch 'em

diff --git a/node_modules/eslint-plugin-use-encapsulation/rules/prefer-custom-hooks.js b/node_modules/eslint-plugin-use-encapsulation/rules/prefer-custom-hooks.js
index 379b20b..99afe4a 100644
--- a/node_modules/eslint-plugin-use-encapsulation/rules/prefer-custom-hooks.js
+++ b/node_modules/eslint-plugin-use-encapsulation/rules/prefer-custom-hooks.js
@@ -1,26 +1,33 @@
 const { HOOK_PATTERN, REACT_HOOKS } = require('../src/constants')
 const { difference, union } = require('../src/utils')
 
-function getHookParent(node) {
-  if (node.type === 'Program') return
+const DEFAULT_OPTIONS = {
+  block: [],
+  allow: [],
+}
 
-  if (node.type === 'FunctionDeclaration') {
-    return node
-  }
+function isInsideComponent(node) {
+  const child = node;
 
-  if (
-    node.type === 'VariableDeclarator' &&
-    node.init.type === 'ArrowFunctionExpression'
-  ) {
-    return node
-  }
+  while (node) {
+    // Named function component.
+    if (node.type === 'FunctionDeclaration' && !(node.name || node.id.name).startsWith('use')) {
+      return true;
+    }
 
-  return getHookParent(node.parent)
-}
+    // Arrow function component.
+    if (node.init && node.init.type === 'ArrowFunctionExpression') {
+      return true;
+    }
 
-const DEFAULT_OPTIONS = {
-  block: [],
-  allow: [],
+    // Wrapped component.
+    if (node !== child && node.type === 'VariableDeclarator' && !(node.name || node.id.name || "").startsWith('use')) {
+      return true;
+    }
+
+    node = node.parent;
+  }
+  return false;
 }
 
 module.exports = {
@@ -38,13 +45,11 @@ module.exports = {
     return {
       Identifier(node) {
         if (hooksToCheck.has(node.name)) {
-          const hookParent = getHookParent(node)
-
-          if (hookParent && !HOOK_PATTERN.test(hookParent.id.name)) {
+          if (isInsideComponent(node)) {
             context.report({
               node,
               message:
-                'Do not use React Hooks directly in a component. Abstract the functionality into a custom hook and use that instead.',
+                'Hook encapsulation: do not use React Hooks directly in a component; abstract concerns to a custom hook (kyleshevlin.com/use-encapsulation)',
             })
           }
         }

ngokevin avatar Apr 01 '22 05:04 ngokevin

Thanks I'll take a look into this. It would be easier for me to review a PR than to read that diff, but I'll do my best.

kyleshevlin avatar Apr 16 '22 16:04 kyleshevlin