strothj/react-docgen-typescript-loader

Suggestion: consider removal of `includes` and `excludes` options

amacleay opened this issue · 7 comments

Reason: duplicative with Webpack's Rule.exclude and Rule.include options.

I bring this up because I am using a non-standard Storybook webpack configuration: I want react-docgen-typescript to run on a particular node_modules folder. It took me quite a lot of debugging to figure out that, even though I was invoking invoking this loader for my files, I was not running the loader on these files because they matched the default excludes.

@storybook/react@3.4.7, @storybook/core@3.4.7, react-docgen-typescript-webpack-plugin@1.1.0, react-docgen-typescript@1.6.0

Sample webpack configuration:

// cat webpack.config.js
module.exports = (baseConfig, env, config) => {
  config.module.rules.push({
    test: /\.(ts|tsx)$/,
    // Exclude all node_modules EXCEPT @corp/components
    // so that the plugins pick up the source code from that lib
    exclude: /\/node_modules\/(?!@corp\/components)/,
    use: [
      require.resolve('awesome-typescript-loader'),
      require.resolve('react-docgen-typescript-loader'),
    ],
  });
  return config;
};

Fixed version w/ hacky excludes option:

// cat webpack.config.js
module.exports = (baseConfig, env, config) => {
  config.module.rules.push({
    test: /\.(ts|tsx)$/,
    // Exclude all node_modules EXCEPT @corp/components
    // so that the plugins pick up the source code from that lib
    exclude: /\/node_modules\/(?!@corp\/components)/,
    use: [
      require.resolve('awesome-typescript-loader'),
      {
        loader: require.resolve('react-docgen-typescript-loader'),
        options: {
          excludes: [],
        },
      },
    ],
  });
  return config;
};

(Incidentally, I believe that the way the plugin acts with an includes parameter but without an excludes parameter is confusing: if I use options: { includes: [new RegExp('node_modules/some_module')] } the loader will not pick up node_modules/some_module/some_file.tsx; to make it work I would need to additionally provide an empty excludes, as in options: { includes: [new RegExp(node_modules/some_module'), excludes: [] })

You're using a monorepo setup correct? I think I ended up with a similar situation when doing something similar. I'll think this over in more detail on Sunday and see what direction I'd like to take here.

I'm looking into this.

The original concern for me was that I was using the babel-typescript-preset. The same loader is used for both Javascript and TypeScript compilation. I was also hoping to make the startup process of Storybook less CPU intensive on large projects by limiting the docgen to only Storybook stories.

I don't think this choice has made a measurable difference, the performance is still awful.

Yes, I'm essentially using a monorepo setup.

(In fact, it's not actually currently set up as a monorepo, but it is an obvious candidate: more than one repo of component libraries, but one storybook repo to showcase together)

Thanks for taking a look!

@amacleay

Hello,

I've released a release candidate with this change.

You can review the change log here:
https://github.com/strothj/react-docgen-typescript-loader/blob/v3/README.md

The package is available as react-docgen-typescript-loader@next.

I'd like to release v3 this coming Wednesday. If you have any problems or feedback, it would be helpful.

I tested with your new code and it does seem to do what's on the tin. Thanks!

While testing, though, I ran into another issue which requires a bit of work. I'll add that as another issue shortly. Thanks for all your work.

This should be closed by 3.0.0, right?

This should be closed by 3.0.0, right?

Yes, this was taken care of as v3. Thanks.