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

Support webpack.config.ts #3780

Closed
Ailrun opened this issue Jun 21, 2018 · 18 comments
Closed

Support webpack.config.ts #3780

Ailrun opened this issue Jun 21, 2018 · 18 comments

Comments

@Ailrun
Copy link
Contributor

Ailrun commented Jun 21, 2018

Support request summary

Webpack supports something like webpack.config.ts with https://github.com/js-cli/js-interpret, as they mentioned in https://webpack.js.org/configuration/configuration-languages/.
However, in storybook, it detects fixed webpack.config.js in https://github.com/storybooks/storybook/blob/8757938b4104e96e4d33c292329222cdd3a68e98/lib/core/src/server/config.js#L89
This makes it hard to centralize webpack config, when I use webpack.config.ts.

Please specify which version of Storybook and optionally any affected addons that you're running

  • @storybook/core 3.4.7

Affected platforms

  • Webpack
  • TypeScript
@Ailrun
Copy link
Contributor Author

Ailrun commented Jul 3, 2018

Resolved in #3785.

@Ailrun Ailrun closed this as completed Jul 3, 2018
@issue-sh issue-sh bot removed the merged label Jul 3, 2018
@swernerx
Copy link
Contributor

swernerx commented Jul 4, 2018

Seems that there is some issue when dealing with "normal" "webpack.config.js". After updating I get this error:

WARN => Custom configuration file /Users/swerner/Workspace/init/display/node_modules/edge-storybook/lib/webpack.config.js is detected
WARN    but impossible to import loader for .js
INFO Using default webpack setup based on "create-react-app".

I overlooked this first as this does not throws (which it probably should do). Also I am wondering what it should do for configs which do not require transpilation.

The "interpret" package returns null for both .js and .json it seems. Which affects registerCompiler which returns 0 in this case which leads to this warning.

@Ailrun
Copy link
Contributor Author

Ailrun commented Jul 4, 2018

@swernerx Why do you think it should throw?

@swernerx
Copy link
Contributor

swernerx commented Jul 4, 2018

I think it should throw because there might be unwanted side-effects if my very own config is not loaded. This has to be very explicit and visible to the user. Ignoring the issue somewhat by only raising a warning is IMHO not enough. Just falling back to some default is clearly not what the user expects when defining a custom webpack config.

But this is really another discussion. The bug right now is that actually two folded:

  1. Should correctly handle .js configs like before without showing a warning.
  2. Should output a warning or even throw when requiring the custom webpack config fails.

The second issue is about this code:

      try {
        return requireConfig(customConfigPath);
      } catch (e) {
        // do nothing
      }

We really shouldn't hide e.g. parse errors.

@Ailrun
Copy link
Contributor Author

Ailrun commented Jul 4, 2018

For 1., handling .js or .json is not that hard (since interpret package have null interpreter for them), and for 2., ye I think you're right. Could you make a PR? Nowdays I'm little busy because of work...

FYI, you can use permalink to link specific code like following (which gives a hyperlink to the file)
https://github.com/storybooks/storybook/blob/466b8dff9ec0b4d0e0d64df0fad8dc7b18bc3e48/lib/core/src/server/loadCustomWebpackConfig.js#L77-L81

@swernerx
Copy link
Contributor

swernerx commented Jul 8, 2018

I'll do a PR. Will probably be done during the this week.

@pelotom
Copy link
Contributor

pelotom commented Jul 10, 2018

So is loading a custom webpack.config.js just completely broken right now? Is there any workaround?

@Ailrun
Copy link
Contributor Author

Ailrun commented Jul 10, 2018

@pelotom It works well. It just displays annoying warning when you use .js extension for custom webpack config.

@sakulstra
Copy link

are you sure? I just tried upgrading from latest 3.x to 4.0.0-alpha.13 and it seems like at least webpack aliases defined in webpack.coonfig.js aren't loaded any more.

@pelotom
Copy link
Contributor

pelotom commented Jul 11, 2018

One thing I discovered is that currently if webpack.config.js throws, the error is swallowed and it just silently uses the default config...

@Ailrun
Copy link
Contributor Author

Ailrun commented Jul 11, 2018

@pelotom Silently? I mean, at least it will displays Using default webpack setup based on "create-react-app" or something like that. If you mean "There is no displayed error" by "silently", yes, you are right, and of course displaying error is better and that's what @swernerx said in previous comment.

@sakulstra At least I tested it with my commercial project and storybook examples for .js file, and with my personal project for .ts file.

@pelotom
Copy link
Contributor

pelotom commented Jul 11, 2018

@Ailrun

Silently? I mean, at least it will displays Using default webpack setup based on "create-react-app" or something like that. If you mean "There is no displayed error", yes, you are right, and of course that's better and that's what @swernerx said in previous comment.

I thought @swernerx' comment was about if you use an unsupported language extension for your webpack config... I'm talking about there being a thrown error inside your webpack config.

@Ailrun
Copy link
Contributor Author

Ailrun commented Jul 11, 2018

@pelotom See 2. of #3780 (comment), he mentioned about error emitted while requiring custom webpack config.

@pelotom
Copy link
Contributor

pelotom commented Jul 11, 2018

@Ailrun I see, thanks

@sirtimbly
Copy link

Just to be clear: I believe custom webpack.config.js isn't even being loaded right now. I was able to get a custom webpack.config.js to work when I went back to alpha.10 (before this change went in) Do you have tests you can use to confirm custom webpack configs are actually being used?

@Ailrun
Copy link
Contributor Author

Ailrun commented Aug 9, 2018

@sirtimbly yes, I tested with the custom webpack files in the example. When custom on didn't load, it gave me an error.

@Ailrun
Copy link
Contributor Author

Ailrun commented Aug 9, 2018

Probably your webpack config have problem and errors are swallowed by that try-catch.

BR0kEN- added a commit to BR0kEN-/storybook that referenced this issue Aug 23, 2018
BR0kEN- added a commit to BR0kEN-/storybook that referenced this issue Aug 23, 2018
BR0kEN- added a commit to BR0kEN-/storybook that referenced this issue Aug 24, 2018
BR0kEN- added a commit to BR0kEN-/storybook that referenced this issue Aug 24, 2018
igor-dv added a commit that referenced this issue Aug 24, 2018
#3780: Do not display wrong warning when loading ".js" or ".json"
@1999
Copy link

1999 commented Jun 20, 2019

I can't say that this message is very helpful: I'm trying to set up the webpack config, webpack.config.ts file exists but it obviously doesn't even get read and this message doesn't say anything about how to fix it.

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

8 participants