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 - build prop table from dynamic propTypes? #10536

Closed
azinod opened this issue Apr 24, 2020 · 22 comments
Closed

Addon Docs - build prop table from dynamic propTypes? #10536

azinod opened this issue Apr 24, 2020 · 22 comments

Comments

@azinod
Copy link

azinod commented Apr 24, 2020

Describe the bug

Using the Addon-Docs, I am not being able to get a prop table from a component that has dynamic propTypes. By dynamic propTypes I mean: propTypes built by destructuring objects into the propTypes object rather than defining static properties, which is what we normally do.

This approach is used by styled-systems and similar libraries.

To Reproduce

Create a component with dynamic propTypes (like the one below) in a React application with storybook and Addon-Docs and try to get the prop table for that component with the dynamic/destructured propTypes in it.

What happened when I tried was:

  • static properties added in the propTypes are displayed as expected;
  • dynamic/destructured propTypes properties are not displayed in the props table;
  • also, in my tests, props destructured in the arguments show up on the prop table if they have default values. So, in the case below, prop2 is displayed even if I omit it from the propTypes, but prop1 isn't, apparently due to not having a default value. Adding a default value to it get it to display;

Code snippet

import React from 'react';
import PropTypes from 'prop-types';
import styled from 'styled-components';
import {space, color} from 'styled-system';
import propTypes from '@styled-system/prop-types';

const StyledDiv = styled.div`
  ${space}
  ${color}
`;

const Box = ({prop1, prop2='default_value', ...otherProps}) => (
  <StyledDiv
    prop1={prop1}
    prop2={prop2}
    {...otherProps}
  />
);

Box.propTypes = {
  prop1: PropTypes.any,
  prop2: PropTypes.any,
  ...propTypes.space,
  ...propTypes.color
};

export default Box;

Expected behavior

All props in propTypes to display in the prop table, including the dynamic ones added via destructuring.

System:

Environment Info:

System:
OS: Linux 4.4 Ubuntu 16.04.3 LTS (Xenial Xerus) (actually using windows WSL)
CPU: (4) x64 Intel(R) Core(TM) i5-5300U CPU @ 2.30GHz
Binaries:
Node: 10.19.0 - ~/.nvm/versions/node/v10.19.0/bin/node
Yarn: 1.17.3 - /mnt/c/Program Files (x86)/Yarn/bin/yarn
npm: 6.13.4 - ~/.nvm/versions/node/v10.19.0/bin/npm
npmPackages:
@storybook/addon-a11y: ^5.3.18 => 5.3.18
@storybook/addon-actions: ^5.3.18 => 5.3.18
@storybook/addon-docs: ^5.3.18 => 5.3.18
@storybook/addon-knobs: ^5.3.18 => 5.3.18
@storybook/addon-links: ^5.3.18 => 5.3.18
@storybook/addon-storysource: ^5.3.18 => 5.3.18
@storybook/addon-viewport: ^5.3.18 => 5.3.18
@storybook/addons: ^5.3.18 => 5.3.18
@storybook/react: ^5.3.18 => 5.3.18

@shilman
Copy link
Member

shilman commented Apr 24, 2020

does it work in react-docgen? if not, i don't think it will ever work in storybook since we rely on that to generate the props tables. we have some hacked up (sorry @patricklafrance, not trying to insult) code that does some degree of dynamism, but there's only so far we can go with this approach ... and it's a maintenance nightmare.

@patricklafrance the other option is to make propType processing fully dynamic. i think we had this at one point but you deleted it. bringing this back might actually be simpler than the current setup. WDYT?

@azinod
Copy link
Author

azinod commented Apr 24, 2020

@shilman docgen goes like this for me:
✔ Description for the component, placing a docgen right above component definition in the component's file
✔ Description for static propTypes, placing a docgen right above it
❌ Description for anything in propTypes object that is not a static property (i.e. destructured/dynamic props)
❌ Description for stories, placing a docgen right above a story (CSF)
❌ Descriptions anywhere else before (tried different spots before, after and between stories to see if anything would show up, just to be sure)

