resolve TODOS and fix handling of some TypeScript typeof types (#747)

danez · web-flow · commit 1aa0249f2937 · 2023-01-28T23:35:39.000Z
diff --git a/.changeset/smart-files-tell.md b/.changeset/smart-files-tell.md
@@ -0,0 +1,6 @@
+---
+'react-docgen': patch
+---
+
+Handle `typeof import('...')` and `typeof MyType.property` correctly in
+TypeScript
diff --git a/packages/react-docgen/src/handlers/codeTypeHandler.ts b/packages/react-docgen/src/handlers/codeTypeHandler.ts
@@ -37,7 +37,6 @@ function setPropDescriptor(
       return;
     }
 
-    // TODO what about other types here
     const id = argument.get('id') as NodePath;
 
     if (!id.hasNode() || !id.isIdentifier()) {
diff --git a/packages/react-docgen/src/utils/__tests__/__snapshots__/getTSType-test.ts.snap b/packages/react-docgen/src/utils/__tests__/__snapshots__/getTSType-test.ts.snap
@@ -835,7 +835,13 @@ exports[`getTSType > handles self-referencing type cycles 1`] = `
 }
 `;
 
-exports[`getTSType > handles typeof types 1`] = `
+exports[`getTSType > handles typeof qualified type 1`] = `
+{
+  "name": "MyType.a",
+}
+`;
+
+exports[`getTSType > handles typeof type 1`] = `
 {
   "name": "signature",
   "raw": "{ a: string, b: xyz }",
@@ -1385,7 +1391,13 @@ exports[`getTSType > resolves keyof with inline object to union 1`] = `
 }
 `;
 
-exports[`getTSType > resolves typeof of imported types 1`] = `
+exports[`getTSType > resolves typeof of import type 1`] = `
+{
+  "name": "import('MyType')",
+}
+`;
+
+exports[`getTSType > resolves typeof of imported type 1`] = `
 {
   "name": "signature",
   "raw": "{ a: number, b: xyz }",
diff --git a/packages/react-docgen/src/utils/__tests__/__snapshots__/getTypeParameters-test.ts.snap b/packages/react-docgen/src/utils/__tests__/__snapshots__/getTypeParameters-test.ts.snap
@@ -0,0 +1,41 @@
+// Vitest Snapshot v1
+
+exports[`getTypeParameters > Flow > detects simple type 1`] = `
+{
+  "T": Node {
+    "id": Node {
+      "name": "T",
+      "type": "Identifier",
+    },
+    "type": "GenericTypeAnnotation",
+    "typeParameters": null,
+  },
+}
+`;
+
+exports[`getTypeParameters > TypeScript > detects default 1`] = `
+{
+  "R": Node {
+    "type": "TSStringKeyword",
+  },
+  "T": Node {
+    "type": "TSTypeReference",
+    "typeName": Node {
+      "name": "T",
+      "type": "Identifier",
+    },
+  },
+}
+`;
+
+exports[`getTypeParameters > TypeScript > detects simple type 1`] = `
+{
+  "T": Node {
+    "type": "TSTypeReference",
+    "typeName": Node {
+      "name": "T",
+      "type": "Identifier",
+    },
+  },
+}
+`;
diff --git a/packages/react-docgen/src/utils/__tests__/getTSType-test.ts b/packages/react-docgen/src/utils/__tests__/getTSType-test.ts
@@ -461,7 +461,7 @@ describe('getTSType', () => {
     expect(getTSType(typePath)).toMatchSnapshot();
   });
 
-  test('handles typeof types', () => {
+  test('handles typeof type', () => {
     const typePath = typeAlias(`
       var x: typeof MyType = {};
 
@@ -471,7 +471,17 @@ describe('getTSType', () => {
     expect(getTSType(typePath)).toMatchSnapshot();
   });
 
-  test('resolves typeof of imported types', () => {
+  test('handles typeof qualified type', () => {
+    const typePath = typeAlias(`
+      var x: typeof MyType.a = {};
+
+      type MyType = { a: string, b: xyz };
+    `);
+
+    expect(getTSType(typePath)).toMatchSnapshot();
+  });
+
+  test('resolves typeof of imported type', () => {
     const typePath = typeAlias(
       `
       var x: typeof MyType = {};
@@ -483,6 +493,15 @@ describe('getTSType', () => {
     expect(getTSType(typePath)).toMatchSnapshot();
   });
 
+  test('resolves typeof of import type', () => {
+    const typePath = typeAlias(
+      "var x: typeof import('MyType') = {};",
+      mockImporter,
+    );
+
+    expect(getTSType(typePath)).toMatchSnapshot();
+  });
+
   test('handles qualified type identifiers', () => {
     const typePath = typeAlias(`
       var x: MyType.x = {};
diff --git a/packages/react-docgen/src/utils/__tests__/getTypeParameters-test.ts b/packages/react-docgen/src/utils/__tests__/getTypeParameters-test.ts
@@ -0,0 +1,61 @@
+import type {
+  TSTypeAliasDeclaration,
+  TSTypeParameterDeclaration,
+  TSTypeParameterInstantiation,
+  TypeAlias,
+  TypeParameterDeclaration,
+  TypeParameterInstantiation,
+} from '@babel/types';
+import { parse, parseTypescript } from '../../../tests/utils';
+import getTypeParameters from '../getTypeParameters.js';
+import { describe, expect, test } from 'vitest';
+import type { NodePath } from '@babel/traverse';
+
+describe('getTypeParameters', () => {
+  describe('TypeScript', () => {
+    test('detects simple type', () => {
+      const path =
+        parseTypescript.statement<TSTypeAliasDeclaration>('type x<T> = y<T>');
+
+      expect(
+        getTypeParameters(
+          path.get('typeParameters') as NodePath<TSTypeParameterDeclaration>,
+          path
+            .get('typeAnnotation')
+            .get('typeParameters') as NodePath<TSTypeParameterInstantiation>,
+          null,
+        ),
+      ).toMatchSnapshot();
+    });
+    test('detects default', () => {
+      const path = parseTypescript.statement<TSTypeAliasDeclaration>(
+        'type x<T, R = string> = y<T>;',
+      );
+
+      expect(
+        getTypeParameters(
+          path.get('typeParameters') as NodePath<TSTypeParameterDeclaration>,
+          path
+            .get('typeAnnotation')
+            .get('typeParameters') as NodePath<TSTypeParameterInstantiation>,
+          null,
+        ),
+      ).toMatchSnapshot();
+    });
+  });
+  describe('Flow', () => {
+    test('detects simple type', () => {
+      const path = parse.statement<TypeAlias>('type x<T> = y<T>');
+
+      expect(
+        getTypeParameters(
+          path.get('typeParameters') as NodePath<TypeParameterDeclaration>,
+          path
+            .get('right')
+            .get('typeParameters') as NodePath<TypeParameterInstantiation>,
+          null,
+        ),
+      ).toMatchSnapshot();
+    });
+  });
+});
diff --git a/packages/react-docgen/src/utils/getTSType.ts b/packages/react-docgen/src/utils/getTSType.ts
@@ -35,6 +35,7 @@ import type {
   TSTypeParameterDeclaration,
   RestElement,
   TypeScript,
+  TSQualifiedName,
 } from '@babel/types';
 import { getDocblock } from './docblock.js';
 
@@ -68,6 +69,22 @@ const namedTypes = {
   TSIndexedAccessType: handleTSIndexedAccessType,
 };
 
+function handleTSQualifiedName(
+  path: NodePath<TSQualifiedName>,
+): TypeDescriptor<TSFunctionSignatureType> {
+  const left = path.get('left');
+  const right = path.get('right');
+
+  if (left.isIdentifier({ name: 'React' }) && right.isIdentifier()) {
+    return {
+      name: `${left.node.name}${right.node.name}`,
+      raw: printValue(path),
+    };
+  }
+
+  return { name: printValue(path).replace(/<.*>$/, '') };
+}
+
 function handleTSArrayType(
   path: NodePath<TSArrayType>,
   typeParams: TypeParameters | null,
@@ -87,17 +104,7 @@ function handleTSTypeReference(
   const typeName = path.get('typeName');
 
   if (typeName.isTSQualifiedName()) {
-    const left = typeName.get('left');
-    const right = typeName.get('right');
-
-    if (left.isIdentifier({ name: 'React' }) && right.isIdentifier()) {
-      type = {
-        name: `${left.node.name}${right.node.name}`,
-        raw: printValue(typeName),
-      };
-    } else {
-      type = { name: printValue(typeName).replace(/<.*>$/, '') };
-    }
+    type = handleTSQualifiedName(typeName);
   } else {
     type = { name: (typeName as NodePath<Identifier>).node.name };
   }
@@ -366,17 +373,25 @@ function handleTSTypeQuery(
   path: NodePath<TSTypeQuery>,
   typeParams: TypeParameters | null,
 ): TypeDescriptor<TSFunctionSignatureType> {
-  const resolvedPath = resolveToValue(path.get('exprName'));
+  const exprName = path.get('exprName');
 
-  if ('typeAnnotation' in resolvedPath.node) {
-    return getTSTypeWithResolvedTypes(
-      resolvedPath.get('typeAnnotation') as NodePath<TypeScript>,
-      typeParams,
-    );
-  }
+  if (exprName.isIdentifier()) {
+    const resolvedPath = resolveToValue(path.get('exprName'));
+
+    if (resolvedPath.has('typeAnnotation')) {
+      return getTSTypeWithResolvedTypes(
+        resolvedPath.get('typeAnnotation') as NodePath<TypeScript>,
+        typeParams,
+      );
+    }
 
-  // @ts-ignore Do we need to handle TsQualifiedName here TODO
-  return { name: path.node.exprName.name };
+    return { name: exprName.node.name };
+  } else if (exprName.isTSQualifiedName()) {
+    return handleTSQualifiedName(exprName);
+  } else {
+    // TSImportType
+    return { name: printValue(exprName) };
+  }
 }
 
 function handleTSTypeOperator(
diff --git a/packages/react-docgen/src/utils/getTypeParameters.ts b/packages/react-docgen/src/utils/getTypeParameters.ts
@@ -1,18 +1,17 @@
 import resolveGenericTypeAnnotation from '../utils/resolveGenericTypeAnnotation.js';
 import type { NodePath } from '@babel/traverse';
 import type {
-  FlowType,
   Identifier,
   QualifiedTypeIdentifier,
   TSQualifiedName,
+  TSTypeParameter,
   TSTypeParameterDeclaration,
   TSTypeParameterInstantiation,
+  TypeParameter,
   TypeParameterDeclaration,
   TypeParameterInstantiation,
 } from '@babel/types';
 
-// TODO needs tests TS && flow
-
 export type TypeParameters = Record<string, NodePath>;
 
 export default function getTypeParameters(
@@ -27,41 +26,42 @@ export default function getTypeParameters(
 
   let i = 0;
 
-  declaration.get('params').forEach((paramPath) => {
-    const key = paramPath.node.name;
-    const defaultTypePath = paramPath.node.default
-      ? (paramPath.get('default') as NodePath<FlowType>)
-      : null;
-    const typePath =
-      i < numInstantiationParams
-        ? instantiation.get('params')[i++]
-        : defaultTypePath;
+  declaration
+    .get('params')
+    .forEach((paramPath: NodePath<TSTypeParameter | TypeParameter>) => {
+      const key = paramPath.node.name;
+      const defaultProp = paramPath.get('default');
+      const defaultTypePath = defaultProp.hasNode() ? defaultProp : null;
+      const typePath =
+        i < numInstantiationParams
+          ? instantiation.get('params')[i++]
+          : defaultTypePath;
 
-    if (typePath) {
-      let resolvedTypePath: NodePath =
-        resolveGenericTypeAnnotation(typePath) || typePath;
-      let typeName:
-        | NodePath<Identifier | QualifiedTypeIdentifier | TSQualifiedName>
-        | undefined;
+      if (typePath) {
+        let resolvedTypePath: NodePath =
+          resolveGenericTypeAnnotation(typePath) || typePath;
+        let typeName:
+          | NodePath<Identifier | QualifiedTypeIdentifier | TSQualifiedName>
+          | undefined;
 
-      if (resolvedTypePath.isTSTypeReference()) {
-        typeName = resolvedTypePath.get('typeName');
-      } else if (resolvedTypePath.isGenericTypeAnnotation()) {
-        typeName = resolvedTypePath.get('id');
-      }
+        if (resolvedTypePath.isTSTypeReference()) {
+          typeName = resolvedTypePath.get('typeName');
+        } else if (resolvedTypePath.isGenericTypeAnnotation()) {
+          typeName = resolvedTypePath.get('id');
+        }
 
-      if (
-        typeName &&
-        inputParams &&
-        typeName.isIdentifier() &&
-        inputParams[typeName.node.name]
-      ) {
-        resolvedTypePath = inputParams[typeName.node.name];
-      }
+        if (
+          typeName &&
+          inputParams &&
+          typeName.isIdentifier() &&
+          inputParams[typeName.node.name]
+        ) {
+          resolvedTypePath = inputParams[typeName.node.name];
+        }
 
-      params[key] = resolvedTypePath;
-    }
-  });
+        params[key] = resolvedTypePath;
+      }
+    });
 
   return params;
 }
diff --git a/packages/react-docgen/src/utils/resolveFunctionDefinitionToReturnValue.ts b/packages/react-docgen/src/utils/resolveFunctionDefinitionToReturnValue.ts
@@ -27,8 +27,6 @@ const explodedVisitors = visitors.explode<TraverseState>({
   },
 });
 
-// TODO needs unit test
-
 export default function resolveFunctionDefinitionToReturnValue(
   path: NodePath<BabelFunction>,
 ): NodePath<Expression> | null {
diff --git a/packages/react-docgen/src/utils/resolveToValue.ts b/packages/react-docgen/src/utils/resolveToValue.ts
@@ -21,7 +21,6 @@ function findScopePath(
       resolvedParentPath.isImportDefaultSpecifier() ||
       resolvedParentPath.isImportSpecifier()
     ) {
-      // TODO TESTME
       let exportName: string | undefined;
 
       if (resolvedParentPath.isImportDefaultSpecifier()) {
@@ -184,7 +183,7 @@ export default function resolveToValue(path: NodePath): NodePath {
       if (property.isIdentifier() || property.isStringLiteral()) {
         const memberPath = getMemberValuePath(
           resolved,
-          property.isIdentifier() ? property.node.name : property.node.value, // TODO TESTME
+          property.isIdentifier() ? property.node.name : property.node.value,
         );
 
         if (memberPath) {

Original file line number	Diff line number	Diff line change
`@@ -37,7 +37,6 @@ function setPropDescriptor(`
`37`	`37`	`return;`
`38`	`38`	`}`
`39`	`39`
`40`		`- // TODO what about other types here`
`41`	`40`	`const id = argument.get('id') as NodePath;`
`42`	`41`
`43`	`42`	`if (!id.hasNode() \|\| !id.isIdentifier()) {`