[Refactor] variableUtil: Avoid creating a single flat variable scop…

…e for each lookup
jsx-eslint · Jul 15, 2024 · 4d2fd86 · 4d2fd86
1 parent 6a83d67
commit 4d2fd86
Show file tree

Hide file tree

Showing 13 changed files with 39 additions and 51 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,6 +12,10 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
 * [`forbid-component-props`]: add `propNamePattern` to allow / disallow prop name patterns ([#3774][] @akulsr0)
 * [`jsx-handler-names`]: support ignoring component names ([#3772][] @akulsr0)
 
+### Changed
+* [Refactor] `variableUtil`: Avoid creating a single flat variable scope for each lookup ([#3782][] @DanielRosenwasser)
+
+[#3782]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3782
 [#3774]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3774
 [#3772]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3772
 [#3759]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3759

diff --git a/lib/rules/jsx-max-depth.js b/lib/rules/jsx-max-depth.js
@@ -88,7 +88,7 @@ module.exports = {
       });
     }
 
-    function findJSXElementOrFragment(variables, name, previousReferences) {
+    function findJSXElementOrFragment(startNode, name, previousReferences) {
       function find(refs, prevRefs) {
         for (let i = refs.length - 1; i >= 0; i--) {
           if (has(refs[i], 'writeExpr')) {
@@ -97,14 +97,14 @@ module.exports = {
             return (jsxUtil.isJSX(writeExpr)
               && writeExpr)
               || ((writeExpr && writeExpr.type === 'Identifier')
-              && findJSXElementOrFragment(variables, writeExpr.name, prevRefs));
+              && findJSXElementOrFragment(startNode, writeExpr.name, prevRefs));
           }
         }
 
         return null;
       }
 
-      const variable = variableUtil.getVariable(variables, name);
+      const variable = variableUtil.getVariableFromContext(context, startNode, name);
       if (variable && variable.references) {
         const containDuplicates = previousReferences.some((ref) => includes(variable.references, ref));
 
@@ -150,8 +150,7 @@ module.exports = {
           return;
         }
 
-        const variables = variableUtil.variablesInScope(context, node);
-        const element = findJSXElementOrFragment(variables, node.expression.name, []);
+        const element = findJSXElementOrFragment(node, node.expression.name, []);
 
         if (element) {
           const baseDepth = getDepth(node);

diff --git a/lib/rules/jsx-no-leaked-render.js b/lib/rules/jsx-no-leaked-render.js
@@ -161,8 +161,7 @@ module.exports = {
           if (isCoerceValidLeftSide || getIsCoerceValidNestedLogicalExpression(leftSide)) {
             return;
           }
-          const variables = variableUtil.variablesInScope(context, node);
-          const leftSideVar = variableUtil.getVariable(variables, leftSide.name);
+          const leftSideVar = variableUtil.getVariableFromContext(context, node, leftSide.name);
           if (leftSideVar) {
             const leftSideValue = leftSideVar.defs
               && leftSideVar.defs.length

diff --git a/lib/rules/jsx-sort-default-props.js b/lib/rules/jsx-sort-default-props.js
@@ -97,8 +97,7 @@ module.exports = {
      */
     function findVariableByName(node, name) {
       const variable = variableUtil
-        .variablesInScope(context, node)
-        .find((item) => item.name === name);
+        .getVariableFromContext(context, node, name);
 
       if (!variable || !variable.defs[0] || !variable.defs[0].node) {
         return null;

diff --git a/lib/rules/no-danger-with-children.js b/lib/rules/no-danger-with-children.js
@@ -32,8 +32,7 @@ module.exports = {
   },
   create(context) {
     function findSpreadVariable(node, name) {
-      return variableUtil.variablesInScope(context, node)
-        .find((item) => item.name === name);
+      return variableUtil.getVariableFromContext(context, node, name);
     }
     /**
      * Takes a ObjectExpression and returns the value of the prop if it has it
@@ -128,8 +127,7 @@ module.exports = {
           let props = node.arguments[1];
 
           if (props.type === 'Identifier') {
-            const variable = variableUtil.variablesInScope(context, node)
-              .find((item) => item.name === props.name);
+            const variable = variableUtil.getVariableFromContext(context, node, props.name);
             if (variable && variable.defs.length && variable.defs[0].node.init) {
               props = variable.defs[0].node.init;
             }

diff --git a/lib/rules/react-in-jsx-scope.js b/lib/rules/react-in-jsx-scope.js
@@ -37,8 +37,7 @@ module.exports = {
     const pragma = pragmaUtil.getFromContext(context);
 
     function checkIfReactIsInScope(node) {
-      const variables = variableUtil.variablesInScope(context, node);
-      if (variableUtil.findVariable(variables, pragma)) {
+      if (variableUtil.getVariableFromContext(context, node, pragma)) {
         return;
       }
       report(context, messages.notInScope, 'notInScope', {

diff --git a/lib/rules/sort-default-props.js b/lib/rules/sort-default-props.js
@@ -91,8 +91,7 @@ module.exports = {
      * @returns {ASTNode|null} Return null if the variable could not be found, ASTNode otherwise.
      */
     function findVariableByName(node, name) {
-      const variable = variableUtil.variablesInScope(context, node)
-        .find((item) => item.name === name);
+      const variable = variableUtil.getVariableFromContext(context, node, name);
 
       if (!variable || !variable.defs[0] || !variable.defs[0].node) {
         return null;

diff --git a/lib/rules/style-prop-object.js b/lib/rules/style-prop-object.js
@@ -61,8 +61,7 @@ module.exports = {
      * @param {object} node A Identifier node
      */
     function checkIdentifiers(node) {
-      const variable = variableUtil.variablesInScope(context, node)
-        .find((item) => item.name === node.name);
+      const variable = variableUtil.getVariableFromContext(context, node, node.name);
 
       if (!variable || !variable.defs[0] || !variable.defs[0].node.init) {
         return;

diff --git a/lib/util/Components.js b/lib/util/Components.js
@@ -669,14 +669,7 @@ function componentRule(rule, context) {
       if (!variableName) {
         return null;
       }
-      let variableInScope;
-      const variables = variableUtil.variablesInScope(context, node);
-      for (i = 0, j = variables.length; i < j; i++) {
-        if (variables[i].name === variableName) {
-          variableInScope = variables[i];
-          break;
-        }
-      }
+      const variableInScope = variableUtil.getVariableFromContext(context, node, variableName);
       if (!variableInScope) {
         return null;
       }

diff --git a/lib/util/isDestructuredFromPragmaImport.js b/lib/util/isDestructuredFromPragmaImport.js
@@ -13,8 +13,7 @@ const variableUtil = require('./variable');
  */
 module.exports = function isDestructuredFromPragmaImport(context, node, variable) {
   const pragma = pragmaUtil.getFromContext(context);
-  const variables = variableUtil.variablesInScope(context, node);
-  const variableInScope = variableUtil.getVariable(variables, variable);
+  const variableInScope = variableUtil.getVariableFromContext(context, node, variable);
   if (variableInScope) {
     const latestDef = variableUtil.getLatestVariableDefinition(variableInScope);
     if (latestDef) {

diff --git a/lib/util/propTypes.js b/lib/util/propTypes.js
@@ -965,8 +965,7 @@ module.exports = function propTypesInstructions(context, components, utils) {
         break;
       }
       case 'Identifier': {
-        const firstMatchingVariable = variableUtil.variablesInScope(context, node)
-          .find((variableInScope) => variableInScope.name === propTypes.name);
+        const firstMatchingVariable = variableUtil.getVariableFromContext(context, node, propTypes.name);
         if (firstMatchingVariable) {
           const defInScope = firstMatchingVariable.defs[firstMatchingVariable.defs.length - 1];
           markPropTypesAsDeclared(node, defInScope.node && defInScope.node.init, rootNode);

diff --git a/lib/util/variable.js b/lib/util/variable.js
@@ -5,7 +5,6 @@
 
 'use strict';
 
-const toReversed = require('array.prototype.toreversed');
 const getScope = require('./eslint').getScope;
 
 /**
@@ -29,30 +28,33 @@ function getVariable(variables, name) {
 }
 
 /**
- * List all variable in a given scope
- *
- * Contain a patch for babel-eslint to avoid https://github.com/babel/babel-eslint/issues/21
+ * Searches for a variable in the given scope.
  *
  * @param {Object} context The current rule context.
  * @param {ASTNode} node The node to start looking from.
- * @returns {Array} The variables list
+ * @param {string} name The name of the variable to search.
+ * @returns {Object | undefined} Variable if the variable was found, undefined if not.
  */
-function variablesInScope(context, node) {
+function getVariableFromContext(context, node, name) {
   let scope = getScope(context, node);
-  let variables = scope.variables;
 
-  while (scope.type !== 'global') {
-    scope = scope.upper;
-    variables = scope.variables.concat(variables);
-  }
-  if (scope.childScopes.length) {
-    variables = scope.childScopes[0].variables.concat(variables);
-    if (scope.childScopes[0].childScopes.length) {
-      variables = scope.childScopes[0].childScopes[0].variables.concat(variables);
+  while (scope) {
+    let variable = getVariable(scope.variables, name);
+
+    if (!variable && scope.childScopes.length) {
+      variable = getVariable(scope.childScopes[0].variables, name);
+
+      if (!variable && scope.childScopes[0].childScopes.length) {
+        variable = getVariable(scope.childScopes[0].childScopes[0].variables, name);
+      }
     }
-  }
 
-  return toReversed(variables);
+    if (variable) {
+      return variable;
+    }
+    scope = scope.upper;
+  }
+  return undefined;
 }
 
 /**
@@ -63,7 +65,7 @@ function variablesInScope(context, node) {
  * @returns {ASTNode|null} Return null if the variable could not be found, ASTNode otherwise.
  */
 function findVariableByName(context, node, name) {
-  const variable = getVariable(variablesInScope(context, node), name);
+  const variable = getVariableFromContext(context, node, name);
 
   if (!variable || !variable.defs[0] || !variable.defs[0].node) {
     return null;
@@ -93,6 +95,6 @@ module.exports = {
   findVariable,
   findVariableByName,
   getVariable,
-  variablesInScope,
+  getVariableFromContext,
   getLatestVariableDefinition,
 };
diff --git a/package.json b/package.json
@@ -28,7 +28,6 @@
     "array-includes": "^3.1.8",
     "array.prototype.findlast": "^1.2.5",
     "array.prototype.flatmap": "^1.3.2",
-    "array.prototype.toreversed": "^1.1.2",
     "array.prototype.tosorted": "^1.1.4",
     "doctrine": "^2.1.0",
     "es-iterator-helpers": "^1.0.19",