storybookjs / storybook

Storybook is a frontend workshop for building UI components and pages in isolation. Made for UI development, testing, and documentation.
https://storybook.js.org
MIT License
83.35k stars 9.11k forks source link

Args: Support dynamic PropTypes #14092

Open shilman opened 3 years ago

shilman commented 3 years ago

What

Inspect a component's runtime propTypes to determine what to show in its ArgsTable

Why

Currently, Storybook extracts ArgTypes for JS React components using react-docgen. React-docgen performs a file-by-file static analysis of your components. This is efficient, generally well-supported, and is able to pull out things like JSDoc comments from the source code. However, it has a couple issues:

By inspecting the runtime propTypes of a component and using this as a fallback (or combining it with the docgen?) we can overcome some of these problems.

We chose not to do this in the original implementation because it can easily obscure issues in the docgen setup. However, in practice many users want it, so we should consider adding this as part of React's ArgType extraction.

User story

As a component developer,
I want Storybook to automatically dynamic PropTypes
So that I don't need to manually document them

Related issues

13252 #12798 #12816

Sm1t commented 3 years ago

@shilman will https://github.com/reactjs/react-docgen/pull/464 fix this?

shilman commented 3 years ago

@Sm1t it should address part of the problem (externally defined props) but not all of it (dynamic props defined in functions). and it requires some additional work on the storybook side to integrate.

malixsys commented 3 years ago

Will this support extracting ArgType select from the following?


LevelPicker.propTypes = {
  /**
   * Selected level
   */
  level: PropTypes.oneOf(['None', 'Read', 'ReadWrite', 'Manage', 'Owner']),
};```
darkowic commented 3 years ago

Found one more case:

const someDefaultProps = { prop: 'default' }

....

defaultProps = {
...someDefaultProps,
prop: 'override'
}

The value default is passed to the component instead of 'override'. Tested on 6.2

rohitnatesh commented 3 years ago

We tried setting up propTypes in a separate file and import it into the component file. But was disappointed as it does not work. :(

Something like,

import propTypes from './propTypes';

....

Component.propTypes = propTypes;

...
MondaySam commented 3 years ago

I stumbled upon this very issue today:

1) Two component ReactJS files Button.js and SpecialButton.js 2) Button.js includes a small object with button sizes: export const BTN_SIZES = {SMALL: 'small', REGULAR: 'regular', LARGE: 'large'} 3) SpecialButton.js imports the Button.js as well as the size class: import {Button, BTN_SIZES} from "./Button"; 4) In the PropTypes of both Button and SpecialButton, I define size as follows: size: PropTypes.oneOf(Object.values(BTN_SIZES)),

Now PropTypes.oneOf reads Object.values(BTN_SIZES) as a String and returns a dropdown with numbers from 0 to 23 (the length of the string)

Sawtaytoes commented 2 years ago

@shilman For dynamic values, is there a good workaround?

A coworker just ran into this issue. I'm assuming you have to manually add those to the argsTable to fix it for now.

erosenberg commented 2 years ago

I can confirm that this is happening for me as well.

Sawtaytoes commented 2 years ago

I think the only workaround is to use argsTable as a manual replacement for these values.

keenanthomson commented 2 years ago

Bumping this issue -- generating dynamic PropType lists from imported enums would be a really valuable feature. Are there any plans to tackle this in 2022?

siminino commented 2 years ago

Hey, it seems a new version of React Docgen (v5.4.1) was just release supporting some kind of dynamic PropTypes. Could we expect having a new release of storybook including this version? https://github.com/reactjs/react-docgen/releases/tag/v5.4.1

shilman commented 2 years ago

nice @siminino !! i'd love to kick the tires on this in 7.0 -- might be already possible using the vite builder cc @IanVS @joshwooding

joshwooding commented 2 years ago

@shilman @siminino We currently use the v6 alpha in the vite builder: https://github.com/storybookjs/builder-vite/blob/e9da98e86f56996596678ac6892ebf067c209e30/packages/builder-vite/package.json#L26 might be worth asking if there are plans to release a new alpha with those changes.

IanVS commented 2 years ago

@siminino this is an in-range update to react-docgen, which will be picked up by babel-plugin-react-docgen. If you'd like to try it out, you can either remove your lockfile and node_modules, and re-install (which should give you up-to-date transitive dependencies), or you might be able to fiddle with the lockfile directly to get a new version of just react-docgen. yarn up might also work, though I'm not sure of its effect on transitive deps.

JC-Tirado-86 commented 2 years ago

I know may be is too late to comment on this thread but taking @MondaySam as an example.

If you put constant BTN_SIZES in SpecialButton.js file then the issue is gone. Apparently this is not working when you are importing the constant from another location.

Still an issue but this is a workaround.

Hope this helps.

brunoenribeiro commented 1 year ago

Stumbled upon this issue as well.

We used argTypes as a workaround. @MondaySam's example would look like this:

// button.js
export const BTN_SIZES = { SMALL: 'small', REGULAR: 'regular', LARGE: 'large' }

// button.stories.js
import Button, { BTN_SIZES } from './button';

export default {
  title: 'Button',
  component: Button,
  argTypes: {
    size: {
      options: Object.values(BTN_SIZES)
    }
  }
}

It's not as good as inferring from PropTypes, but at least it keeps a single source of truth.

Sidnioulz commented 1 year ago

In my previous company, we built another, much more comprehensive workaround.

It was an insane endeavour, but it worked without maintenance, for all prop types, including our own custom ones. I suppose all the default PropTypes could be natively supported by aliasing the prop-types imports through to a wrapper; and such a wrapper could have a declarative API to add custom PropTypes... I don't know if such code could ever be considered robust, though.

mstade commented 8 months ago

@shilman any news on this? Does anyone know of a decent workaround to this issue?