From 4d2fd861d2f9e561144c5926cc8b73fe7fff7f3e Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Mon, 15 Jul 2024 10:46:00 -0700 Subject: [PATCH] [Refactor] `variableUtil`: Avoid creating a single flat variable scope for each lookup --- CHANGELOG.md | 4 +++ lib/rules/jsx-max-depth.js | 9 +++-- lib/rules/jsx-no-leaked-render.js | 3 +- lib/rules/jsx-sort-default-props.js | 3 +- lib/rules/no-danger-with-children.js | 6 ++-- lib/rules/react-in-jsx-scope.js | 3 +- lib/rules/sort-default-props.js | 3 +- lib/rules/style-prop-object.js | 3 +- lib/util/Components.js | 9 +---- lib/util/isDestructuredFromPragmaImport.js | 3 +- lib/util/propTypes.js | 3 +- lib/util/variable.js | 40 ++++++++++++---------- package.json | 1 - 13 files changed, 39 insertions(+), 51 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d138fc53d6..b6aed61a3f 100644 --- 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 index be0f582264..12206a4081 100644 --- 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 index f7eea1ad52..71c44e7b29 100644 --- 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 index 89b7bb64cc..379d80677c 100644 --- 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 index d3508721fe..9f07d0c535 100644 --- 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 index f2d8bc4837..097e647764 100644 --- 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 index aca868f95d..2101307e00 100644 --- 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 index 4f77beccb2..b0fd7f76ce 100644 --- 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 index 67fac5afc5..0ef615a133 100644 --- 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 index 4d64596274..6abf86e84c 100644 --- 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 index 84f5479f15..9450f9222f 100644 --- 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 index 2903c67344..c0017533be 100644 --- 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 index 2938ae22a1..1cd54c514c 100644 --- 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",