[docgen] better error handling and update to v3 (gatsbyjs#5604)

* better error handling and update to v3 * fix rdg version: * update doc-handler * Update package.json * make test more robust * fix test
gpetrioli · Jun 7, 2018 · 7a468b8 · 7a468b8
1 parent 7e4833a
commit 7a468b8
Show file tree

Hide file tree

Showing 7 changed files with 384 additions and 320 deletions.
diff --git a/packages/gatsby-transformer-react-docgen/package.json b/packages/gatsby-transformer-react-docgen/package.json
@@ -7,28 +7,27 @@
     "url": "https://github.com/gatsbyjs/gatsby/issues"
   },
   "dependencies": {
+    "@babel/code-frame": "^7.0.0-beta.49",
     "@babel/runtime": "7.0.0-beta.47",
     "@babel/types": "7.0.0-beta.47",
     "common-tags": "^1.4.0",
     "graphql-type-json": "^0.1.4",
-    "react-docgen": "^2.20.1"
+    "react-docgen": "3.0.0-beta12",
+    "react-docgen-displayname-handler": " ^2.1.0"
   },
   "devDependencies": {
     "@babel/cli": "7.0.0-beta.47",
     "@babel/core": "7.0.0-beta.47",
     "cross-env": "^5.1.4",
     "lodash": "^4.17.4"
   },
-  "homepage": "https://github.com/gatsbyjs/gatsby/tree/master/packages/gatsby-transformer-react-docgen#readme",
-  "keywords": [
-    "gatsby",
-    "gatsby-plugin",
-    "react",
-    "react-docgen"
-  ],
+  "homepage":
+    "https://github.com/gatsbyjs/gatsby/tree/master/packages/gatsby-transformer-react-docgen#readme",
+  "keywords": ["gatsby", "gatsby-plugin", "react", "react-docgen"],
   "license": "MIT",
   "main": "index.js",
