Repository: yannickcr/eslint-plugin-react
Pull request #3265 (2022-04-06 01:38:09)

[Refactor]: improve performance for detecting function components
Benchmark on a private repo with `time npx eslint .` (real time):

|  | before | after |
| --- | -- | -- |
| run 1 | 6.6s | 4.8s |
| run 2 | 6.5s | 4.7s |
| run 3 | 6.7s | 4.8s |

Benchmark with `TIMING=10 npx eslint .`:

Before:
```
Rule                                | Time (ms) | Relative
:-----------------------------------|----------:|--------:
react/destructuring-assignment      |   345.923 |     7.0%
react/no-unstable-nested-components |   208.838 |     4.2%
react/no-did-mount-set-state        |   189.097 |     3.8%
react/boolean-prop-naming           |   187.663 |     3.8%
react/no-direct-mutation-state      |   186.664 |     3.8%
react/no-deprecated                 |   184.020 |     3.7%
react/hook-use-state                |   146.379 |     3.0%
react/no-access-state-in-setstate   |   145.210 |     3.0%
react/prefer-stateless-function     |   133.564 |     2.7%
react/require-optimization          |   128.301 |     2.6%
```

After:
```
Rule                                | Time (ms) | Relative
:-----------------------------------|----------:|--------:
react/no-did-mount-set-state        |   181.362 |     6.2%
react/destructuring-assignment      |   162.586 |     5.5%
react/no-deprecated                 |   134.652 |     4.6%
react/no-direct-mutation-state      |   124.110 |     4.2%
react/boolean-prop-naming           |   116.978 |     4.0%
react/no-did-update-set-state       |   113.393 |     3.9%
react/no-unknown-property           |    92.802 |     3.2%
react/hook-use-state                |    91.712 |     3.1%
react/no-unstable-nested-components |    90.477 |     3.1%
react/no-access-state-in-setstate   |    87.266 |     3.0%
```


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9698e30f..9c8a8322 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 b1c3c130..8001b416 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 e7ae2357..46930f0b 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;


Test patch:
diff --git a/tests/util/ast.js b/tests/util/ast.js
index 29fcbfea..607a90a9 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);
       });
     });
   });