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

User Webpack config auto discovery #255

Closed
sapegin opened this issue Dec 2, 2016 · 8 comments
Closed

User Webpack config auto discovery #255

sapegin opened this issue Dec 2, 2016 · 8 comments

Comments

@sapegin
Copy link
Member

sapegin commented Dec 2, 2016

With all changes I’ve already made for the upcoming 5.0.0 release (especially removal of babel-loader and css-loader) it might be possible to simplify things even further and make it “just works” at least for some users.

Goals

  1. Can work without config in Create React App projects by default.
  2. Try to find and use user’s Webpack config by default.
  3. Magic can be disabled.

How that should work

  1. If there’s no styleguide.config.js or webpackAutoDiscovery option isn’t false.
  2. Find Webpack config:
    a. If project has react-scripts dependency, use react-scripts/config/webpack.config.dev.js.
    b. Try to find webpack.config.dev.js or webpack.config.js.
  3. Merge it with Stylegudist config, skip entry, output, plugins sections.
  4. Apply webpackConfig and updateWebpackConfig options if they are present in style guide config.

Will that work? What have I missed? What can be improved?

/cc @MoOx @okonet

@sapegin sapegin added this to the 5.0.0 milestone Dec 2, 2016
@okonet
Copy link
Member

okonet commented Dec 2, 2016

I have only one minor note:

Try to find webpack.config.dev.js or webpack.config.js

It should probably firstly try to locate webpack.config.js. I even not sure it should go further since, by default webpack will do the same: it will try locate webpack.config.js otherwise you'll have to provide the path the config.

One more thing is that isn't clear from the above is what will it take to add the path the webpack config to styleguide.config.js.

@okonet
Copy link
Member

okonet commented Dec 2, 2016

If there’s no styleguide.config.js or webpackAutoDiscovery option isn’t false.

What happens if there is styleguide.config.js. Can you add a webpackConfigPath similarly how https://github.com/benmosher/eslint-plugin-import does?

I'm not sure about the webpackAutoDiscovery option either. What's the purpose for it?

I think, with CRA it should definitively just work out of the box. With the manual installation, I think it's okay to have the styleguide.config.js with webpackConfigPath (it still should by default try to load webpack.config.js and if this fails, tell about that).

I think it's less magic but still very simple.

@sapegin
Copy link
Member Author

sapegin commented Dec 2, 2016

by default webpack will do the same: it will try locate webpack.config.js otherwise you'll have to provide the path the config.

Could you point me to the code where they do it?

What happens if there is styleguide.config.js

The same as before but now it’s optional.

Can you add a webpackConfigPath

That’s very good idea!

I'm not sure about the webpackAutoDiscovery option either.

To disable magic. webpackConfigPath: false could do the same.

I think, with CRA it should definitively just work out of the box. With the manual installation, I think it's okay to have the styleguide.config.js with webpackConfigPath (it still should by default try to load webpack.config.js and if this fails, tell about that).

Exactly how I see it.

@okonet
Copy link
Member

okonet commented Dec 2, 2016

Could you point me to the code where they do it?

Nope, I have no idea where it is. Search the repo?

The same as before but now it’s optional.
To disable magic. webpackConfigPath: false could do the same.

Exactly. That's why I think for standalone apps this is too much magic.

Exactly how I see it.

It's settled then? 👏

@sapegin
Copy link
Member Author

sapegin commented Dec 2, 2016

We should try and see if that would work. I don’t see any problems for CRA, for Webpack projects only real testing will tell. I also want to include it in 5.0.0 release because it already has some breaking changes and this is a breaking change too.

@tizmagik
Copy link
Collaborator

tizmagik commented Dec 2, 2016

I think this is great! Let me know if I can assist 👍

@sapegin
Copy link
Member Author

sapegin commented Dec 2, 2016

@tizmagik We’ll need help with testing this at least ;-)

@sapegin sapegin mentioned this issue Dec 8, 2016
9 tasks
@sapegin
Copy link
Member Author

sapegin commented Dec 8, 2016

As @bebraw suggested it should check if Webpack config exports a function and call it with a current environment as a first argument.

@sapegin sapegin closed this as completed Jan 5, 2017
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

3 participants