strothj / react-docgen-typescript-loader

Webpack loader to generate docgen information from Typescript React components.
Other
360 stars 47 forks source link

forwardRef trips up docgen #76

Closed jonathanloske closed 4 years ago

jonathanloske commented 4 years ago

Describe the bug When implementing a Component in the way described below, the Storybook Props block (created via react-docgen-typescript-loader) only shows the following text:

"Cannot read property 'appearance' of undefined"

To Reproduce Declare a component in way mentioned under "Code snippets".

Expected behavior The Props block should display the corresponding Props.

Screenshots 69129062-f1c41180-0aad-11ea-82ce-f3cedbac3a57

Code snippets Button.tsx

type ButtonProps = {
  appearance?: "primary" | "secondary" | "ghost";
} & JSX.IntrinsicElements["button"];

const Button: RefForwardingComponent<HTMLButtonElement, ButtonProps> = React.forwardRef(({ appearance, ...otherProps}, ref) => (
  <button>...</button>
))

Button.displayName = "Button";

Button.defaultProps = {
  appearance: "primary"
};

Button.stories.tsx

import React from "react";
import { storiesOf } from "@storybook/react";
import { Button } from "./Button";

storiesOf("Button", module)
  .addParameters({
    component: Button
  })
  .add("primary", () => (
    <Button appearance="primary">
      My primary button!
    </Button>
  ))
;

System:

package.json

"react-docgen-typescript-loader": "^3.3.0"

Update: Also tested with 3.6.0.

Tried out with both Storybook 5.2.5 and Storybook 5.3.0-beta.1

Using stories in the TSX file format but encountered the error also when using the JSX file format.

Component is in TSX format.

Please paste the results of npx -p @storybook/cli@next sb info here.

  System:
    OS: macOS 10.15.1
    CPU: (4) x64 Intel(R) Core(TM) i7-7567U CPU @ 3.50GHz
  Binaries:
    Node: 10.16.0 - ~/.nvm/versions/node/v10.16.0/bin/node
    npm: 6.9.0 - ~/.nvm/versions/node/v10.16.0/bin/npm
  Browsers:
    Chrome: 78.0.3904.97
    Safari: 13.0.3
  npmPackages:
    @storybook/cli: ^5.3.0-beta.1 => 5.3.0-beta.1

Additional context Used to work before introducing RefForwardingComponent and React.forwardRef

jonathanloske commented 4 years ago

Mirror issue on Storybook side: https://github.com/storybookjs/storybook/issues/8881

jonathanloske commented 4 years ago

Relevant issue in react-docgen: https://github.com/reactjs/react-docgen/issues/267

jonathanloske commented 4 years ago

This might be fixed with react-docgen 5.0.0-beta.1 which includes a pull request that is titled "Fully support forwardRef": https://github.com/reactjs/react-docgen/pull/350

awinograd commented 4 years ago

@jonathanherdt i ran into this issue as well. I dont think the react-docgen issue is related since this loader uses something else under the hood.

I was able to get the docgenInfo object to show up by destructuring forwardRef and use it directly. e.g.


// bad
import React from 'react';
export const MyComponent = React.forwardRef<A, B>( ... )
// good
import React, { forwardRef } from 'react';
export const MyComponent = forwardRef<A, B>( ... )
`
jeffcstock commented 4 years ago

This is not working for me, event with destructuring. Getting the cannot read property ... of undefined.

I'm on version 5.2.8.

Can confirm that taking the forwardRef out displays all props correctly.

import React, { ElementType, forwardRef, Ref, ReactElement } from 'react'

export const Button = forwardRef(
  (props: ButtonProps, ref: Ref<HTMLAnchorElement>) => {
   ...
  }
)

Screen Shot 2020-01-10 at 10 57 07 AM

awinograd commented 4 years ago

@jeffcstock have you tried parameterizing your forwardRef invocation?

i.e. forwardRef<HTMLAnchorElement, ButtonProps>((props, ref) => { ... })

jeffcstock commented 4 years ago

@awinograd thanks for your response, but yes I can confirm that doesn't help things. Same exact result. 😕

awinograd commented 4 years ago

@jeffcstock no problem. sorry i wasn't more help!

jonathanloske commented 4 years ago

@jeffcstock Storybook 5.3 was just released and fixes this for me. Maybe give it a shot?

jeffcstock commented 4 years ago

That fixed it, thank you @jonathanherdt!

strothj commented 4 years ago

Closing this issue because of multiple reports that it's been resolved. Leave a message if it's still a problem.