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

addon-docs: Props doesn't work when using React.memo. #9586

Closed
sinkz opened this issue Jan 21, 2020 · 35 comments · Fixed by #12686
Closed

addon-docs: Props doesn't work when using React.memo. #9586

sinkz opened this issue Jan 21, 2020 · 35 comments · Fixed by #12686

Comments

@sinkz
Copy link

sinkz commented Jan 21, 2020

Describe the bug
I trying show the props in docs page using <Props of={Component}/> in .mdx file. But i receive this message "No props found for this component"

To Reproduce
Steps to reproduce the behavior:

  1. Create Component in React and wrapped this with React.memo
  2. Run Storybook and open docs tab
  3. The prop table does not appear

Screenshots
ex1

Code snippets

Button.js

function Button({ label, onClick }) {
  return (
    <ButtonContainer onClick={onClick}>
      <Label> {label} </Label>
    </ButtonContainer>
  );
}

Button.propTypes = {
  label: PropTypes.string.isRequired,
  onClick: PropTypes.func.isRequired,
};

export default React.memo(Button);

button.stories.mdx

# Button

Exemplo do **botão** padrão.

<Preview withToolbar>
  <Story name="Default" height="100px" parameters={{ decorators: [withKnobs] }}>
    <Button 
      labelColor={text("Label Color", theme.colors.white)}
      onClick={() => alert("Button Click")} />
  </Story>
</Preview>

## Props

<Props of={Button} />

System:
OS: Linux 4.15 Ubuntu 18.04.3 LTS (Bionic Beaver)
CPU: (4) x64 Intel(R) Core(TM) i5-3570 CPU @ 3.40GHz
Binaries:
Node: 10.15.3 - /usr/local/bin/node
Yarn: 1.19.1 - /usr/bin/yarn
npm: 6.4.1 - /usr/local/bin/npm
Browsers:
Chrome: 78.0.3904.108
Firefox: 72.0.1
npmPackages:
@storybook/addon-actions: 5.3.7 => 5.3.7
@storybook/addon-backgrounds: 5.3.7 => 5.3.7
@storybook/addon-centered: 5.3.7 => 5.3.7
@storybook/addon-docs: 5.3.7 => 5.3.7
@storybook/addon-knobs: 5.3.7 => 5.3.7
@storybook/addon-storysource: 5.3.7 => 5.3.7
@storybook/addon-viewport: 5.3.7 => 5.3.7
@storybook/core: 5.3.7 => 5.3.7
@storybook/react: 5.3.7 => 5.3.7
@storybook/theming: 5.3.7 => 5.3.7
npmGlobalPackages:
@storybook/cli: 5.3.4

@shilman
Copy link
Member

shilman commented Jan 21, 2020

Unsatisfying workaround is to also export your pure component and use that for documentation purposes

@advl
Copy link

advl commented Jan 21, 2020

I ran into the same problem and found a solution some time ago: add the proptypes to the memo'ed component and you will be fine. Also I believe this has less to do than storybook than with the way the PropTypes work.

@stale
Copy link

stale bot commented Feb 11, 2020

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

@stale stale bot added the inactive label Feb 11, 2020
shilman added a commit that referenced this issue Feb 14, 2020
@shilman
Copy link
Member

shilman commented Feb 14, 2020

Repro: ee4144a

@breytex
Copy link

breytex commented Mar 16, 2020

Sorry for the shameless bump, but is this scheduled to be fixed in an upcoming release? :)

@shilman
Copy link
Member

shilman commented Mar 16, 2020

@breytex we've consolidated on react-docgen for prop table generation, and according to my repro above it appears to be working fine. Can you try upgrading and let me know if it's working for you?

https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#react-prop-tables-with-typescript

@Vanuan
Copy link
Contributor

Vanuan commented Mar 31, 2020

