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

React: Disable react-docgen #7743

Closed
2 tasks
shilman opened this issue Aug 12, 2019 · 17 comments
Closed
2 tasks

React: Disable react-docgen #7743

shilman opened this issue Aug 12, 2019 · 17 comments

Comments

@shilman
Copy link
Member

shilman commented Aug 12, 2019

React-docgen can cause the terser plugin to blow up, e.g. @nicholasbraun's issue in #7092
#5935

We should:

  • Allow users to opt-out of docgen in 5.3
  • Disable docgen by default and move it into a preset in 6.0
@justinanastos
Copy link

Could another solution to be disable terser? I'm trying to build a very small project with the new docs page and am seeing the same out of memory errors during build-storybook; but running with start-storybook works file. I'd rather have a build that wasn't minified than not have props tables.

apollographql/space-kit#81

@charlieTheBotDev
Copy link

tl:dr; - Fof @storybook/react@5.2.4 you can workaround by removing react-docgen plugin from babel manually:

module.exports = ({ config }) => {
  config.module.rules[0].use[0].options.plugins.pop();
  return config;
};

More details

I've just run into this issue after adding PropTypes to my project (unfortunately not open source so I can't share a link 😓)

It always gets to 38% and then crashes. I saw the 'Use hack...' commits and tried the same. It seemingly goes into an infinite loop as it just eats all the memory I give it (tried up to 16GB, it just keeps using more and eventually crashes)

After finding this issue and seeing the plan to allow react-docgen to be disabled, I ended up removing the react-docgen plugin from the default babel config and thought this might help others until this issue is merged.

caveat - My documentation is outside of Storybook so I'm not using react-docgen there. If you want to use something that depends on it (i.e. @storybook/addons-info) my assumption would be that it won't generate the docs.

The default config uses the following babel rule:

config.module.rules (@storybook/react@5.2.4)

