Skip to content

Commit

Permalink
[Refactor] improve performance for detecting function components
Browse files Browse the repository at this point in the history
  • Loading branch information
golopot authored and ljharb committed Apr 6, 2022
1 parent c297a96 commit 365849c
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 67 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -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
Expand Down
30 changes: 21 additions & 9 deletions lib/util/ast.js
Expand Up @@ -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
Expand Down Expand Up @@ -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();
}
},
});
Expand Down
96 changes: 40 additions & 56 deletions lib/util/jsx.js
Expand Up @@ -4,7 +4,6 @@

'use strict';

const estraverse = require('estraverse');
const elementType = require('jsx-ast-utils/elementType');

const astUtil = require('./ast');
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions tests/util/ast.js
Expand Up @@ -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);
});
});
});
Expand Down

0 comments on commit 365849c

Please sign in to comment.