Remove match utility and cleanup code (#744)

Author: danez

.changeset/lazy-lions-dress.md

@@ -0,0 +1,9 @@
+---
+'react-docgen': major
+---
+
+Remove match utility.
+
+The utility can be replaced by babel helpers and is not needed anymore. Also
+using explicit checks like `path.isMemberExpression()` is better for type safety
+and catching potential bugs.

packages/react-docgen/src/handlers/componentMethodsHandler.ts

@@ -66,8 +66,7 @@ const explodedVisitors = visitors.explode<TraverseState>({
       if (
         binding &&
         left.isMemberExpression() &&
-        left.get('object').isIdentifier() &&
-        (left.node.object as Identifier).name === name &&
+        left.get('object').isIdentifier({ name }) &&
         binding.scope === scope &&
         resolveToValue(assignmentPath.get('right')).isFunction()
       ) {

packages/react-docgen/src/importer/makeFsImporter.ts

@@ -4,7 +4,7 @@ import { dirname, extname } from 'path';
 import fs from 'fs';
 import type { NodePath } from '@babel/traverse';
 import { visitors } from '@babel/traverse';
-import type { ExportSpecifier, Identifier, ObjectProperty } from '@babel/types';
+import type { ExportSpecifier, ObjectProperty } from '@babel/types';
 import type { Importer, ImportPath } from './index.js';
 import type FileState from '../FileState.js';
 import { resolveObjectPatternPropertyToValue } from '../utils/index.js';
@@ -164,7 +164,7 @@ export default function makeFsImporter(
             const id = declPath.get('id');
             const init = declPath.get('init');
 
-            if (id.isIdentifier() && id.node.name === name && init.hasNode()) {
+            if (id.isIdentifier({ name }) && init.hasNode()) {
               // export const/var a = <init>
 
               state.resultPath = init;
@@ -177,7 +177,7 @@ export default function makeFsImporter(
                 if (prop.isObjectProperty()) {
                   const value = prop.get('value');
 
-                  return value.isIdentifier() && value.node.name === name;
+                  return value.isIdentifier({ name });
                 }
                 // We don't handle RestElement here yet as complicated
 
@@ -197,8 +197,7 @@ export default function makeFsImporter(
         } else if (
           declaration.hasNode() &&
           declaration.has('id') &&
-          (declaration.get('id') as NodePath).isIdentifier() &&
-          (declaration.get('id') as NodePath<Identifier>).node.name === name
+          (declaration.get('id') as NodePath).isIdentifier({ name })
         ) {
           // export function/class/type/interface/enum ...
 
@@ -212,7 +211,7 @@ export default function makeFsImporter(
             }
             const exported = specifierPath.get('exported');
 
-            if (exported.isIdentifier() && exported.node.name === name) {
+            if (exported.isIdentifier({ name })) {
               // export ... from ''
               if (path.has('source')) {
                 const local = specifierPath.isExportSpecifier()

packages/react-docgen/src/utils/__tests__/isReactChildrenElementCall-test.ts

@@ -0,0 +1,80 @@
+import { parse } from '../../../tests/utils';
+import isReactChildrenElementCall from '../isReactChildrenElementCall.js';
+import { describe, expect, test } from 'vitest';
+
+describe('isReactChildrenElementCall', () => {
+  describe('true', () => {
+    test('React.Children.map', () => {
+      const def = parse.expressionLast(`
+        var React = require("React");
+        React.Children.map(() => {});
+      `);
+
+      expect(isReactChildrenElementCall(def)).toBe(true);
+    });
+
+    test('React.Children.only', () => {
+      const def = parse.expressionLast(`
+        var React = require("React");
+        React.Children.only(() => {});
+      `);
+
+      expect(isReactChildrenElementCall(def)).toBe(true);
+    });
+  });
+  describe('false', () => {
+    test('not call expression', () => {
+      const def = parse.expressionLast(`
+        var React = require("React");
+        React.Children.map;
+      `);
+
+      expect(isReactChildrenElementCall(def)).toBe(false);
+    });
+
+    test('not MemberExpression', () => {
+      const def = parse.expressionLast(`
+        var React = require("React");
+        map();
+      `);
+
+      expect(isReactChildrenElementCall(def)).toBe(false);
+    });
+
+    test('not only or map', () => {
+      const def = parse.expressionLast(`
+        var React = require("React");
+        React.Children.abc(() => {});
+      `);
+
+      expect(isReactChildrenElementCall(def)).toBe(false);
+    });
+
+    test('not double MemberExpression', () => {
+      const def = parse.expressionLast(`
+        var React = require("React");
+        Children.map(() => {});
+      `);
+
+      expect(isReactChildrenElementCall(def)).toBe(false);
+    });
+
+    test('not Children', () => {
+      const def = parse.expressionLast(`
+        var React = require("React");
+        React.Parent.map(() => {});
+      `);
+
+      expect(isReactChildrenElementCall(def)).toBe(false);
+    });
+
+    test('not react module', () => {
+      const def = parse.expressionLast(`
+        var React = require("test");
+        React.Children.map(() => {});
+      `);
+
+      expect(isReactChildrenElementCall(def)).toBe(false);
+    });
+  });
+});

packages/react-docgen/src/utils/__tests__/match-test.ts

@@ -124,11 +124,7 @@ function handleGenericTypeAnnotation(
   const id = path.get('id');
   const typeParameters = path.get('typeParameters');
 
-  if (
-    id.isIdentifier() &&
-    id.node.name === '$Keys' &&
-    typeParameters.hasNode()
-  ) {
+  if (id.isIdentifier({ name: '$Keys' }) && typeParameters.hasNode()) {
     return handleKeysHelper(path);
   }
 
@@ -137,7 +133,7 @@ function handleGenericTypeAnnotation(
   if (id.isQualifiedTypeIdentifier()) {
     const qualification = id.get('qualification');
 
-    if (qualification.isIdentifier() && qualification.node.name === 'React') {
+    if (qualification.isIdentifier({ name: 'React' })) {
       type = {
         name: `${qualification.node.name}${id.node.id.name}`,
         raw: printValue(id),
@@ -391,7 +387,7 @@ function getFlowTypeWithResolvedTypes(
 
   const isTypeAlias = parent.isTypeAlias();
 
-  // When we see a typealias mark it as visited so that the next
+  // When we see a TypeAlias mark it as visited so that the next
   // call of this function does not run into an endless loop
   if (isTypeAlias) {
     if (visitedTypes[parent.node.id.name] === true) {

packages/react-docgen/src/utils/getFlowType.ts

@@ -90,11 +90,7 @@ function handleTSTypeReference(
     const left = typeName.get('left');
     const right = typeName.get('right');
 
-    if (
-      left.isIdentifier() &&
-      left.node.name === 'React' &&
-      right.isIdentifier()
-    ) {
+    if (left.isIdentifier({ name: 'React' }) && right.isIdentifier()) {
       type = {
         name: `${left.node.name}${right.node.name}`,
         raw: printValue(typeName),

packages/react-docgen/src/utils/getTSType.ts

@@ -48,7 +48,6 @@ export { default as isReactModuleName } from './isReactModuleName.js';
 export { default as isRequiredPropType } from './isRequiredPropType.js';
 export { default as isStatelessComponent } from './isStatelessComponent.js';
 export { default as isUnreachableFlowType } from './isUnreachableFlowType.js';
-export { default as match } from './match.js';
 export { default as normalizeClassDefinition } from './normalizeClassDefinition.js';
 export { default as parseJsDoc } from './parseJsDoc.js';
 export { default as postProcessDocumentation } from './postProcessDocumentation.js';

packages/react-docgen/src/utils/index.ts

@@ -14,9 +14,5 @@ export default function isDestructuringAssignment(
 
   const id = path.get('key');
 
-  return (
-    id.isIdentifier() &&
-    id.node.name === name &&
-    path.parentPath.isObjectPattern()
-  );
+  return id.isIdentifier({ name }) && path.parentPath.isObjectPattern();
 }

packages/react-docgen/src/utils/isDestructuringAssignment.ts

@@ -1,10 +1,9 @@
 import isReactModuleName from './isReactModuleName.js';
-import match from './match.js';
 import resolveToModule from './resolveToModule.js';
 import resolveToValue from './resolveToValue.js';
 import isDestructuringAssignment from './isDestructuringAssignment.js';
 import type { NodePath } from '@babel/traverse';
-import type { CallExpression, MemberExpression } from '@babel/types';
+import type { CallExpression } from '@babel/types';
 
 function isNamedMemberExpression(value: NodePath, name: string): boolean {
   if (!value.isMemberExpression()) {
@@ -33,8 +32,8 @@ function isNamedImportDeclaration(
     const local = specifier.get('local');
 
     return (
-      ((imported.isIdentifier() && imported.node.name === name) ||
-        (imported.isStringLiteral() && imported.node.value === name)) &&
+      (imported.isIdentifier({ name }) ||
+        imported.isStringLiteral({ value: name })) &&
       local.node.name === callee.node.name
     );
   });
@@ -53,17 +52,20 @@ export default function isReactBuiltinCall(
   }
 
   if (path.isCallExpression()) {
-    if (match(path.node, { callee: { property: { name } } })) {
-      const module = resolveToModule(
-        (path.get('callee') as NodePath<MemberExpression>).get('object'),
-      );
+    const callee = path.get('callee');
+
+    if (
+      callee.isMemberExpression() &&
+      callee.get('property').isIdentifier({ name })
+    ) {
+      const module = resolveToModule(callee.get('object'));
 
       return Boolean(module && isReactModuleName(module));
     }
 
-    const value = resolveToValue(path.get('callee'));
+    const value = resolveToValue(callee);
 
-    if (value === path.get('callee')) {
+    if (value === callee) {
       return false;
     }
 
@@ -73,7 +75,7 @@ export default function isReactBuiltinCall(
       // `require('react').createElement`
       isNamedMemberExpression(value, name) ||
       // `import { createElement } from 'react'`
-      isNamedImportDeclaration(value, path.get('callee'), name)
+      isNamedImportDeclaration(value, callee, name)
     ) {
       const module = resolveToModule(value);
 

packages/react-docgen/src/utils/isReactBuiltinCall.ts

@@ -1,32 +1,36 @@
 import type { NodePath } from '@babel/traverse';
-import type { MemberExpression } from '@babel/types';
 import isReactModuleName from './isReactModuleName.js';
-import match from './match.js';
 import resolveToModule from './resolveToModule.js';
 
-// TODO unit tests
-
 /**
  * Returns true if the expression is a function call of the form
- * `React.Children.only(...)`.
+ * `React.Children.only(...)` or `React.Children.map(...)`.
  */
 export default function isReactChildrenElementCall(path: NodePath): boolean {
   if (path.isExpressionStatement()) {
     path = path.get('expression');
   }
 
+  if (!path.isCallExpression()) {
+    return false;
+  }
+
+  const callee = path.get('callee');
+
   if (
-    !match(path.node, { callee: { property: { name: 'only' } } }) &&
-    !match(path.node, { callee: { property: { name: 'map' } } })
+    !callee.isMemberExpression() ||
+    (!callee.get('property').isIdentifier({ name: 'only' }) &&
+      !callee.get('property').isIdentifier({ name: 'map' }))
   ) {
     return false;
   }
 
-  const calleeObj = (path.get('callee') as NodePath<MemberExpression>).get(
-    'object',
-  );
+  const calleeObj = callee.get('object');
 
-  if (!match(calleeObj.node, { property: { name: 'Children' } })) {
+  if (
+    !calleeObj.isMemberExpression() ||
+    !calleeObj.get('property').isIdentifier({ name: 'Children' })
+  ) {
     return false;
   }
 

packages/react-docgen/src/utils/isReactChildrenElementCall.ts

@@ -7,9 +7,7 @@ import getMembers from '../utils/getMembers.js';
 export default function isRequiredPropType(path: NodePath<Node>): boolean {
   return getMembers(path).some(
     ({ computed, path: memberPath }) =>
-      (!computed &&
-        memberPath.isIdentifier() &&
-        memberPath.node.name === 'isRequired') ||
-      (memberPath.isStringLiteral() && memberPath.node.value === 'isRequired'),
+      (!computed && memberPath.isIdentifier({ name: 'isRequired' })) ||
+      memberPath.isStringLiteral({ value: 'isRequired' }),
   );
 }