@shilman Is it available in 5.3.x? What should be changed here:

    webpackFinal: async config => {
      config.module.rules.push({
        test: /\.(ts|tsx)$/,
        use: [
          {
            loader: require.resolve('ts-loader'),
            options: {
              configFile: path.resolve(__dirname, './tsconfig.json'),
              transpileOnly: true,
            },
          },
          // Automatics props
          {
            loader: require.resolve('react-docgen-typescript-loader'),
          },
        ],
      });
      config.resolve.extensions.push('.ts', '.tsx');
      return config;
    },
  };

@shilman
Copy link
Member

shilman commented Apr 1, 2020

@Vanuan Yeah, it should be available in 5.3.x. You can should be able to add @storybook/preset-typescript@3.0.0 and get the new config.

@mkinfrared
Copy link

Still not working if I use default export.
No props found for this component

@mkinfrared
Copy link

What I also noticed is when I use named export, which is currently a working solution rignt now, docs doesn't read the children prop, even if I specifically declare that in my interface.

@shilman
Copy link
Member

shilman commented Apr 6, 2020

@mkinfrared are you using babel-plugin-react-docgen or react-docgen-typescript-loader?

@mkinfrared
Copy link

@shilman I'm using react-docgen-typescript-loader
I can push my test project to github if it can help you somehow

@shilman
Copy link
Member

shilman commented Apr 7, 2020

@mkinfrared it's a known issue in RDTL that's fixed in react-docgen. currently both solutions leave a lot to be desired. See #9556

@hipstersmoothie
Copy link
Contributor

@mkinfrared if you submit a pr to react-docgen-typescript-loader with just a test case I will try to fix it

@igloude
Copy link

igloude commented Apr 28, 2020

FWIW I discovered that <Props of={MemoizedComponent.type} /> will correctly render a props table. No idea what's going on under the hood there, lol

@shilman shilman added the react label May 23, 2020
@Somil91
Copy link

Somil91 commented Aug 23, 2020

Bump

I am on version 6 now.
If we are using CSF format to write stories for a memoized component
The props table is not getting generated, also the control tab is not getting populated correctly.

