storybook icon indicating copy to clipboard operation
storybook copied to clipboard

Args: Support dynamic PropTypes

Open shilman opened this issue 3 years ago • 17 comments

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:

  • PropTypes can be functions, and these are difficult to analyze statically
  • PropTypes can be defined in separate files, and these are currently unsupported

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

shilman avatar Mar 02 '21 02:03 shilman

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

Sm1t avatar Mar 04 '21 15:03 Sm1t

@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.

shilman avatar Mar 04 '21 23:03 shilman

Will this support extracting ArgType select from the following?

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

malixsys avatar Mar 09 '21 22:03 malixsys

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

darkowic avatar Mar 31 '21 09:03 darkowic

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;

...

rohitnatesh avatar May 20 '21 11:05 rohitnatesh

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)

MondaySam avatar Jun 28 '21 15:06 MondaySam

@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.

Sawtaytoes avatar Sep 02 '21 21:09 Sawtaytoes

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

erosenberg avatar Sep 03 '21 18:09 erosenberg

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

Sawtaytoes avatar Sep 03 '21 22:09 Sawtaytoes

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?

keenanthomson avatar Mar 28 '22 18:03 keenanthomson

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

siminino avatar May 30 '22 08:05 siminino

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

shilman avatar May 31 '22 11:05 shilman

@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.

joshwooding avatar May 31 '22 11:05 joshwooding

@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.

IanVS avatar May 31 '22 15:05 IanVS

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.

JC-Tirado-86 avatar Jun 24 '22 21:06 JC-Tirado-86

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.

brunoenribeiro avatar Sep 07 '22 14:09 brunoenribeiro

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

  • We replaced the PropTypes library with our own, that wrapped the original one
  • We made StoryBook use our dev build of our PropTypes wrapper, so the typechecking functions would run
  • We wrote an override to the docs Props component, which set a special global flag
  • When that flag was on, all our typechecking wrappers would throw an exception indicating their type instead of returning the typechecking function
  • The Props component then performed a fake server render of the component to retrieve all the metadata from each prop declaration, and transformed this into the relevant state
  • We didn't use JSDoc comments to document our propTypes, but we added a propDescriptions property to our components which contained the text

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.

Sidnioulz avatar Oct 19 '22 14:10 Sidnioulz

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

mstade avatar Oct 14 '23 22:10 mstade