image

@shilman
Copy link
Member

shilman commented Apr 24, 2020

Description for stories, placing a docgen right above a story (CSF)

See #8527 and the the community-created webpack loader

@patricklafrance
Copy link
Member

patricklafrance commented Apr 24, 2020

I don't think this ever worked @shilman even before my changes since the props table always relied on docgen to loop through the component props and docgen doesn't return props from external objects.

It works when using a spread operator for a local object, but not for an external object.

BTW, you should add this use case to your recurring issues docs since it's like the 15th issues about this ;)

@Smolations
Copy link

I feel like this is a dupe because I've had this exact same issue.

@azinod There's an unfinished PR open in the react-docgen project that was aimed to address this, so you might wanna upvote/nudge/etc. in that thread to try and keep things moving. 👍

@shilman
Copy link
Member

shilman commented Apr 26, 2020

@patricklafrance I'm thinking about bringing back the dynamic propTypes code that you deleted in 5.3 and using that to handle this case. WDYT?

@patricklafrance
Copy link
Member

@shilman no strong opinion, if you feel you need it, go ahead.

@shilman
Copy link
Member

shilman commented Apr 27, 2020

@patricklafrance Do you think it will also handle what we’re currently using acorn for?

@patricklafrance
Copy link
Member

@shilman no

@jtieu-r7
Copy link

jtieu-r7 commented May 7, 2020

Hey I'm also running into this issue, +1 for a potential solution.

If I'm reading this correctly, there is no existing solution for generating a props table for any components propTypes that are composed from another imported component's propTypes?

@shilman
Copy link
Member

shilman commented May 7, 2020

@jtieu-r7 currently that's the case, do to limitations of react-docgen. if we support runtime generation of the prop types we can support arbitrary JS, but at the cost of a bunch of features (e.g. jsdoc comments, probably), and some added complexity.

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

Hi @shilman , I started using addon-docs and see the dynamic prop types problem.
Is there another way to render props table to the screen?

@shilman
Copy link
Member

shilman commented Jun 8, 2020

In SB6 you can manually define argtypes that will get rendered into the table: https://github.com/storybookjs/storybook/blob/next/addons/docs/docs/props-tables.md#customizing-argtypes

@ofekshalom
Copy link

I want to create costume argTypes in MXD format. I didn't see an example of creating costume argTypes in MDX format in the docs.

@stale
Copy link

stale bot commented Jul 3, 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 Jul 3, 2020
@stale
Copy link

stale bot commented Aug 2, 2020

Hey there, it's me again! I am going close this issue to help our maintainers focus on the current development roadmap instead. If the issue mentioned is still a concern, please open a new ticket and mention this old one. Cheers and thanks for using Storybook!

@stale stale bot closed this as completed Aug 2, 2020
@lobaak
Copy link

lobaak commented Oct 29, 2020

This is still an issue

@c10b10
Copy link

c10b10 commented Dec 10, 2020

The issue may have been resolved in react-docgen:

reactjs/react-docgen#352
reactjs/react-docgen#464

@jguffey
Copy link

jguffey commented Apr 29, 2021

Seems to still be an issue in 6.2.8

@yairEO
Copy link
Contributor

yairEO commented Jun 2, 2021

This makes storybook automatic ArgsTable not detect any propTypes, highly likely for the majority of serious projects, where proptypes are not all statically written but imported and also dynamically-built.

For Docs to be truly beneficial, automatic propTypes detection must be bulletproof.
If react-docgen does not supply the goods, maybe another package does, and if not, then I suggest writing one from scratch. Solving automatic proptypes documentation is quite an important task.

It is extremely unreliable & hassle for developers to manually maintain both the "real" propTypes (on the components themselves) and in another file, only for Storybook presentation, especially for large projects with dozens of developers at different skill levels

@shilman
Copy link
Member

shilman commented Nov 17, 2021

closing this as dupe to #14092

@shilman shilman closed this as completed Nov 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants