Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Docgen info not generated for typescript components #721

Closed
vnys opened this issue Oct 21, 2020 · 12 comments
Closed

Docgen info not generated for typescript components #721

vnys opened this issue Oct 21, 2020 · 12 comments
Assignees
Labels
🐛 bug Something isn't working

Comments

@vnys
Copy link
Member

vnys commented Oct 21, 2020

Describe the bug

A clear and concise description of what the bug is.

Steps to reproduce the bug

  1. Rewrite a component in typescript
  2. Run build:for-storybook

Expected behavior

The bundle should have a docgen object for each component

@vnys vnys added the 🐛 bug Something isn't working label Oct 21, 2020
@vnys vnys self-assigned this Oct 21, 2020
@vnys
Copy link
Member Author

vnys commented Oct 21, 2020

Made a simple component in src/Component/index.tsx that looks like this:

import * as React from 'react'
import { FunctionComponent } from 'react'

type Props = {
  color?: string
} & React.HTMLAttributes<HTMLDivElement>

export const Component: FunctionComponent = ({
  children,
  color = 'orange',

this results in the following docgen info:

Component.__docgenInfo = {
  description: '',
  methods: [],
  displayName: 'Component',
  props: {
    color: {
      defaultValue: {
        value: "'orange'",
        computed: false,
      },
      required: false,
    },
  },
}

@mimarz
Copy link
Contributor

mimarz commented Oct 21, 2020

Suggest testing with a component using forwardRef @vnys

@wenche
Copy link
Contributor

wenche commented Oct 21, 2020

The description field is undefined in Snackbar even if there is a comment in the code.

@vnys
Copy link
Member Author

vnys commented Oct 21, 2020

I get a description if I place the comment right before the function. If it’s at the top or above the type definition it’s not added.

import * as React from 'react'
import { FunctionComponent } from 'react'

type Props = {
  color?: string
} & React.HTMLAttributes<HTMLDivElement>

/**
 * This is the Component description
 */

export const Component: FunctionComponent = ({
  children,
  color = 'orange',
}: Props) => <div style={{ color: color }}>{children}</div>

results in

Component.__docgenInfo = {
  "description": "This is the Component description",
  "methods": [],
  "displayName": "Component",
  "props": {
    "color": {
      "defaultValue": {
        "value": "'orange'",
        "computed": false
      },
      "required": false
    }
  }
};

@vnys
Copy link
Member Author

vnys commented Oct 21, 2020

forwardRef works…

export const Component = forwardRef<HTMLDivElement, Props>(
  function EdsComponent({ children, color = 'orange' }, ref) {
    return (
      <div style={{ color: color }} ref={ref}>
        {children}
      </div>
    )
  },
)

@vnys
Copy link
Member Author

vnys commented Oct 21, 2020

Compound components work…

import { OtherComponent } from './OtherComponent'
import { Component as BaseComponent } from './Component'

type ComponentTypes = typeof BaseComponent & {
  OtherComponent: typeof OtherComponent
}

const Component = BaseComponent as ComponentTypes
Component.OtherComponent = OtherComponent

export { Component }

@vnys
Copy link
Member Author

vnys commented Oct 21, 2020

The Snackbar component works, but SnackbarAction does not. But Snackbar only has one prop, and that’s children. If I add another prop, such as foo?: string – then that type is only picked up if the component destructs the prop in the function signature with a default value: foo = ''. It looks like the same goes for required props 🤔

@vnys
Copy link
Member Author

vnys commented Oct 21, 2020

Took a look at <Accordion/>. This component imports types from an external file. This seem to work fine. But what does not work are spreads rest parameters 🙄. So if I remove ...props the types are picked up. A bit stuck on this one. I suggest we move to typescript for the storybook as well and see if that fixes these issues.

@vnys
Copy link
Member Author

vnys commented Oct 22, 2020

There are two processors that can be used to extract types from components, react-docgen-typescript and react-docgen. The first one is made by Styleguidist, who interestingly enough use react-docgen. Storybook uses react-docgen-typescript in the version we have, because react-docgen did not display props from imported types at the time. Michael Shilman wrote about that in a gist where he compares the two. This can be seen by adding the following snippet to .storybook/main.js:

  typescript: {
    reactDocgen: 'react-docgen',
  },

…which causes the Accordion component to only show internal types in the Storybook – and also removes the description from the types for some reason.

@vnys
Copy link
Member Author

vnys commented Oct 22, 2020

What I’ve done now, in #723, is to disable all the stories for components that have displayNames with hyphens. Also, the displayName must match the name of the component. So Accordion cannot have EdsAccordion as displayName, because then the props won’t show up in the Storybook.

@mimarz
Copy link
Contributor

mimarz commented Oct 22, 2020

What I’ve done now, in #723, is to disable all the stories for components that have displayNames with hyphens. Also, the displayName must match the name of the component. So Accordion cannot have EdsAccordion as displayName, because then the props won’t show up in the Storybook.

So does that mean the function name must match displayName? or is prefixing using Eds a no go in general?

@vnys
Copy link
Member Author

vnys commented Oct 22, 2020

That’s correct. In some components we can just leave out displayName, and it will be generated from the component name, in other components eslint complains when I remove the displayName. I expect this to be solved when we streamline all the components once we’re done with the Typescript conversion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants