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

Prop table broken in 5.3.2 with propTypes #9415

Closed
jeffcstock opened this issue Jan 13, 2020 · 38 comments
Closed

Prop table broken in 5.3.2 with propTypes #9415

jeffcstock opened this issue Jan 13, 2020 · 38 comments

Comments

@jeffcstock
Copy link

Describe the bug
On upgrading to 5.3.2, any props defined with propTypes are not building the prop table

To Reproduce
Declare propTypes for a component. Open storybook and see that it hasn't generated a prop table.

Expected behavior
Expected to see prop information

Screenshots

Before

image

After upgrading

image

Code snippets

class Dropdown extends React.Component {
  constructor() {
    super()
    this.state = { show_action_sheet: false }
    this.toggleActionSheet = this.toggleActionSheet.bind(this)
  }

  toggleActionSheet() {
    this.setState({ show_action_sheet: !this.state.show_action_sheet })
  }

  render() {
    const {
      split,
    } = this.props

    const isMobile = detectMobile()

    let DropdownComponent

    if (split) {
      DropdownComponent = isMobile ? SplitDropdownActionSheet : SplitDropdownRegular
    } else {
      DropdownComponent = isMobile ? DropdownActionSheet : DropdownRegular
    }

    return (
      <DropdownComponent {...this.props}
        show_action_sheet={this.state.show_action_sheet}
        toggleActionSheet={this.toggleActionSheet}
      />
    )
  }
}

Dropdown.propTypes = {
  split: PropTypes.bool, // pass true for a split button. If true, must pass either href or onClick
  href: PropTypes.string, // href for split button
  onClick: PropTypes.func, // onClick handler for split button
  noCaret: PropTypes.bool, // pass true to not show a caret
  action_sheet_title: PropTypes.oneOfType([ // pass this to show a title displayed for context on action sheet
    PropTypes.string,
    PropTypes.element,
  ]),
  className: PropTypes.string,
  jane_only: PropTypes.bool,
  pullRight: PropTypes.bool,
  disabled: PropTypes.bool,
  options: PropTypes.arrayOf(PropTypes.object).isRequired, // [{}, {}]
  bsSize: PropTypes.oneOf(
    ['large', 'lg', 'small', 'sm', 'xsmall', 'xs']
  ),
  bsStyle: PropTypes.oneOf(
    ["success", "warning", "danger", "info", "default", "primary", "link"]
  ),
}

System:

  System:
    OS: macOS Mojave 10.14.6
    CPU: (12) x64 Intel(R) Core(TM) i9-8950HK CPU @ 2.90GHz
  Binaries:
    Node: 8.11.3 - ~/.nvm/versions/node/v8.11.3/bin/node
    Yarn: 1.19.2 - /usr/local/bin/yarn
    npm: 6.12.1 - ~/.nvm/versions/node/v8.11.3/bin/npm
  Browsers:
    Chrome: 79.0.3945.117
    Safari: 13.0.4
  npmPackages:
    @storybook/addon-actions: ^5.3.2 => 5.2.5
    @storybook/addon-docs: ^5.3.2 => 5.2.5
    @storybook/addon-knobs: ^5.3.2 => 5.2.5
    @storybook/addon-links: ^5.3.2 => 5.2.5
    @storybook/addons: ^5.3.2 => 5.2.5
    @storybook/react: ^5.3.2 => 5.2.5

Additional context
Add any other context about the problem here.

@shilman
Copy link
Member

shilman commented Jan 13, 2020

The props table in 5.2 had two different code paths. One that used babel-plugin-react-docgen to generate prop table data, and a fallback case that just looked at the runtime propTypes annotating a component.

The updated code in 5.3 does not have the fallback path. Since you're not seeing the props at all, I'm guessing that you've overriden your babel settings and babel-plugin-react-docgen is not getting properly applied.

cc @patricklafrance

@danilovaz
Copy link
Contributor

danilovaz commented Jan 13, 2020

I have same problem here, after upgrade to 5.3.0.

I'll try look in babel settings

@shilman
Copy link
Member

shilman commented Jan 13, 2020

@patricklafrance I think technically this was a breaking change and we should consider adding back the fallback code for 5.3, and then removing it in 6.0. WDYT?

@jeffcstock
Copy link
Author

Thanks for the helpful explanation @shilman, it explained why it was working before and I realized that we actually hadn't previously added that as a dependency as it had worked without it.

@danilovaz I fixed by adding babel-plugin-react-docgen as a dev dependency, and then adding react-docgen to plugins for the babel-loader in my webpack config (as our project doesn't use a .babelrc).

@patricklafrance
Copy link
Member

patricklafrance commented Jan 14, 2020

Its'a an option @shilman , I dont like it though.

It shouldn't have ever work without the proper babel plugin setup and it's an easy fix for the user.

If we had back the fallback, I fear we might end up with weird issues that might take a lot of our time to figure out what's happening.

Could we instead update the error message to suggest to configure properly the babel plugin required by the props table?

@shilman
Copy link
Member

shilman commented Jan 14, 2020

@patricklafrance Ok better error message is an improvement. I just think going from something that kind of works to something that doesn't work at all would be a terrific disappointment for users, and we've seen that happen again and again since that change...

@patricklafrance
Copy link
Member

@shilman I understand.

I guess it only happens to the TypeScript users? Since with 5.3 react-docgen is added out of the box.

Could we add the typescript loader to the doc preset by default? Maybe then it will solve it for MOST of the users?

If you don't use presets and have a fully customized webpack setup, I guess you can expect to make a few changes to your config between versions.

@danilovaz
Copy link
Contributor

@patricklafrance That happens with my React project and I don't use TypeScript 😢

@patricklafrance
Copy link
Member

@danilovaz that's weird!

How is it possible @shilman ?

@danilovaz
Copy link
Contributor

@patricklafrance really weird! 😞 before 5.3 it's works, but after upgrade, stopped works

An example of my proptypes

ZButton.defaultProps = {
  tag: 'button',
  isDisabled: false,
  variant: '',
  behavior: '',
  size: '',
  state: '',
};

ZButton.propTypes = {
  tag: PropTypes.oneOf(['a', 'button']),
  children: PropTypes.node.isRequired,
  className: PropTypes.string,
  isDisabled: PropTypes.bool,
  behavior: PropTypes.oneOf(['', 'block']),
  state: PropTypes.oneOf(['', 'is-loading']),
  size: PropTypes.oneOf(['', 'small', 'medium', 'large']),
  variant: PropTypes.oneOf(['', 'primary', 'secondary', 'success', 'danger']),
  onClick: PropTypes.func,
};

@brianbento
Copy link

brianbento commented Jan 14, 2020

@danilovaz Do you have a custom babel config? I'm having similar issue and noticed that if I axed my custom babel config the props table started working again. Problem then is that I can't build storybook. I can only run it locally.

Here's my babelconfig.js for reference:

module.exports = {
  plugins: ['babel-plugin-styled-components'],
  presets: ['@babel/preset-env', '@babel/preset-react'],
  ignore: ['node_modules']
}

As soon as I remove it prop tables work again.

UPDATE: Those having the same issue. I solved it by adding a blank .babelrc file to my .storybook directory.

@aareman
Copy link

aareman commented Jan 16, 2020

UPDATE: Those having the same issue. I solved it by adding a blank .babelrc file to my .storybook directory.

@danilovaz When I add a blank .babelrc I get a json parse error.

@shilman shilman added this to the 5.3.x milestone Jan 16, 2020
@advl
Copy link

advl commented Jan 19, 2020

5.3.6, broken as well. I have tried to install babel-plugin-react-docgen with no luck. I have also updated the preset as recommended in the version docs

I also have remarked that console.log(ComponentName.__docgen) yields nothing when included at the end of the file where the component is defined (after the export).

@brianbento
Copy link

UPDATE: Those having the same issue. I solved it by adding a blank .babelrc file to my .storybook directory.

@danilovaz When I add a blank .babelrc I get a json parse error.

Try copying your babel config in your root to your .babelrc in your .storybook folder.

I think this fix is also now broken in 5.3.6

@patricklafrance
Copy link
Member

patricklafrance commented Jan 20, 2020

If your component doesn't have any docgen info then, for sure it will not work.

There are a few possibilities:

You can see the final webpack configuration that storybook is running on your files with:

yarn storybook --debug-webpack

@advl
Copy link

advl commented Jan 20, 2020

@patricklafrance THanks, Patrick, unfortunately this didn't work.

I write components in ES (before webpack/babel) and stories in js (not mdx)

=> yarn storybook --debug-webpack gives me the following, which is, I suspect, the default storybook babel configuration, that executes added to my user settings defined in babel.config.jsin the root of /myproject.

{
  test: /\.mdx$/,
  exclude: /\.(stories|story).mdx$/,
  use: [
    {
      loader: 'babel-loader',
      options: {
        cacheDirectory: '/my_project/node_modules/.cache/storybook',
        presets: [
          [
            '/my_project/node_modules/@babel/preset-env/lib/index.js',
            {
              shippedProposals: true,
              useBuiltIns: 'usage',
              corejs: '3'
            }
          ],
          '/my_project/node_modules/@babel/preset-react/lib/index.js',
          '/my_project/node_modules/@babel/preset-flow/lib/index.js'
        ],
        plugins: [
          '/my_project/node_modules/@babel/plugin-proposal-object-rest-spread/lib/index.js',
          '/my_project/node_modules/@babel/plugin-proposal-class-properties/lib/index.js',
          '/my_project/node_modules/@babel/plugin-syntax-dynamic-import/lib/index.js',
          [
            '/my_project/node_modules/babel-plugin-emotion/dist/babel-plugin-emotion.cjs.js',
            { sourceMap: true, autoLabel: true }
          ],
          '/my_project/node_modules/babel-plugin-macros/dist/index.js',
          '/my_project/node_modules/@babel/plugin-transform-react-constant-elements/lib/index.js',
          '/my_project/node_modules/babel-plugin-add-react-displayname/index.js',
          [
            '/my_project/node_modules/babel-plugin-react-docgen/lib/index.js',
            {
              DOC_GEN_COLLECTION_NAME: 'STORYBOOK_REACT_CLASSES'
            }
          ]
        ]
      }
    },
    {
      loader: '@mdx-js/loader',
      options: {
        remarkPlugins: [ [Function: slug], [Function: externalLinks] ]
      }
    }
  ]
},

As you can see, the docgen plugin is actually loaded. I have tried, without much understanding, to copy my babel.config.js to the .storybook folder, without any result either.

My presets file ./.storybook/presets.js' is as follows
module.exports = ['@storybook/addon-docs/preset'];

Also I noticed console.log(Component.__docgenInfo) yields nothing

My components are all generated with this template (inside a yeoman generator): https://github.com/fwrlines/generator-react-component/blob/master/generators/app/templates/Component.js

My stories look all like this (also generated with yeoman) :
https://github.com/fwrlines/generator-storybook-story/blob/master/generators/app/templates/story.js

I have tried with Class Components as well.

In 5.2.8 this was half working : the propotypes table and defaults were detected and appeared in storybook docs, but the description of each prop didn't show at all. Now I only have a "No props found for this component" message for every component.

Edit

  • Update to 5.3.7 didn't fix it
  • Going to create a blank project with @storybook/cli to try to isolate this

Edit 2 : fixed it, unknowingly

  • I created a new storybook project with npx -p @storybook/cli sb init --type react, installed npm i -D prop-types and used the same yeoman generators for stories/components than before. Docgen works fine
  • copied my babel.config.js to the root of the new bootstrapped project. Installed the babel plugins through npm. I was wondering whether this would mess up the working config but no, docgens still works fine.
  • I tried both default es export and named exports and both work all right.
  • I mirrored the folder structure of the bugged docgen repo (by having multiple levels of exports), and it still works fine.
  • I didn't need to install babel-plugin-react-docgen
  • I didn't need to copy or create over any .babelrc file (actually I use babel.config.js of which there is only one copy, in the project root, not the .storybook folder)

Edit 3 : found the source of the bug

I was doing this :

 import(/* webpackChunkName: "css.module_name" */ './module_name.scss')

Instead of

import styles from './module_name.scss'

In files where components are defined

For a weird reason, Storybook 5.3 voids the contents of the docgen when using the first option, independently of where the dynamic import is in the file

@aareman
Copy link

aareman commented Jan 21, 2020

I have the same issue, but i don't use the import() syntax.

@shilman
Copy link
Member

shilman commented Jan 22, 2020

@779g I'm confused about your config above. The docgen plugin needs to run on your components, not your stories, so the rule you've included isn't valid. It's great that you solved your problem, but I just want to point that out in case it saves other people headaches.

@stale
Copy link

stale bot commented Feb 12, 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 12, 2020
@thoughtworks-tcaceres
Copy link

thoughtworks-tcaceres commented Mar 11, 2020

Thanks for the helpful explanation @shilman, it explained why it was working before and I realized that we actually hadn't previously added that as a dependency as it had worked without it.

@danilovaz I fixed by adding babel-plugin-react-docgen as a dev dependency, and then adding react-docgen to plugins for the babel-loader in my webpack config (as our project doesn't use a .babelrc).

Could you please paste your webpack config?

Note: was able to get it to work by adding this into the config.module.rules

{ test: /\.jsx?$/, exclude: /node_modules/, resolve: { extensions: ['.js', '.jsx'] }, use: { loader: 'babel-loader', options: { plugins: ['babel-plugin-react-docgen'] } } }

@stale stale bot removed the inactive label Mar 11, 2020
@shilman
Copy link
Member

shilman commented Apr 1, 2020

FYI, the 6.0 typescript recommendations have been updated here. I don't know whether it will fix this issue, but it's been pretty good in a lot of cases:

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

@stale stale bot removed the inactive label Apr 1, 2020
@stale
Copy link

stale bot commented Apr 23, 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!

@supersnager
Copy link

supersnager commented May 26, 2020

I have same problem here in version 5.3.18.
I'm not using Typescript, babel-plugin-react-docgen is loaded and configured, __docgenInfo is empty.
In earlier versions (don't know exactly which) it was ok.

@shilman
Copy link
Member

shilman commented May 26, 2020

@supersnager do you have a repro?

@supersnager
Copy link

@shilman unfortunately no. My project is quite complex, and it's impossible to share it now.
But i will try to reproduce this issue on sth smaller and give you know.

@supersnager
Copy link

@shilman Not prepared repro yet, but found, that props are not displayed only when component is imported to story from package (node_modules).
When component is placed directly in storybook project everthing is OK. Maybe it is some hint?

@shilman
Copy link
Member

shilman commented May 27, 2020

@supersnager Yeah that's a big hint. Storybook doesn't support props from an installed package yet! 😄

@artemAp
Copy link

artemAp commented May 29, 2020

@shilman Will you plan to support props from installed package in future?

@shilman
Copy link
Member

shilman commented May 29, 2020

@artemAp yes, if it’s even possible. A lot of the information we need might already be stripped out of the published package. At any rate, we’re still stabilizing the current props table for now

@supersnager
Copy link

@supersnager Yeah that's a big hint. Storybook doesn't support props from an installed package yet!

@shilman Thank you, that explains a lot ;) Any workaround?

@stale
Copy link

stale bot commented Jun 20, 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 Jun 20, 2020
@stale stale bot removed the inactive label Jun 23, 2020
@stale
Copy link

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

I found workaround.
Probably not working for all packages, but for me it's ok because i need this for my own package installed as module.

Just added custom webpack configuration:

  webpackFinal: async (config, { configType }) => {

    config.module.rules.push({
      include: [
        path.resolve(
          __dirname,
          "../node_modules/@my-scope/my-package-name"
        )
      ],
      test: /\.js$/,
      use: [
        {
          loader: "babel-loader",
          options: {
            plugins: [
              [
                "babel-plugin-react-docgen",
                { DOC_GEN_COLLECTION_NAME: "STORYBOOK_REACT_CLASSES" },
              ],
            ],
          },
        },
      ],
    });

    return config;
  },

@stale stale bot removed the inactive label Jul 27, 2020
@supersnager
Copy link

Another possible reason why props table is not generated can be class component.
See here: storybookjs/babel-plugin-react-docgen#66 (comment)

@shilman shilman removed this from the 5.3.x milestone Jul 30, 2020
@stale
Copy link

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

stale bot commented Oct 4, 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!

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