Merge pull request #23825 from storybookjs/shilman/fix-react-docgen-enum

React: Upgrade `react-docgen` to 6.0.x and improve argTypes
storybookjs · Oct 4, 2023 · 109eb20 · 109eb20
2 parents 723481e + 9fe358f
commit 109eb20
Show file tree

Hide file tree

Showing 17 changed files with 273 additions and 137 deletions.
diff --git a/code/frameworks/react-vite/package.json b/code/frameworks/react-vite/package.json
@@ -52,9 +52,8 @@
     "@storybook/builder-vite": "workspace:*",
     "@storybook/react": "workspace:*",
     "@vitejs/plugin-react": "^3.0.1",
-    "ast-types": "^0.14.2",
     "magic-string": "^0.30.0",
-    "react-docgen": "6.0.0-alpha.3"
+    "react-docgen": "^6.0.2"
   },
   "devDependencies": {
     "@types/node": "^16.0.0",

diff --git a/code/frameworks/react-vite/src/plugins/docgen-handlers/actualNameHandler.ts b/code/frameworks/react-vite/src/plugins/docgen-handlers/actualNameHandler.ts
@@ -9,40 +9,46 @@
  * directly from displayNameHandler, using the same approach as babel-plugin-react-docgen.
  */
 
-import { namedTypes as t } from 'ast-types';
-import type { NodePath } from 'ast-types/lib/node-path';
-import { getNameOrValue, isReactForwardRefCall } from 'react-docgen/dist/utils';
-import type { Importer } from 'react-docgen/dist/parse';
-import type Documentation from 'react-docgen/dist/Documentation';
+import type { Handler, NodePath, babelTypes as t } from 'react-docgen';
+import { utils } from 'react-docgen';
 
-export default function actualNameHandler(
-  documentation: Documentation,
-  path: NodePath,
-  importer: Importer
-): void {
-  if (t.ClassDeclaration.check(path.node) || t.FunctionDeclaration.check(path.node)) {
-    documentation.set('actualName', getNameOrValue(path.get('id')));
+const { getNameOrValue, isReactForwardRefCall } = utils;
+
+const actualNameHandler: Handler = function actualNameHandler(documentation, componentDefinition) {
+  if (
+    (componentDefinition.isClassDeclaration() || componentDefinition.isFunctionDeclaration()) &&
+    componentDefinition.has('id')
+  ) {
+    documentation.set(
+      'actualName',
+      getNameOrValue(componentDefinition.get('id') as NodePath<t.Identifier>)
+    );
   } else if (
-    t.ArrowFunctionExpression.check(path.node) ||
-    t.FunctionExpression.check(path.node) ||
-    isReactForwardRefCall(path, importer)
+    componentDefinition.isArrowFunctionExpression() ||
+    componentDefinition.isFunctionExpression() ||
+    isReactForwardRefCall(componentDefinition)
   ) {
-    let currentPath = path;
-    while (currentPath.parent) {
-      if (t.VariableDeclarator.check(currentPath.parent.node)) {
-        documentation.set('actualName', getNameOrValue(currentPath.parent.get('id')));
+    let currentPath: NodePath = componentDefinition;
+
+    while (currentPath.parentPath) {
+      if (currentPath.parentPath.isVariableDeclarator()) {
+        documentation.set('actualName', getNameOrValue(currentPath.parentPath.get('id')));
         return;
       }
-      if (t.AssignmentExpression.check(currentPath.parent.node)) {
-        const leftPath = currentPath.parent.get('left');
-        if (t.Identifier.check(leftPath.node) || t.Literal.check(leftPath.node)) {
+      if (currentPath.parentPath.isAssignmentExpression()) {
+        const leftPath = currentPath.parentPath.get('left');
+
+        if (leftPath.isIdentifier() || leftPath.isLiteral()) {
           documentation.set('actualName', getNameOrValue(leftPath));
           return;
         }
       }
-      currentPath = currentPath.parent;
+
+      currentPath = currentPath.parentPath;
     }
     // Could not find an actual name
     documentation.set('actualName', '');
   }
-}
+};
+
+export default actualNameHandler;
diff --git a/code/frameworks/react-vite/src/plugins/react-docgen.ts b/code/frameworks/react-vite/src/plugins/react-docgen.ts
@@ -1,22 +1,23 @@
 import path from 'path';
 import { createFilter } from '@rollup/pluginutils';
+import type { Documentation } from 'react-docgen';
 import {
+  ERROR_CODES,
   parse,
-  handlers as docgenHandlers,
-  resolver as docgenResolver,
-  importers as docgenImporters,
+  builtinHandlers as docgenHandlers,
+  builtinResolvers as docgenResolver,
+  builtinImporters as docgenImporters,
 } from 'react-docgen';
-import type { DocumentationObject } from 'react-docgen/dist/Documentation';
 import MagicString from 'magic-string';
 import type { PluginOption } from 'vite';
 import actualNameHandler from './docgen-handlers/actualNameHandler';
 
-type DocObj = DocumentationObject & { actualName: string };
+type DocObj = Documentation & { actualName: string };
 
 // TODO: None of these are able to be overridden, so `default` is aspirational here.
 const defaultHandlers = Object.values(docgenHandlers).map((handler) => handler);
-const defaultResolver = docgenResolver.findAllExportedComponentDefinitions;
-const defaultImporter = docgenImporters.makeFsImporter();
+const defaultResolver = new docgenResolver.FindExportedDefinitionsResolver();
+const defaultImporter = docgenImporters.fsImporter;
 const handlers = [...defaultHandlers, actualNameHandler];
 
 type Options = {
@@ -39,8 +40,9 @@ export function reactDocgen({
       if (!filter(relPath)) return;
 
       try {
-        // Since we're using `findAllExportedComponentDefinitions`, this will always be an array.
-        const docgenResults = parse(src, defaultResolver, handlers, {
+        const docgenResults = parse(src, {
+          resolver: defaultResolver,
+          handlers,
           importer: defaultImporter,
           filename: id,
         }) as DocObj[];
@@ -59,10 +61,12 @@ export function reactDocgen({
           code: s.toString(),
           map: s.generateMap(),
         };
-      } catch (e) {
-        // Usually this is just an error from react-docgen that it couldn't find a component
-        // Only uncomment for troubleshooting
-        // console.error(e);
+      } catch (e: any) {
+        // Ignore the error when react-docgen cannot find a react component
+        if (e.code === ERROR_CODES.MISSING_DEFINITION) {
+          return;
+        }
+        throw e;
       }
     },
   };

diff --git a/code/lib/docs-tools/src/argTypes/convert/convert.test.ts b/code/lib/docs-tools/src/argTypes/convert/convert.test.ts
@@ -180,30 +180,18 @@ describe('storybook type system', () => {
         {
           "kind": {
             "raw": "'default' | 'action'",
-            "name": "union",
+            "name": "enum",
             "value": [
-              {
-                "name": "other",
-                "value": "literal"
-              },
-              {
-                "name": "other",
-                "value": "literal"
-              }
+              "default",
+              "action"
             ]
           },
           "inlinedNumericLiteralUnion": {
             "raw": "0 | 1",
-            "name": "union",
+            "name": "enum",
             "value": [
-              {
-                "name": "other",
-                "value": "literal"
-              },
-              {
-                "name": "other",
-                "value": "literal"
-              }
+              0,
+              1
             ]
           },
           "enumUnion": {

diff --git a/code/lib/docs-tools/src/argTypes/convert/proptypes/convert.ts b/code/lib/docs-tools/src/argTypes/convert/proptypes/convert.ts
@@ -2,7 +2,7 @@
 import mapValues from 'lodash/mapValues.js';
 import type { SBType } from '@storybook/types';
 import type { PTType } from './types';
-import { includesQuotes, trimQuotes } from '../utils';
+import { parseLiteral } from '../utils';
 
 const SIGNATURE_REGEXP = /^\(.*\) => /;
 
@@ -13,15 +13,7 @@ export const convert = (type: PTType): SBType | any => {
 
   switch (name) {
     case 'enum': {
-      const values = computed
-        ? value
-        : value.map((v: PTType) => {
-            const trimmedValue = trimQuotes(v.value);
-
-            return includesQuotes(v.value) || Number.isNaN(Number(trimmedValue))
-              ? trimmedValue
-              : Number(trimmedValue);
-          });
+      const values = computed ? value : value.map((v: PTType) => parseLiteral(v.value));
       return { ...base, name, value: values };
     }
     case 'string':

diff --git a/code/lib/docs-tools/src/argTypes/convert/typescript/convert.ts b/code/lib/docs-tools/src/argTypes/convert/typescript/convert.ts
@@ -1,6 +1,7 @@
 /* eslint-disable no-case-declarations */
 import type { SBType } from '@storybook/types';
 import type { TSType, TSSigType } from './types';
+import { parseLiteral } from '../utils';
 
 const convertSig = (type: TSSigType) => {
   switch (type.type) {
@@ -37,6 +38,18 @@ export const convert = (type: TSType): SBType | void => {
     case 'signature':
       return { ...base, ...convertSig(type) };
     case 'union':
+      let result;
+      if (type.elements.every((element) => element.name === 'literal')) {
+        result = {
+          ...base,
+          name: 'enum',
+          // @ts-expect-error fix types
+          value: type.elements.map((v) => parseLiteral(v.value)),
+        };
+      } else {
+        result = { ...base, name, value: type.elements.map(convert) };
+      }
+      return result;
     case 'intersection':
       return { ...base, name, value: type.elements.map(convert) };
     default:

diff --git a/code/lib/docs-tools/src/argTypes/convert/typescript/types.ts b/code/lib/docs-tools/src/argTypes/convert/typescript/types.ts
@@ -33,7 +33,7 @@ type TSObjectSigType = TSBaseType & {
 };
 
 type TSScalarType = TSBaseType & {
-  name: 'any' | 'boolean' | 'number' | 'void' | 'string' | 'symbol';
+  name: 'any' | 'boolean' | 'number' | 'void' | 'string' | 'symbol' | 'literal';
 };
 
 type TSArrayType = TSBaseType & {

diff --git a/code/lib/docs-tools/src/argTypes/convert/utils.ts b/code/lib/docs-tools/src/argTypes/convert/utils.ts
@@ -1,3 +1,10 @@
 const QUOTE_REGEX = /^['"]|['"]$/g;
 export const trimQuotes = (str: string) => str.replace(QUOTE_REGEX, '');
 export const includesQuotes = (str: string) => QUOTE_REGEX.test(str);
+export const parseLiteral = (str: string) => {
+  const trimmedValue = trimQuotes(str);
+
+  return includesQuotes(str) || Number.isNaN(Number(trimmedValue))
+    ? trimmedValue
+    : Number(trimmedValue);
+};
diff --git a/code/lib/docs-tools/src/argTypes/docgen/typeScript/createType.ts b/code/lib/docs-tools/src/argTypes/docgen/typeScript/createType.ts
@@ -9,9 +9,12 @@ export function createType({ tsType, required }: DocgenInfo): PropType {
     return null;
   }
 
+  let typeName = tsType.name;
   if (!required) {
-    return createSummaryValue(tsType.name.replace(' | undefined', ''));
+    typeName = typeName.replace(' | undefined', '');
   }
 
-  return createSummaryValue(tsType.name);
+  return createSummaryValue(
+    ['Array', 'Record', 'signature'].includes(tsType.name) ? tsType.raw : typeName
+  );
 }
diff --git a/code/lib/docs-tools/src/argTypes/docgen/types.ts b/code/lib/docs-tools/src/argTypes/docgen/types.ts
@@ -26,8 +26,9 @@ export interface DocgenFlowType extends DocgenType {
   elements?: any[];
 }
 
-// eslint-disable-next-line @typescript-eslint/no-empty-interface
-export interface DocgenTypeScriptType extends DocgenType {}
+export interface DocgenTypeScriptType extends DocgenType {
+  raw?: string;
+}
 
 // export type DocgenType = DocgenPropType | DocgenFlowType | DocgenTypeScriptType;
 

diff --git a/...renderers/react/template/stories/docgen-components/9493-ts-display-name/argTypes.snapshot b/...renderers/react/template/stories/docgen-components/9493-ts-display-name/argTypes.snapshot
@@ -23,10 +23,15 @@ Object {
   },
   "title": Object {
     "control": Object {
-      "type": "object",
+      "type": "radio",
     },
     "description": "A title that brings attention to the alert.",
     "name": "title",
+    "options": Array [
+      "Code Red",
+      "Code Yellow",
+      "Code Green",
+    ],
     "table": Object {
       "defaultValue": Object {
         "detail": undefined,
@@ -39,22 +44,13 @@ Object {
       },
     },
     "type": Object {
-      "name": "union",
+      "name": "enum",
       "raw": "'Code Red' | 'Code Yellow' | 'Code Green'",
       "required": false,
       "value": Array [
-        Object {
-          "name": "other",
-          "value": "literal",
-        },
-        Object {
-          "name": "other",
-          "value": "literal",
-        },
-        Object {
-          "name": "other",
-          "value": "literal",
-        },
+        "Code Red",
+        "Code Yellow",
+        "Code Green",
       ],
     },
   },

diff --git a/...nderers/react/template/stories/docgen-components/9493-ts-display-name/properties.snapshot b/...nderers/react/template/stories/docgen-components/9493-ts-display-name/properties.snapshot
@@ -12,21 +12,12 @@ Object {
       "name": "title",
       "required": false,
       "sbType": Object {
-        "name": "union",
+        "name": "enum",
         "raw": "'Code Red' | 'Code Yellow' | 'Code Green'",
         "value": Array [
-          Object {
-            "name": "other",
-            "value": "literal",
-          },
-          Object {
-            "name": "other",
-            "value": "literal",
-          },
-          Object {
-            "name": "other",
-            "value": "literal",
-          },
+          "Code Red",
+          "Code Yellow",
+          "Code Green",
         ],
       },
       "type": Object {

diff --git a/...erers/react/template/stories/docgen-components/9721-ts-deprecated-jsdoc/argTypes.snapshot b/...erers/react/template/stories/docgen-components/9721-ts-deprecated-jsdoc/argTypes.snapshot
@@ -13,7 +13,7 @@ Object {
       "jsDocTags": undefined,
       "type": Object {
         "detail": undefined,
-        "summary": "signature",
+        "summary": "{ width: number; height: number }",
       },
     },
     "type": Object {

diff --git a/...ers/react/template/stories/docgen-components/9721-ts-deprecated-jsdoc/properties.snapshot b/...ers/react/template/stories/docgen-components/9721-ts-deprecated-jsdoc/properties.snapshot
@@ -22,7 +22,7 @@ Object {
       },
       "type": Object {
         "detail": undefined,
-        "summary": "signature",
+        "summary": "{ width: number; height: number }",
       },
     },
   ],

diff --git a/...nderers/react/template/stories/docgen-components/9827-ts-default-values/argTypes.snapshot b/...nderers/react/template/stories/docgen-components/9827-ts-default-values/argTypes.snapshot
@@ -13,7 +13,7 @@ Object {
       "jsDocTags": undefined,
       "type": Object {
         "detail": undefined,
-        "summary": "Array",
+        "summary": "string[]",
       },
     },
     "type": Object {

diff --git a/...erers/react/template/stories/docgen-components/9827-ts-default-values/properties.snapshot b/...erers/react/template/stories/docgen-components/9827-ts-default-values/properties.snapshot
@@ -48,7 +48,7 @@ Object {
       },
       "type": Object {
         "detail": undefined,
-        "summary": "Array",
+        "summary": "string[]",
       },
     },
   ],