Change webpack config options behavior in 5.0.0

[sapegin]

I’m still unsure about the current webpack configuration options in 5.0.0 (next branch):

1. It tries to find webpack config by recursively looking for webpack.dev.config.js or webpack.config.js.

2. As soon as you have webpackConfigFile or webpackConfig in your style guide config it disable auto load and you have to use webpackConfigFile to specify you webpack config location.

Problems I see here:

• The 1st is different from config autoload in webpack itself.
• The 2nd already surprised a few people (including myself).

Possible solution:

1. Behave as close to webpack as possible. They load webpack.config or webpackfile with different extensions (.js, .babel.js) only in the project root folder.

2. Do not disable autoload if webpackConfig option is used. Probably webpackConfigFile: false do disable autoload explicitly.

3. Warn if there’s no webpackConfigFile or webpackConfig and config in the default location not found (just a friendly warning, not an error with exit).

4. If we don’t support .babel.js we should document it and recommend to convert config to native Node since Node 4+ supports enough ES6.

I’m quite sure about the 1st (except I don’t really believe in usefulness of .babel.js thing with Node 4+) but 2nd and 3rd are still not clear for me.

I’m trying to find the least surprising behavior. Possible not the most useful. What do you think?

Todo

• Try to load config from webpack.config.js or webpackfile.js in the project root folder.
• Get rid of webpackConfigFile.
• New option to add webpack entries.
• Warn if there’s no webpackConfig and config in the default location not found (just a friendly warning, not an error with exit).
• Update docs.
• Document how to load webpack config from a file and merge it with custom options.
• Document it and recommend to convert config to native Node since Node 4+ supports enough ES6.

[okonet]

I agree on both points. 1st should act as close to webpack behavior as possible. Is there a chance to re-use this from webpack? Not sure they have this as a module but I believe that would be useful for many tools.

2nd: I think config lookup should be prevented only if webpackConfigPath is set.

[sapegin]

They have agreed to accept a pull request with extraction this logic to a separate file (not package) but until we can drop webpack 1 support we’ll have to copypaste it.

2nd: I think config lookup should be prevented only if webpackConfigPath is set.

In this case yes. You either specify a path or Styleguidist will try to use default location. So only webpackConfig should stop autoload.

[kof]

I think the most streight forward way is to just let user always specify the webpackConfig, its also easy enough for the user to require it and pass it.

This way you have just one option instead of 2 and you can always modify whatever you want before giving it to sg.

[kof]

We are optimizing here for the case where sg can work without any config right after install, right? Is it realistic anyways?

[sapegin]

The goal is to make it work with an existing webpack configuration. And to simplify initial configuration. To be able to run and show something useful with as little configuration as possible.

This way you have just one option instead of 2…

This make sense. Right now the only difference in merging behavior is that webpackConfig allow entry when webpackConfigPath doesn’t. Ability to easily add entries for extra files (like stylesheets) is something I’d like to preserve. We may find another way to do it, not via webpack configuration. Then we could drop webpackConfigPath option.

…you can always modify whatever you want before giving it to sg.

Users don’t want to modify their already working configuration. Especially if they have no idea what they have to modify to make it work ;-)

We are optimizing here for the case where sg can work without any config right after install, right? Is it realistic anyways?

With create-react-app — yes.

[kof]

I think we have 3 logics for configuring which aimed to make the whole thing easier but potentially introduce to many options and at the end make it ambiguous. 1. Auto config finder 2. Path to a config option 3. config object. I personally would stick to one, its easier to document and understand the behavior.

[kof]

I would reduce it to one property webpackConfig which can be a string path to the config or an object.

[n1313]

I would propose going with the least config options as possible as first goal and having the least magic happening as the second goal.

So, to reduce the amount of magic, search for webpack.config.js (the most common webpack config file name) only in the root folder (the most common location) and nowhere else. Let the user handle their configuration explicitly if they want to.

And I was going to propose merging webpackConfigFile option with webpackConfig in the same way @kof just did but apparently these options are not mutually exclusive so that doesn't work.

[sapegin]

String doesn’t make any advantage over require('./webpack.config.js') in my opinion.

There are a few use cases that I don’t want to overcomplicate.