const MyComponent = React.memo(function MyComponent({param1, ) {
.....
});

Strangely it seems to work fine for normal function component.

Any workaround for the same.

@shilman
Copy link
Member

shilman commented Aug 23, 2020

@SomilKumar what happens when you set:

export default {
  title: '...',
  component: MyComponent.type,
}

@GGAGloria
Copy link

@SomilKumar what happens when you set:

export default {
  title: '...',
  component: MyComponent.type,
}

Hi I am using forwardRef in my components and met the same problem, I tried your solution like

export default {
    title: 'Input',
    component: Input.type,
    parameters: {
        docs: { page: docs }
  }, 
}

When I wrote this in my Input.stories, the props is not loaded, I even could not see the box with 'No props found for this component'.

I am on "@storybook/react": "^5.3.19".

@Somil91
Copy link

Somil91 commented Aug 24, 2020

@shilman it doesn't render the story altogether. Both for memoized component and normal functional component.

@hipstersmoothie
Copy link
Contributor

@shilman Open PR to react-docgen-typescript to fix the React.memo behavior

styleguidist/react-docgen-typescript#288

@tm1000
Copy link

tm1000 commented Sep 4, 2020

Thanks @hipstersmoothie !

Merged 11 days ago and released in v1.20.4 of react-docgen-typescript. I can confirm my deps went from react-docgen-typescript@1.20.2 to react-docgen-typescript@1.20.4

Unfortunately I still have to unwrap memo from my components or export them separately for documentation as even with your updates there is no documentation from when wrapped in memo

@hipstersmoothie
Copy link
Contributor

if you get a sample repo up I can take a look

@tm1000
Copy link

tm1000 commented Sep 4, 2020

@hipstersmoothie thanks Andrew! (I am also an Andrew!)

Here's the requested repo for you! https://github.com/tm1000/storybook-repro

Look in the stories folder. I commented by providing two examples. One with the direct export (which works) and one with the wrapped export (using memo, does not work)

@hipstersmoothie
Copy link
Contributor

Thanks!

@Somil91
Copy link

Somil91 commented Sep 12, 2020

I am not sure if this is resolved, but I am still facing the same issue, where controls ( props ) are not getting generated automatically if I wrap my components using React.memo
"@storybook/addon-essentials": "^6.0.21",
"@storybook/react": "^6.0.21",
"react-docgen-typescript@1.20.4",

@BhanuSanghi-Dev
Copy link

@SomilKumar what happens when you set:

export default {
  title: '...',
  component: MyComponent.type,
}

Oh god this solved the same problem I was facing.
Can we please add this as a note in documentation somewhere, I had wasted 2 days just to understand that it is not working because of me using memo. It would have never occurred to me that it was the problem. I had to do hundreds of trial and error to just understand this.

@tm1000
Copy link

tm1000 commented Sep 22, 2020

@BhanuSanghi-Dev well... it should be fixed looking at the code changes on react-docgen-typescript@1.20.4. So I think the intent of the developers is this is fixed so no note needs to be made. At this point it's more of a bug than anything else. Hopefully the provided repo can help out and it can be solved.

@shilman shilman added the P2 label Sep 24, 2020
@shilman shilman added this to the 6.1 docs milestone Sep 24, 2020
@shilman
Copy link
Member

shilman commented Oct 5, 2020

@andezzat this is the issue i was mentioning. i'm thinking maybe your "exotic component logic" from #12638 can fix this problem if applied here:

https://github.com/storybookjs/storybook/blob/next/addons/docs/src/frameworks/react/extractProps.ts#L31-L34

you can see that there's an attempt to do something like it already, but that it's not working as desired.

@lwaghorn
Copy link

lwaghorn commented Oct 6, 2020

I was able to resolve this by changing my component from

const myComponent = memo(()=><div>...</div>)
export default myComponent

to

const myComponent = ()=><div>...</div>
export default memo(myComponent)

and then in storybook doing the

export default {
  title: 'myComponent',
  component: myComponent.type,
  decorators: [withKnobs],
};

@andezzat
Copy link
Member

andezzat commented Oct 7, 2020

@lwaghorn

Actually, the exported component is the same in both cases.
It's passing storybook the component as myComponent.type that did the trick!

My PR should make this unnecessary 👍

@andezzat andezzat linked a pull request Oct 7, 2020 that will close this issue
5 tasks
@andezzat andezzat changed the title addon-docs: Props doenst work when using React.memo. addon-docs: Props doesn't work when using React.memo. Oct 8, 2020
@shilman
Copy link
Member

shilman commented Oct 8, 2020

Olé!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.1.0-alpha.21 containing PR #12686 that references this issue. Upgrade today to try it out!

You can find this prerelease on the @next NPM tag.

@lukemorales
Copy link

lukemorales commented Dec 16, 2020

Came to this Issue while searching for a similar problem.
Right know, props table are being generated correctly when using React.memo, however, the default value is not being filled when using Component.defaultProps.

Below is an example:

import { ButtonHTMLAttributes, memo, PropsWithChildren } from 'react';

import * as S from './styles';

export interface ButtonProps extends S.StyledButtonProps, ButtonHTMLAttributes<HTMLButtonElement> {}

const Component = (props: PropsWithChildren<ButtonProps>) => {
  const { onClick, isLoading, children, ...rest  } = props

  return (
    <S.Container onClick={onClick} isLoading={isLoading} {...rest} >
      {children}
    </S.Container>
  );
}

Component.defaultProps = {
  type: 'button',
  variant: 'primary',
  className: '',
  fixedWidth: false,
  isLoading: false,
} as ButtonProps

export const Button = memo(Component)

@shilman
Copy link
Member

shilman commented Dec 16, 2020

@andezzat is default value something you can look into?

@shunfan
Copy link

shunfan commented Sep 29, 2021

I encountered the same issue as @lukemorales had. The description and default value of each prop is not being filled.

I ended up using the workaround in the 1st comment.

@iwan-uschka
Copy link

#18136

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment