diff --git a/CHANGELOG.md b/CHANGELOG.md index 9698e30f33..9c8a8322f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,9 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange * [readme] remove global usage and eslint version from readme ([#3254][] @aladdin-add) * [Refactor] fix linter errors ([#3261][] @golopot) * [Docs] [`no-unused-prop-types`]: fix syntax errors ([#3259][] @mrdulin) +* [Refactor] improve performance for detecting function components ([#3265][] @golopot) +[#3265]: https://github.com/yannickcr/eslint-plugin-react/pull/3265 [#3261]: https://github.com/yannickcr/eslint-plugin-react/pull/3261 [#3260]: https://github.com/yannickcr/eslint-plugin-react/pull/3260 [#3259]: https://github.com/yannickcr/eslint-plugin-react/pull/3259 diff --git a/lib/util/ast.js b/lib/util/ast.js index b1c3c130da..8001b4161f 100644 --- a/lib/util/ast.js +++ b/lib/util/ast.js @@ -62,24 +62,28 @@ function findReturnStatement(node) { return loopNodes(bodyNodes); } +// eslint-disable-next-line valid-jsdoc -- valid-jsdoc cannot parse function types. /** * Helper function for traversing "returns" (return statements or the * returned expression in the case of an arrow function) of a function * * @param {ASTNode} ASTNode The AST node being checked * @param {Context} context The context of `ASTNode`. - * @param {function} enterFunc Function to execute for each returnStatement found + * @param {(returnValue: ASTNode, breakTraverse: () => void) => void} onReturn + * Function to execute for each returnStatement found * @returns {undefined} */ -function traverseReturns(ASTNode, context, enterFunc) { +function traverseReturns(ASTNode, context, onReturn) { const nodeType = ASTNode.type; if (nodeType === 'ReturnStatement') { - return enterFunc(ASTNode); + onReturn(ASTNode.argument, () => {}); + return; } if (nodeType === 'ArrowFunctionExpression' && ASTNode.expression) { - return enterFunc(ASTNode.body); + onReturn(ASTNode.body, () => {}); + return; } /* TODO: properly warn on React.forwardRefs having typo properties @@ -111,15 +115,23 @@ function traverseReturns(ASTNode, context, enterFunc) { traverse(ASTNode.body, { enter(node) { + const breakTraverse = () => { + this.break(); + }; switch (node.type) { case 'ReturnStatement': this.skip(); - return enterFunc(node); - case 'FunctionExpression': - case 'FunctionDeclaration': - case 'ArrowFunctionExpression': - return this.skip(); + onReturn(node.argument, breakTraverse); + return; + case 'BlockStatement': + case 'IfStatement': + case 'ForStatement': + case 'WhileStatement': + case 'SwitchStatement': + case 'SwitchCase': + return; default: + this.skip(); } }, }); diff --git a/lib/util/jsx.js b/lib/util/jsx.js index e7ae2357d7..46930f0b9d 100644 --- a/lib/util/jsx.js +++ b/lib/util/jsx.js @@ -4,7 +4,6 @@ 'use strict'; -const estraverse = require('estraverse'); const elementType = require('jsx-ast-utils/elementType'); const astUtil = require('./ast'); @@ -95,63 +94,48 @@ function isWhiteSpaces(value) { * @returns {Boolean} True if the node is returning JSX or null, false if not */ function isReturningJSX(isCreateElement, ASTnode, context, strict, ignoreNull) { - let found = false; - astUtil.traverseReturns(ASTnode, context, (node) => { - // Traverse return statement - astUtil.traverse(node, { - enter(childNode) { - const setFound = () => { - found = true; - this.skip(); - }; - switch (childNode.type) { - case 'FunctionExpression': - case 'FunctionDeclaration': - case 'ArrowFunctionExpression': - // Do not traverse into inner function definitions - return this.skip(); - case 'ConditionalExpression': - if (!strict) break; - if (isJSX(childNode.consequent) && isJSX(childNode.alternate)) { - setFound(); - } - this.skip(); - break; - case 'LogicalExpression': - if (!strict) break; - if (isJSX(childNode.left) && isJSX(childNode.right)) { - setFound(); - } - this.skip(); - break; - case 'JSXElement': - case 'JSXFragment': - setFound(); - break; - case 'CallExpression': - if (isCreateElement(childNode)) { - setFound(); - } - this.skip(); - break; - case 'Literal': - if (!ignoreNull && childNode.value === null) { - setFound(); - } - break; - case 'Identifier': { - const variable = variableUtil.findVariableByName(context, childNode.name); - if (isJSX(variable)) { - setFound(); - } - break; - } - default: + const isJSXValue = (node) => { + if (!node) { + return false; + } + switch (node.type) { + case 'ConditionalExpression': + if (strict) { + return isJSXValue(node.consequent) && isJSXValue(node.alternate); } - }, - }); + return isJSXValue(node.consequent) || isJSXValue(node.alternate); + case 'LogicalExpression': + if (strict) { + return isJSXValue(node.left) && isJSXValue(node.right); + } + return isJSXValue(node.left) || isJSXValue(node.right); + case 'SequenceExpression': + return isJSXValue(node.expressions[node.expressions.length - 1]); + case 'JSXElement': + case 'JSXFragment': + return true; + case 'CallExpression': + return isCreateElement(node); + case 'Literal': + if (!ignoreNull && node.value === null) { + return true; + } + return false; + case 'Identifier': { + const variable = variableUtil.findVariableByName(context, node.name); + return isJSX(variable); + } + default: + return false; + } + }; - return found && estraverse.VisitorOption.Break; + let found = false; + astUtil.traverseReturns(ASTnode, context, (node, breakTraverse) => { + if (isJSXValue(node)) { + found = true; + breakTraverse(); + } }); return found; diff --git a/tests/util/ast.js b/tests/util/ast.js index 29fcbfeaa3..607a90a99b 100644 --- a/tests/util/ast.js +++ b/tests/util/ast.js @@ -96,8 +96,8 @@ describe('ast', () => { assert.strictEqual(enterCalls.length, 6); - enterCalls.forEach((node, idx) => { - assert.strictEqual(node.lastArg.argument.value, idx); + enterCalls.forEach((call, idx) => { + assert.strictEqual(call.args[0].value, idx); }); }); });