refactor(eslint-plugin-react-hooks): change array type and improve co…

…nditionals (#32400) - [build(eslint-plugin-react-hooks): add ts-linting](4c0fbe7) This change adds configuration to the eslint config governing `eslint-plugin-react-hooks` to use the typescript-eslint plugin and parser. It adds the typescript-recommended config, and configures the team's preferred `array-type` convention. - [refactor(eslint-plugin-react-hooks): improve conditionals](540d0d9) This change addresses several feedback items from #32240 - [ci (eslint-e2e): exclude nested node_modules from cache](a3279f4) This change removes the nested fixture `node_modules` from being cached, so that the symbolic link can be made after the build happens.
facebook · Feb 17, 2025 · 4632e36 · 4632e36
1 parent eb1f77d
commit 4632e36
Show file tree

Hide file tree

Showing 6 changed files with 97 additions and 26 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -446,10 +446,7 @@ module.exports = {
       },
     },
     {
-      files: [
-        'scripts/eslint-rules/*.js',
-        'packages/eslint-plugin-react-hooks/src/*.js',
-      ],
+      files: ['scripts/eslint-rules/*.js'],
       plugins: ['eslint-plugin'],
       rules: {
         'eslint-plugin/prefer-object-rule': ERROR,
@@ -517,6 +514,26 @@ module.exports = {
         __IS_INTERNAL_VERSION__: 'readonly',
       },
     },
+    {
+      files: ['packages/eslint-plugin-react-hooks/src/**/*'],
+      extends: ['plugin:@typescript-eslint/recommended'],
+      parser: '@typescript-eslint/parser',
+      plugins: ['@typescript-eslint', 'eslint-plugin'],
+      rules: {
+        '@typescript-eslint/no-explicit-any': OFF,
+        '@typescript-eslint/no-non-null-assertion': OFF,
+        '@typescript-eslint/array-type': [ERROR, {default: 'generic'}],
+
+        'es/no-optional-chaining': OFF,
+
+        'eslint-plugin/prefer-object-rule': ERROR,
+        'eslint-plugin/require-meta-fixable': [
+          ERROR,
+          {catchNoFixerButFixableProperty: true},
+        ],
+        'eslint-plugin/require-meta-has-suggestions': ERROR,
+      },
+    },
   ],
 
   env: {

diff --git a/.github/workflows/runtime_eslint_plugin_e2e.yml b/.github/workflows/runtime_eslint_plugin_e2e.yml
@@ -40,7 +40,7 @@ jobs:
         uses: actions/cache@v4
         id: node_modules
         with:
-          path: "**/node_modules"
+          path: "node_modules"
           key: runtime-eslint_e2e-node_modules-${{ runner.arch }}-${{ runner.os }}-${{ hashFiles('yarn.lock') }}
       - name: Ensure clean build directory
         run: rm -rf build

diff --git a/package.json b/package.json
@@ -42,6 +42,8 @@
     "@rollup/plugin-node-resolve": "^15.0.1",
     "@rollup/plugin-replace": "^5.0.2",
     "@rollup/plugin-typescript": "^12.1.2",
+    "@typescript-eslint/eslint-plugin": "^6.21.0",
+    "@typescript-eslint/parser": "^6.21.0",
     "abortcontroller-polyfill": "^1.7.5",
     "art": "0.10.1",
     "babel-plugin-syntax-trailing-function-commas": "^6.5.0",

diff --git a/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.ts b/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.ts
@@ -28,7 +28,7 @@ type DeclaredDependency = {
 
 type Dependency = {
   isStable: boolean;
-  references: Scope.Reference[];
+  references: Array<Scope.Reference>;
 };
 
 type DependencyTreeNode = {
@@ -184,7 +184,9 @@ const rule = {
       // Get the current scope.
       const scope = scopeManager.acquire(node);
       if (!scope) {
-        return;
+        throw new Error(
+          'Unable to acquire scope for the current node. This is a bug in eslint-plugin-react-hooks, please file an issue.',
+        );
       }
 
       // Find all our "pure scopes". On every re-render of a component these
@@ -255,7 +257,7 @@ const rule = {
         // Detect primitive constants
         // const foo = 42
         let declaration = defNode.parent;
-        if (declaration == null && componentScope) {
+        if (declaration == null && componentScope != null) {
           // This might happen if variable is declared after the callback.
           // In that case ESLint won't set up .parent refs.
           // So we'll set them up manually.
@@ -266,7 +268,7 @@ const rule = {
           }
         }
         if (
-          declaration &&
+          declaration != null &&
           'kind' in declaration &&
           declaration.kind === 'const' &&
           init.type === 'Literal' &&
@@ -454,7 +456,7 @@ const rule = {
       function isInsideEffectCleanup(reference: Scope.Reference): boolean {
         let curScope: Scope.Scope | null = reference.from;
         let isInReturnedFunction = false;
-        while (curScope && curScope.block !== node) {
+        while (curScope != null && curScope.block !== node) {
           if (curScope.type === 'function') {
             isInReturnedFunction =
               curScope.block.parent != null &&
@@ -529,7 +531,7 @@ const rule = {
             continue;
           }
           // Ignore references to the function itself as it's not defined yet.
-          if (def.node && def.node.init === node.parent) {
+          if (def.node != null && def.node.init === node.parent) {
             continue;
           }
           // Ignore Flow type parameters
@@ -566,8 +568,8 @@ const rule = {
           // Is React managing this ref or us?
           // Let's see if we can find a .current assignment.
           let foundCurrentAssignment = false;
-          for (const reference of references) {
-            const {identifier} = reference;
+          for (const ref of references) {
+            const {identifier} = ref;
             const {parent} = identifier;
             if (
               parent != null &&
@@ -660,7 +662,7 @@ const rule = {
             }
 
             let fnScope: Scope.Scope | null = reference.from;
-            while (fnScope && fnScope.type !== 'function') {
+            while (fnScope != null && fnScope.type !== 'function') {
               fnScope = fnScope.upper;
             }
             const isDirectlyInsideEffect = fnScope?.block === node;
@@ -704,7 +706,7 @@ const rule = {
         return;
       }
 
-      const declaredDependencies: DeclaredDependency[] = [];
+      const declaredDependencies: Array<DeclaredDependency> = [];
       const externalDependencies = new Set<string>();
       const isArrayExpression =
         declaredDependenciesNode.type === 'ArrayExpression';
@@ -1469,7 +1471,7 @@ function collectRecommendations({
   isEffect,
 }: {
   dependencies: Map<string, Dependency>;
-  declaredDependencies: DeclaredDependency[];
+  declaredDependencies: Array<DeclaredDependency>;
   stableDependencies: Set<string>;
   externalDependencies: Set<string>;
   isEffect: boolean;
@@ -1592,7 +1594,7 @@ function collectRecommendations({
   }
 
   // Collect suggestions in the order they were originally specified.
-  const suggestedDependencies: string[] = [];
+  const suggestedDependencies: Array<string> = [];
   const unnecessaryDependencies = new Set<string>();
   const duplicateDependencies = new Set<string>();
   declaredDependencies.forEach(({key}) => {
@@ -1699,7 +1701,7 @@ function scanForConstructions({
   componentScope,
   scope,
 }: {
-  declaredDependencies: DeclaredDependency[];
+  declaredDependencies: Array<DeclaredDependency>;
   declaredDependenciesNode: Node;
   componentScope: Scope.Scope;
   scope: Scope.Scope;
@@ -1747,7 +1749,7 @@ function scanForConstructions({
       }
       return null;
     })
-    .filter(Boolean) as [Scope.Variable, string][];
+    .filter(Boolean) as Array<[Scope.Variable, string]>;
 
   function isUsedOutsideOfHook(ref: Scope.Variable): boolean {
     let foundWriteExpr = false;
@@ -2007,7 +2009,7 @@ function fastFindReferenceWithParent(start: Node, target: Node): Node | null {
   return null;
 }
 
-function joinEnglish(arr: string[]): string {
+function joinEnglish(arr: Array<string>): string {
   let s = '';
   for (let i = 0; i < arr.length; i++) {
     s += arr[i];

diff --git a/packages/eslint-plugin-react-hooks/src/RulesOfHooks.ts b/packages/eslint-plugin-react-hooks/src/RulesOfHooks.ts
@@ -130,8 +130,10 @@ const rule = {
   },
   create(context: Rule.RuleContext) {
     let lastEffect: CallExpression | null = null;
-    const codePathReactHooksMapStack: Map<Rule.CodePathSegment, Node[]>[] = [];
-    const codePathSegmentStack: Rule.CodePathSegment[] = [];
+    const codePathReactHooksMapStack: Array<
+      Map<Rule.CodePathSegment, Array<Node>>
+    > = [];
+    const codePathSegmentStack: Array<Rule.CodePathSegment> = [];
     const useEffectEventFunctions = new WeakSet();
 
     // For a given scope, iterate through the references and add all useEffectEvent definitions. We can
@@ -190,7 +192,7 @@ const rule = {
       // Maintain code path stack as we traverse.
       onCodePathStart: () =>
         codePathReactHooksMapStack.push(
-          new Map<Rule.CodePathSegment, Node[]>(),
+          new Map<Rule.CodePathSegment, Array<Node>>(),
         ),
 
       // Process our code path.
@@ -561,7 +563,7 @@ const rule = {
                 context.report({node: hook, message});
               }
             } else if (
-              codePathNode.parent &&
+              codePathNode.parent != null &&
               (codePathNode.parent.type === 'MethodDefinition' ||
                 // @ts-expect-error `ClassProperty` was removed from typescript-estree in https://github.com/typescript-eslint/typescript-eslint/pull/3806
                 codePathNode.parent.type === 'ClassProperty' ||
@@ -758,7 +760,7 @@ function getFunctionName(node: Node) {
 /**
  * Convenience function for peeking the last item in a stack.
  */
-function last<T>(array: T[]): T {
+function last<T>(array: Array<T>): T {
   return array[array.length - 1] as T;
 }
 

diff --git a/yarn.lock b/yarn.lock
@@ -2410,6 +2410,11 @@
   dependencies:
     eslint-visitor-keys "^3.3.0"
 
+"@eslint-community/regexpp@^4.5.1":
+  version "4.12.1"
+  resolved "https://registry.yarnpkg.com/@eslint-community/regexpp/-/regexpp-4.12.1.tgz#cfc6cffe39df390a3841cde2abccf92eaa7ae0e0"
+  integrity sha512-CCZCDJuduB9OUkFkY2IgppNZMi2lBQgD2qzwXkEia16cge2pijY/aXi96CJMquDMn3nJdlPV1A5KrJEXwfLNzQ==
+
 "@eslint-community/regexpp@^4.6.1":
   version "4.10.0"
   resolved "https://registry.yarnpkg.com/@eslint-community/regexpp/-/regexpp-4.10.0.tgz#548f6de556857c8bb73bbee70c35dc82a2e74d63"
@@ -3718,6 +3723,23 @@
   dependencies:
     "@types/node" "*"
 
+"@typescript-eslint/eslint-plugin@^6.21.0":
+  version "6.21.0"
+  resolved "https://registry.yarnpkg.com/@typescript-eslint/eslint-plugin/-/eslint-plugin-6.21.0.tgz#30830c1ca81fd5f3c2714e524c4303e0194f9cd3"
+  integrity sha512-oy9+hTPCUFpngkEZUSzbf9MxI65wbKFoQYsgPdILTfbUldp5ovUuphZVe4i30emU9M/kP+T64Di0mxl7dSw3MA==
+  dependencies:
+    "@eslint-community/regexpp" "^4.5.1"
+    "@typescript-eslint/scope-manager" "6.21.0"
+    "@typescript-eslint/type-utils" "6.21.0"
+    "@typescript-eslint/utils" "6.21.0"
+    "@typescript-eslint/visitor-keys" "6.21.0"
+    debug "^4.3.4"
+    graphemer "^1.4.0"
+    ignore "^5.2.4"
+    natural-compare "^1.4.0"
+    semver "^7.5.4"
+    ts-api-utils "^1.0.1"
+
 "@typescript-eslint/[email protected]":
   version "2.34.0"
   resolved "https://registry.yarnpkg.com/@typescript-eslint/experimental-utils/-/experimental-utils-2.34.0.tgz#d3524b644cdb40eebceca67f8cf3e4cc9c8f980f"
@@ -3780,6 +3802,17 @@
     "@typescript-eslint/typescript-estree" "5.62.0"
     debug "^4.3.4"
 
+"@typescript-eslint/parser@^6.21.0":
+  version "6.21.0"
+  resolved "https://registry.yarnpkg.com/@typescript-eslint/parser/-/parser-6.21.0.tgz#af8fcf66feee2edc86bc5d1cf45e33b0630bf35b"
+  integrity sha512-tbsV1jPne5CkFQCgPBcDOt30ItF7aJoZL997JSF7MhGQqOeT3svWRYxiqlfA5RUdlHN6Fi+EI9bxqbdyAUZjYQ==
+  dependencies:
+    "@typescript-eslint/scope-manager" "6.21.0"
+    "@typescript-eslint/types" "6.21.0"
+    "@typescript-eslint/typescript-estree" "6.21.0"
+    "@typescript-eslint/visitor-keys" "6.21.0"
+    debug "^4.3.4"
+
 "@typescript-eslint/[email protected]":
   version "4.1.0"
   resolved "https://registry.yarnpkg.com/@typescript-eslint/scope-manager/-/scope-manager-4.1.0.tgz#9e389745ee9cfe12252ed1e9958808abd6b3a683"
@@ -3804,6 +3837,16 @@
     "@typescript-eslint/types" "6.21.0"
     "@typescript-eslint/visitor-keys" "6.21.0"
 
+"@typescript-eslint/[email protected]":
+  version "6.21.0"
+  resolved "https://registry.yarnpkg.com/@typescript-eslint/type-utils/-/type-utils-6.21.0.tgz#6473281cfed4dacabe8004e8521cee0bd9d4c01e"
+  integrity sha512-rZQI7wHfao8qMX3Rd3xqeYSMCL3SoiSQLBATSiVKARdFGCYSRvmViieZjqc58jKgs8Y8i9YvVVhRbHSTA4VBag==
+  dependencies:
+    "@typescript-eslint/typescript-estree" "6.21.0"
+    "@typescript-eslint/utils" "6.21.0"
+    debug "^4.3.4"
+    ts-api-utils "^1.0.1"
+
 "@typescript-eslint/[email protected]":
   version "3.10.1"
   resolved "https://registry.yarnpkg.com/@typescript-eslint/types/-/types-3.10.1.tgz#1d7463fa7c32d8a23ab508a803ca2fe26e758727"
@@ -3892,7 +3935,7 @@
     semver "^7.5.4"
     ts-api-utils "^1.0.1"
 
-"@typescript-eslint/utils@^6.0.0":
+"@typescript-eslint/utils@6.21.0", "@typescript-eslint/utils@^6.0.0":
   version "6.21.0"
   resolved "https://registry.yarnpkg.com/@typescript-eslint/utils/-/utils-6.21.0.tgz#4714e7a6b39e773c1c8e97ec587f520840cd8134"
   integrity sha512-NfWVaC8HP9T8cbKQxHcsJBY5YE1O33+jpMwN45qzWWaPDZgLIbo12toGMWnmhvCpd3sIxkpDw3Wv1B3dYrbDQQ==
@@ -9799,6 +9842,11 @@ ignore@^5.2.0:
   resolved "https://registry.yarnpkg.com/ignore/-/ignore-5.3.1.tgz#5073e554cd42c5b33b394375f538b8593e34d4ef"
   integrity sha512-5Fytz/IraMjqpwfd34ke28PTVMjZjJG2MPn5t7OE4eUCUNf8BAa7b5WUS9/Qvr6mwOQS7Mk6vdsMno5he+T8Xw==
 
+ignore@^5.2.4:
+  version "5.3.2"
+  resolved "https://registry.yarnpkg.com/ignore/-/ignore-5.3.2.tgz#3cd40e729f3643fd87cb04e50bf0eb722bc596f5"
+  integrity sha512-hsBTNUqQTDwkWtcdYI2i06Y/nUBEsNEDJKjWdigLvegy8kDuJAS8uRlpkkcQpyEXL0Z/pjDy5HBmMjRCJ2gq+g==
+
 [email protected]:
   version "1.1.1"
   resolved "https://registry.yarnpkg.com/image-size/-/image-size-1.1.1.tgz#ddd67d4dc340e52ac29ce5f546a09f4e29e840ac"