1. Overriding Styleguidist components:

webpackConfigFile: './config/webpack.config.js',
webpackConfig: {
	resolve: {
		alias: {
			// Override Styleguidist components
			'rsg-components/Logo':
				path.join(__dirname, 'styleguide/components/Logo'),
			'rsg-components/StyleGuide/StyleGuideRenderer':
				path.join(__dirname, 'styleguide/components/StyleGuide'),
		},
	},
}

2. Adding new entries:

webpackConfigFile: './config/webpack.config.js',
webpackConfig: {
    entry: [
      'babel-polyfill'
  ]
}

So I’d like to have an easy ability to merge multiple configs. We’re using webpack-merge internally, so that would work for example (I’m fine with this solution):

webpackConfig: [
   require('./config/webpack.config.js'),
   {
	resolve: {
		alias: {
			// Override Styleguidist components
			'rsg-components/Logo':
				path.join(__dirname, 'styleguide/components/Logo'),
			'rsg-components/StyleGuide/StyleGuideRenderer':
				path.join(__dirname, 'styleguide/components/StyleGuide'),
		},
	},
   }
],

(This actually won’t work for new entries.)

So webpackConfig can be an object, an array or a function.

[kof]

To me it was at least not obvious that webpackConfig option will be deeply merged with the webpackConfigFile option. Its convenient , but not obvious.

String doesn’t make any advantage over require('./webpack.config.js') in my opinion

yeah

[n1313]

By the way, it's very non-obvious to me that webpackConfig will override the config loaded from webpackConfigFile. I look at this and wonder if the order in which these things are matters, and whether switching them around (webpackConfig first, webpackConfigFile second) will change the output.

[okonet]

Well, initially I was proposing to have just one option instead of 2 as well. I think merge of config should should be explicit to remove magic. It is of course makes it harder for specific use cases but I still believe it's better.

I'd stick with auto lookup unless webpackConfig is defined. In this case it should be user's responsibility to require and merge their config.

[sapegin]

OK, some intermediate conclusions:

1. We all agree that we want only one option: webpackConfig. No webpackConfigFile anymore.

2. No recursive auto load.

3. Auto load or user config. In the latter case user is responsible to require his config and pass it to webpackConfig option.

But we still have more questions:

1. Merging: responsibility of the user or allow to pass an array of config objects?

2. Defining new entries.

[kof]

Merging: responsibility of the user or allow to pass an array of config objects?

Do u have any other choice considered point 1?

[n1313]

Ok, I'm lost, how is overriding going to work in this case?

[okonet]

Webpack supports array and object out of box so the proposal to merge if it's an array is very ambiguous. If you want to merge configs why not include webpack-merge or your preferred method as a dependency? I think optimizing for advanced users will fail either way.

[kof]

Yeah, I think its not a big deal to import a deep-merge function and do it in the config. Its more code, but its explicit.

[sapegin]

@okonet You mean webpack support it as a separate configs? This is a good point. So we’ll just suggest webpack-merge in example (we’ll need a lot of them).

So the only question is new entries.

[n1313]

We're talking about both webpacks, right? v1 and v2?

[sapegin]

@n1313 Yup. Are there any differences that we’re missing?

[n1313]

@sapegin nah, I'm just making sure that we're talking about the same thing, and that webpack 1 is not being deprecated just yet.

[sapegin]

Nooo, I’d be happy to but it’s too early 😆

[sapegin]

So what should we do with adding new webpack entries? I can only think of a new config option like this:

{
  require: [
    'babel-polyfill',
    './src/styleguide/some.css',
  ],
}

[okonet]

@sapegin why not solve both use cases via normal merge? I feel solving such edge cases will always fail since the next request will be "allow override x".

const merge = require('webpack-merge')

module.exports = {
  webpackConfig: (config) => {
     return merge(config, {
       entry: [
         'babel-polyfill',
         './src/styleguide/some.css',
       ]
    })
  }
}

[sapegin]

You can’t solve via merge because entry will be ignored (like some other fields and plugins), otherwise this option will be useless in 99% cases. And that’s exactly what we had in 4.x and it didn’t work.

[sapegin]

Out in 5.0.0-beta.15.