[
  {
    "test": {},
    "use": [
      {
        "loader": "babel-loader",
        "options": {
          "cacheDirectory": "PATH_TO_NODE_MODULES/.cache/storybook",
          "presets": [
            [
              "PATH_TO_NODE_MODULES/@babel/preset-env/lib/index.js",
              {
                "shippedProposals": true,
                "useBuiltIns": "usage",
                "corejs": "3"
              }
            ],
            "PATH_TO_NODE_MODULES/@babel/preset-react/lib/index.js",
            "PATH_TO_NODE_MODULES/@babel/preset-flow/lib/index.js"
          ],
          "plugins": [
            "PATH_TO_NODE_MODULES/@babel/plugin-proposal-object-rest-spread/lib/index.js",
            "PATH_TO_NODE_MODULES/@babel/plugin-proposal-class-properties/lib/index.js",
            "PATH_TO_NODE_MODULES/@babel/plugin-syntax-dynamic-import/lib/index.js",
            [
              "PATH_TO_NODE_MODULES/babel-plugin-emotion/dist/babel-plugin-emotion.cjs.js",
              {
                "sourceMap": true,
                "autoLabel": true
              }
            ],
            "PATH_TO_NODE_MODULES/babel-plugin-macros/dist/index.js",
            "PATH_TO_NODE_MODULES/@babel/plugin-transform-react-constant-elements/lib/index.js",
            "PATH_TO_NODE_MODULES/babel-plugin-add-react-displayname/index.js",
            [
              "PATH_TO_NODE_MODULES/babel-plugin-react-docgen/lib/index.js",
              {
                "DOC_GEN_COLLECTION_NAME": "STORYBOOK_REACT_CLASSES"
              }
            ]
          ]
        }
      }
    ],
...

I manually (somewhat hackily) removed the last plugin from that rule with:

config.module.rules[0].use[0].options.plugins.pop();

This works for the default config in @storybook/react@5.2.4 as rules[0] is the babel loader and the last plugin for that loader is the react-docgen babel plugin.

Versions used

    "@storybook/addon-actions": "5.2.4",
    "@storybook/addon-links": "5.2.4",
    "@storybook/addon-storysource": "5.2.4",
    "@storybook/addons": "5.2.4",
    "@storybook/react": "5.2.4",
    "@storybook/theming": "5.2.4",
    "react": "16.10.2",
    "react-dom": "16.10.2",

Custom webpack config

My full webpack.config.js is:

module.exports = ({ config }) => {
  config.module.rules[0].use[0].options.plugins.pop();

  config.module.rules.push(
    {
      test: /stories\.(js|jsx)?$/,
      loaders: [require.resolve('@storybook/source-loader')],
      enforce: 'pre',
    },
  );

  return config;
};

@EdenTurgeman
Copy link

@shilman I'm a little confused on how this is a solution to a max memory issue with webpack, if i need docgen in my project and want to use this feature, am i somewhere down the line prone to getting a memory issue?
I'm currently experiencing the issue after migrating some stories to CSF and working them out with docgen.
Is there any resolution to that?

@shilman shilman modified the milestones: 5.3.0, 6.0.0 Jan 11, 2020
@EdenTurgeman
Copy link

@shilman I still don't understand if this bug is just a fact of life right now if you want to work with docgen? This makes doc-gen unworkable for us, would love to clear this because I'm probably missing something ☹️

@shilman
Copy link
Member Author

shilman commented Jan 12, 2020

@EdenTurgeman Ok a few different things going on here:

I want to move as much as possible out of the core. You can see this with @storybook/preset-create-react-app. There are architectural/maintenance benefits to this. The user benefit is that these presets are actually user-configurable. So, for example, if you wanted to configure react-docgen to only run on a subset of your files, that would be possible. Right now, you're basically out of luck.

The other option is to allow disabling the terser plugin. AFAIK that's actually what's running out of memory (based on some interaction with docgen).

At any rate, neither of these things are truly storybook issues, they are issues with those specific plugins and how they interact with your project (there are tens of thousands of storybooks in the wild that don't experience this issue). From my perspective, we should support as many Storybooks as we can (including yours), but when we hit up against a wall with the libraries we depend on, the solution is to give users more control to solve it themselves. That's basically what this issue is about.

LMK if that makes sense!

@EdenTurgeman
Copy link

@shilman Absolutely! I understand your need to support as many as possible and doing that through configuration options seems to be the right way to do it.
So what you're basically saying is that our storybook and it's stories are somehow interacting in a bad way with webpack and the docgen plugin(it's terser dependency to be exact)?
I just don't fully understand if you narrowed it down to a terser issue that might resolve with future updates to that plugin or is that some user behavior causing it.

@shilman
Copy link
Member Author

shilman commented Jan 12, 2020

Here's my crude understanding:

  • Webpack chains together a sequence of loaders. Each loader is a string-to-string transformation that reads in a file and outputs an updated file.
  • The babel docgen plugin adds some docgen annotations to each source file, making each file a bit larger (maybe a lot larger in some cases? I'm not sure!)
  • Then later, the terser plugin comes along and reads the file and minimizes it, maybe using some memory-intensive approach. The larger the input file, the larger the memory consumption.

What I've seen, anecdotally, is that terser works much better with smaller input files. In your case, I don't know how much of it is due to your code vs docgen vs terser vs maybe some other loaders that you're ALSO using (??!!) but that's probably what's giving you problems.

@EdenTurgeman
Copy link

@shilman
First of all, thank you so much for taking the time to explain the issue thoroughly.
Second, I have narrowed down the issue to disabling the react-docgen-typescript-loader plugin. it seems that this plugin is the cause of all my problems right now, and is causing the javascript out of memory issue as well as slowing my build times significantly. while looking into this issue i've disabled it for our project which gives up the ability to show props in our docs (which was most of the point for us)

I know this isn't a storybook official plugin, but in your opinion, does docgen need to support typescript docs natively?

@shilman
Copy link
Member Author

shilman commented Jan 14, 2020

@EdenTurgeman That's really useful to know. Apparently react-docgen@5.0 has typescript support built in, so hopefully we can adopt that in the next release (6.0, ETA spring 2020). I don't know whether it will solve the problem, but it will certainly make the whole system simpler. If you're interested in contributing on that project, I'd love to loop you in. Right now I think @patricklafrance and I are both signed up to rework this stuff, but I think it will be a pretty chunky project. 😁

@atanasster
Copy link
Member

@EdenTurgeman - I have published a similar /but lighter / https://github.com/atanasster/webpack-react-docgen-typescript

Can you please try it and see if it solves your issues

@shilman shilman added maintenance User-facing maintenance tasks and removed feature request labels Jan 17, 2020
@EdenTurgeman
Copy link

@shilman Would love to contribute to the project in my free time, though I think i might start with something simpler to get me into the loop.
Also, is there an open issue on adding native typescript support for 6.0 where we can discuss it?

@justdanallen
Copy link

I had this same problem. I also found it mainly contributed to the react-docgen-typescript-loader plugin. I was able to get around it by turning off sourceMaps in the babel-loader options. I haven't tried the loader @atanasster suggested.

@shilman
Copy link
Member Author

shilman commented Feb 12, 2020

FYI, I'm looking at the 6.0 move to react-docgen for typescript this week

@EdenTurgeman
Copy link

Awesome, would love to see some updates on that!

@shilman
Copy link
Member Author

shilman commented Feb 13, 2020

@EdenTurgeman work in progress #9838

tay1orjones added a commit to carbon-design-system/carbon-addons-iot-react that referenced this issue Feb 24, 2020
yarn build:storybook was failing locally due to a node out of memory error. This appears to be related to react-docgen, which should be fixed in v6 of storybook. See storybookjs/storybook#7743
@shilman shilman modified the milestones: 6.0.0, 6.0 breaking changes Mar 10, 2020
@benmvp
Copy link

benmvp commented May 15, 2020

FYI - I just ran into this issue and spent several hours trying to debug and fix the problem.

My problem also was with react-docgen-typescript-loader but it's what I'm using to document my TypeScript components, so there's no way I could take it away. I followed @justdanallen's suggestion about turning off sourcemaps but it was still sometimes blowing up, even w/ an increased bundle size.

But I noticed that my bundle size jumped from 5.5MB to 25.5MB when I upgraded @svgr/core which I was using to turn *.svg files into components. I have a page that was displaying all of those SVG components and the upgrade of the package somehow made react-docgen-typescript-loader parse it better such that it generated tons and tons of data for 300+ components. Therefore the build exploded and so did node.

So what I did is just exclude react-docgen-typescript-loader from the files I didn't need it to create docs for, namely all of the icon components:

module.exports = ({ config }) => {
  config.module.rules.push(
    // Include react-docgen-typescript-loader to help generate prop type
    // docs for components. Exclude the icons folder because we don't need
    // to generate docs for those components and there are hundreds of them.
    {
      test: /\.tsx?$/,
      exclude: resolve(__dirname, '../src/icons'),
      use: [
        'babel-loader',
        {
          loader: 'react-docgen-typescript-loader',
          options: { shouldExtractLiteralValuesFromEnum: true },
        },
      ],
    },
    // Do regular babel-loader for just the icons
    {
      test: /\.tsx?$/,
      include: resolve(__dirname, '../src/icons'),
      use: ['babel-loader'],
    },
  );

  return config;
};

That brought the bundle size back down to 5.5MB and everything works like before! 🎉

@shilman shilman self-assigned this May 22, 2020
@shilman
Copy link
Member Author

shilman commented May 22, 2020

Fixed as part of #10790

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

7 participants