Add Rule.rules

[opl-]

Adds support for nested rules.

Closes #219

[opl-]

Force push to fix build. Somehow managed to forget trailing commas.

(The second force push is to fix the test order to keep rules always before oneOf, as it was in every place but that one test.)

[edmorley]

Hi! Thank you for adding support for this!

I think the toString handling here needs adjusting for nested rules:

webpack-chain/src/Config.js

Lines 83 to 88 in afdf962

	}')${value.__ruleNames
	.slice(1)
	.map(r => `.oneOf('${r}')`)
	.join('')}${
	value.__useName ? `.use('${value.__useName}')` : ``
	} */\n`;

Could you also add a usage of the new nested rules to the existing toString test here?

webpack-chain/test/Config.js

Lines 387 to 445 in 6d3c5c3

	test('toString', t => {
	const config = new Config();
	config.module
	.rule('alpha')
	.oneOf('beta')
	.use('babel')
	.loader('babel-loader');
	const envPluginPath = require.resolve('webpack/lib/EnvironmentPlugin');
	const stringifiedEnvPluginPath = stringify(envPluginPath);
	class FooPlugin {}
	FooPlugin.__expression = `require('foo-plugin')`;
	config
	.plugin('env')
	.use(envPluginPath, [{ VAR: false }])
	.end()
	.plugin('gamma')
	.use(FooPlugin)
	.end()
	.plugin('delta')
	.use(class BarPlugin {}, ['bar'])
	.end()
	.plugin('epsilon')
	.use(class BazPlugin {}, [{ n: 1 }, [2, 3]]);
	config.resolve.plugin('resolver').use(FooPlugin);
	config.optimization.minimizer('minifier').use(FooPlugin);
	t.is(
	config.toString().trim(),
	`
	{
	resolve: {
	plugins: [
	/* config.resolve.plugin('resolver') */
	new (require('foo-plugin'))()
	]
	},
	module: {
	rules: [
	/* config.module.rule('alpha') */
	{
	oneOf: [
	/* config.module.rule('alpha').oneOf('beta') */
	{
	use: [
	/* config.module.rule('alpha').oneOf('beta').use('babel') */
	{
	loader: 'babel-loader'
	}
	]
	}
	]
	}
	]
	},

With those changes, this should be good to merge :-)

[opl-]

Well, I completely missed that. I considered a few ways of doing this, but eventually settled on just making __ruleNames an array of arrays, considering it is an implementation detail and therefore I believe changing it shouldn't be considered a breaking change.

This also fixes the hints for defaultRules being incorrect while slightly simplifying the code.

While working on this I realized that it should be possible to generate the paths on the fly in Module.toConfig, Rule.toConfig and Use.toConfig, which would allow reusing the same Rule instance in several places without the toString output being invalid, but I felt like that was outside of the scope of this PR. This could also be misleading, considering that calling .end() on those objects will always return the original parent, so we would need to come up with some solution for that too. What do you think about a change like this?

[edmorley]

Thank you for updating this, and good spot on the existing defaultRule bug!

Looking at GitHub code search, it seems that there are people relying on __ruleNames even though it's really an internal implementation details:
https://github.com/search?l=JavaScript&q=webpack-chain+__ruleNames&type=Code

As such this would unfortunately be a breaking change.

I know it wouldn't be as clean (and the toString() in src/Config.js would no longer be able to just use a simple .map() on __ruleNames), but what do what do you think about instead adding a separate types array to go alongside names and then a corresponding __ruleTypes? This would be similar to what is done in Plugin currently (aside from Plugin not needing arrays):

webpack-chain/src/Plugin.js

Lines 63 to 64 in df193ec

	__pluginName: { value: this.name },
	__pluginType: { value: this.type },

[opl-]

Good idea with the GitHub search. Keeping backwards compatibility seems to indeed be more important in this case than I expected. I'll change it to use __ruleTypes soon.

[opl-]

Fixed the issue mentioned in the review. Pushed twice because I realized that the name Rule.type instead of Rule.ruleType is just fine, since the ChainedMap stores its data in a store object instead of on the parent. Then, immediately after pushing I regretted not running the tests locally. Rule.type shadows the Rule.type() function, so I guess unless you can come up with a better name than "ruleTypes" it's staying like that.

I really need to get into the habit of running tests before I commit, not after.

[edmorley]

Thank you for the quick update! This new approach looks great.
Just a couple of small things I spotted and we should be good to merge :-)

[edmorley]

Published in v6.3.0 :-)