-  "repository": "https://github.com/gatsbyjs/gatsby/tree/master/packages/gatsby-transformer-react-docgen",
+  "repository":
+    "https://github.com/gatsbyjs/gatsby/tree/master/packages/gatsby-transformer-react-docgen",
   "scripts": {
     "build": "babel src --out-dir . --ignore **/__tests__",
     "prepublish": "cross-env NODE_ENV=production npm run build",

diff --git a/packages/gatsby-transformer-react-docgen/src/__tests__/fixtures/classes.js b/packages/gatsby-transformer-react-docgen/src/__tests__/fixtures/classes.js
@@ -11,6 +11,8 @@ function Foo() {
   return <div />
 }
 
+Baz.Foo = () => <div />
+
 /**
  * Description!
  *

diff --git a/packages/gatsby-transformer-react-docgen/src/__tests__/fixtures/unnamed.js b/packages/gatsby-transformer-react-docgen/src/__tests__/fixtures/unnamed.js
@@ -1,4 +1,4 @@
-export default class extends React.Component {
+module.exports = class extends React.Component {
   render() {
     return <div />
   }

diff --git a/packages/gatsby-transformer-react-docgen/src/__tests__/on-node-create.js b/packages/gatsby-transformer-react-docgen/src/__tests__/on-node-create.js
@@ -23,6 +23,7 @@ describe(`transformer-react-doc-gen: onCreateNode`, () => {
         loadNodeContent,
         actions,
         createNodeId,
+        reporter: { error: console.error },
       },
       opts
     )
@@ -45,10 +46,10 @@ describe(`transformer-react-doc-gen: onCreateNode`, () => {
     }
   })
 
-  it(`should only process javascript and jsx nodes`, () => {
+  it(`should only process javascript and jsx nodes`, async () => {
     loadNodeContent = jest.fn(() => new Promise(() => {}))
 
-    expect(run({ internal: { mediaType: `text/x-foo` } })).toBeNull()
+    expect(await run({ internal: { mediaType: `text/x-foo` } })).toBeUndefined()
     expect(
       run({ internal: { mediaType: `application/javascript` } })
     ).toBeDefined()
@@ -61,14 +62,22 @@ describe(`transformer-react-doc-gen: onCreateNode`, () => {
     await run(node)
 
     let types = groupBy(createdNodes, n => n.internal.type)
-    expect(types.ComponentMetadata).toHaveLength(5)
+    expect(types.ComponentMetadata).toHaveLength(6)
   })
 
   it(`should give all components a name`, async () => {
     await run(node)
 
     let types = groupBy(createdNodes, `internal.type`)
-    expect(types.ComponentMetadata.every(c => c.displayName)).toBe(true)
+
+    expect(types.ComponentMetadata.map(c => c.displayName)).toEqual([
+      `Baz`,
+      `Buz`,
+      `Foo`,
+      `Baz.Foo`,
+      `Bar`,
+      `Qux`,
+    ])
   })
 
   it(`should infer a name`, async () => {
@@ -124,7 +133,9 @@ describe(`transformer-react-doc-gen: onCreateNode`, () => {
     })
     it(`should add flow type info`, async () => {
       await run(node)
-      expect(createdNodes[1].flowType).toEqual({
+      const created = createdNodes.find(f => !!f.flowType)
+
+      expect(created.flowType).toEqual({
         name: `number`,
       })
     })

diff --git a/packages/gatsby-transformer-react-docgen/src/on-node-create.js b/packages/gatsby-transformer-react-docgen/src/on-node-create.js
@@ -63,8 +63,8 @@ function createPropNodes(node, component, actions, createNodeId) {
   return node
 }
 
-export default function onCreateNode(
-  { node, loadNodeContent, actions, createNodeId },
+export default async function onCreateNode(
+  { node, loadNodeContent, actions, createNodeId, reporter },
   pluginOptions
 ) {
   const { createNode, createParentChildLink } = actions
@@ -73,44 +73,53 @@ export default function onCreateNode(
     node.internal.mediaType !== `application/javascript` &&
     node.internal.mediaType !== `text/jsx`
   )
-    return null
-
-  return loadNodeContent(node)
-    .then(content => {
-      const components = parseMetadata(content, node, pluginOptions)
-
-      components.forEach(component => {
-        const strContent = JSON.stringify(component)
-        const contentDigest = digest(strContent)
-        const nodeId = `${node.id}--${component.displayName}--ComponentMetadata`
-
-        let metadataNode = {
-          ...component,
-          props: null, // handled by the prop node creation
-          id: createNodeId(nodeId),
-          children: [],
-          parent: node.id,
-          internal: {
-            contentDigest,
-            type: `ComponentMetadata`,
-          },
-        }
-
-        createParentChildLink({ parent: node, child: metadataNode })
-        metadataNode = createPropNodes(
-          metadataNode,
-          component,
-          actions,
-          createNodeId
-        )
-        metadataNode = createDescriptionNode(
-          metadataNode,
-          component,
-          actions,
-          createNodeId
-        )
-        createNode(metadataNode)
-      })
-    })
-    .catch(err => console.log(err))
+    return
+
+  const content = await loadNodeContent(node)
+
+  let components
+  try {
+    components = parseMetadata(content, node, pluginOptions)
+  } catch (err) {
+    reporter.error(
+      `There was a problem parsing component metadata for file: "${
+        node.relativePath
+      }"`,
+      err
+    )
+    return
+  }
+
+  components.forEach(component => {
+    const strContent = JSON.stringify(component)
+    const contentDigest = digest(strContent)
+    const nodeId = `${node.id}--${component.displayName}--ComponentMetadata`
+
+    let metadataNode = {
+      ...component,
+      props: null, // handled by the prop node creation
+      id: createNodeId(nodeId),
+      children: [],
+      parent: node.id,
+      internal: {
+        contentDigest,
+        type: `ComponentMetadata`,
+      },
+    }
+
+    createParentChildLink({ parent: node, child: metadataNode })
+    metadataNode = createPropNodes(
+      metadataNode,
+      component,
+      actions,
+      createNodeId
+    )
+    metadataNode = createDescriptionNode(
+      metadataNode,
+      component,
+      actions,
+      createNodeId
+    )
+    createNode(metadataNode)
+  })
 }
diff --git a/packages/gatsby-transformer-react-docgen/src/parse.js b/packages/gatsby-transformer-react-docgen/src/parse.js
@@ -1,53 +1,34 @@
-import path from "path"
-
-import * as types from "babel-types"
-import { parse, defaultHandlers } from "react-docgen"
+import { codeFrameColumns } from "@babel/code-frame"
+import { parse, resolver, handlers } from "react-docgen"
 import { ERROR_MISSING_DEFINITION } from "react-docgen/dist/parse"
-import findAllComponentDefinitions from "react-docgen/dist/resolver/findAllComponentDefinitions"
 
 import { cleanDoclets, parseDoclets, applyPropDoclets } from "./Doclets"
+import { createDisplayNameHandler } from "react-docgen-displayname-handler"
 
-function getAssignedIdenifier(path) {
-  let property = path.parentPath
-  while (property) {
-    if (types.isVariableDeclarator(property.node)) return property.node.id.name
-    property = property.parentPath
-  }
-  return null
-}
+const defaultHandlers = [
+  handlers.propTypeHandler,
+  handlers.propTypeCompositionHandler,
+  handlers.propDocBlockHandler,
+  handlers.flowTypeHandler,
+  handlers.defaultPropsHandler,
+  handlers.componentDocblockHandler,
+  handlers.componentMethodsHandler,
+  handlers.componentMethodsJsDocHandler,
+]
 
 let fileCount = 0
-function nameHandler(filePath = `/AnonymousComponent_${++fileCount}`) {
-  let defaultName = path.basename(filePath, path.extname(filePath))
-  let componentCount = 0
-
-  return (docs, nodePath) => {
-    let displayName = docs.get(`displayName`)
-    if (displayName) return
-
-    if (
-      types.isArrowFunctionExpression(nodePath.node) ||
-      types.isFunctionExpression(nodePath.node) ||
-      types.isObjectExpression(nodePath.node)
-    ) {
-      displayName = getAssignedIdenifier(nodePath)
-    } else if (
-      types.isFunctionDeclaration(nodePath.node) ||
-      types.isClassDeclaration(nodePath.node)
-    ) {
-      displayName = nodePath.node.id.name
-    }
-
-    docs.set(`displayName`, displayName || `${defaultName}_${++componentCount}`)
-  }
-}
 
 /**
  * Wrap handlers to pass in additional arguments such as the File node
  */
 function makeHandlers(node, handlers) {
   handlers = (handlers || []).map(h => (...args) => h(...args, node))
-  return [nameHandler(node.absolutePath), ...handlers]
+  return [
+    createDisplayNameHandler(
+      node.absolutePath || `/UnknownComponent${++fileCount}`
+    ),
+    ...handlers,
+  ]
 }
 
 export default function parseMetadata(content, node, options) {
@@ -56,16 +37,27 @@ export default function parseMetadata(content, node, options) {
   try {
     components = parse(
       content,
-      options.resolver || findAllComponentDefinitions,
+      options.resolver || resolver.findAllComponentDefinitions,
       defaultHandlers.concat(makeHandlers(node, options.handlers))
     )
   } catch (err) {
     if (err.message === ERROR_MISSING_DEFINITION) return []
+    // reset the stack to here since it's not helpful to see all the react-docgen guts
+    // const parseErr = new Error(err.message)
+    if (err.loc) {
+      err.codeFrame = codeFrameColumns(
+        content,
+        err.loc.start || { start: err.loc },
+        {
+          highlightCode: true,
+        }
+      )
+    }
     throw err
   }
 
   if (components.length === 1) {
-    components[0].displayName = components[0].displayName.replace(/_\d+$/, ``)
+    components[0].displayName = components[0].displayName.replace(/\d+$/, ``)
   }
 
   components.forEach(component => {
-Original file line number
+Diff line change
@@ Expand Up / @@ -11,6 +11,8 @@ function Foo() { @@
       return <div />
     }
+    Baz.Foo = () => <div />
     /**
      * Description!
      *
@@ Expand Down @@