Skip to content

Commit

Permalink
feature #509 feat: Add method to configure loaders rules (Kocal, weav…
Browse files Browse the repository at this point in the history
…erryan)

This PR was merged into the master branch.

Discussion
----------

feat: Add method to configure loaders rules

This PR is a proposal to resolve #473 and #504 in a clean way.

`Encore.configureLoaderRule()` is a low-level function, and has for goal to let the user having full access to Webpack loaders rules (what we push into `module.rules`) and configure them.

This is the implementation of the idea I had in #504 (comment).

For resolving Vue files linting issue, the next example would be the ideal solution  I think:
```js
Encore
  .configureLoaderRule('eslint', loader => {
    loader.test = /\.(jsx?|vue)$/
  });

// actually, the equivalent code is something like this:
const webpackConfig = Encore.getWebpackConfig();
const eslintLoader = webpackConfig.module.rules.find(rule => rule.loader === 'eslint-loader');

eslintLoader.test = /\.(jsx?|vue)$/;

return webpackConfig;
```

For now, only ESLint loader is supported, but we can easily add other loaders.

Let me know what you think of this solution, thanks!

Commits
-------

94da0c2 language tweak
9dd6ca4 chore(tests): correct some tests due to last feature for CSSModules
4dbe443 chore(cr): add real aliases support
729a696 feat: add warning
5cf3d02 feat: add missing loaders, add more tests
86dc788 refactor(test): `configureLoaderRule()` will be easier to test
39cb9bd chore(cr): move tests into
bc0e9bf chore(cr): add shortcut function applyRuleConfigurationCallback
bc1d025 chore(cr): more user-friendly error for valid configurable loaders
01880cf Add method on "Encore"
49a4258 add tests
9892516 feat: implement "configureLoaderRule" method
  • Loading branch information
weaverryan committed Mar 25, 2019
2 parents 356e6e6 + 94da0c2 commit 112ec4b
Show file tree
Hide file tree
Showing 5 changed files with 325 additions and 22 deletions.
26 changes: 26 additions & 0 deletions index.js
Expand Up @@ -1100,6 +1100,32 @@ class Encore {
return this;
}

/**
* Configure Webpack loaders rules (`module.rules`).
* This is a low-level function, be careful when using it.
*
* https://webpack.js.org/concepts/loaders/#configuration
*
* For example, if you are using Vue and ESLint loader,
* this is how you can configure ESLint to lint Vue files:
*
* Encore
* .enableEslintLoader()
* .enableVueLoader()
* .configureLoaderRule('eslint', (loaderRule) => {
* loaderRule.test = /\.(jsx?|vue)/;
* });
*
* @param {string} name
* @param {function} callback
* @return {Encore}
*/
configureLoaderRule(name, callback) {
webpackConfig.configureLoaderRule(name, callback);

return this;
}

/**
* If enabled, the output directory is emptied between each build (to remove old files).
*
Expand Down
38 changes: 38 additions & 0 deletions lib/WebpackConfig.js
Expand Up @@ -100,6 +100,19 @@ class WebpackConfig {
this.eslintLoaderOptionsCallback = () => {};
this.tsConfigurationCallback = () => {};
this.handlebarsConfigurationCallback = () => {};
this.loaderConfigurationCallbacks = {
javascript: () => {},
css: () => {},
images: () => {},
fonts: () => {},
sass: () => {},
less: () => {},
stylus: () => {},
vue: () => {},
eslint: () => {},
typescript: () => {},
handlebars: () => {},
};

// Plugins options
this.cleanWebpackPluginPaths = ['**/*'];
Expand Down Expand Up @@ -716,6 +729,31 @@ class WebpackConfig {
});
}

configureLoaderRule(name, callback) {
logger.warning('Be careful when using Encore.configureLoaderRule(), this is a low-level method that can potentially break Encore and Webpack when not used carefully.');

// Key: alias, Value: existing loader in `this.loaderConfigurationCallbacks`
const aliases = {
js: 'javascript',
ts: 'typescript',
scss: 'sass',
};

if (name in aliases) {
name = aliases[name];
}

if (!(name in this.loaderConfigurationCallbacks)) {
throw new Error(`Loader "${name}" is not configurable. Valid loaders are "${Object.keys(this.loaderConfigurationCallbacks).join('", "')}" and the aliases "${Object.keys(aliases).join('", "')}".`);
}

if (typeof callback !== 'function') {
throw new Error('Argument 2 to configureLoaderRule() must be a callback function.');
}

this.loaderConfigurationCallbacks[name] = callback;
}

useDevServer() {
return this.runtimeConfig.useDevServer;
}
Expand Down
48 changes: 26 additions & 22 deletions lib/config-generator.js
Expand Up @@ -221,14 +221,18 @@ class ConfigGenerator {
}

buildRulesConfig() {
const applyRuleConfigurationCallback = (name, defaultRules) => {
return applyOptionsCallback(this.webpackConfig.loaderConfigurationCallbacks[name], defaultRules);
};

let rules = [
{
applyRuleConfigurationCallback('javascript', {
// match .js and .jsx
test: /\.jsx?$/,
exclude: this.webpackConfig.babelOptions.exclude,
use: babelLoaderUtil.getLoaders(this.webpackConfig)
},
{
}),
applyRuleConfigurationCallback('css', {
test: /\.css$/,
oneOf: [
{
Expand All @@ -245,7 +249,7 @@ class ConfigGenerator {
)
}
]
}
})
];

if (this.webpackConfig.useImagesLoader) {
Expand All @@ -269,11 +273,11 @@ class ConfigGenerator {
Object.assign(loaderOptions, this.webpackConfig.urlLoaderOptions.images);
}

rules.push({
rules.push(applyRuleConfigurationCallback('images', {
test: /\.(png|jpg|jpeg|gif|ico|svg|webp)$/,
loader: loaderName,
options: loaderOptions
});
}));
}

if (this.webpackConfig.useFontsLoader) {
Expand All @@ -297,15 +301,15 @@ class ConfigGenerator {
Object.assign(loaderOptions, this.webpackConfig.urlLoaderOptions.fonts);
}

rules.push({
rules.push(applyRuleConfigurationCallback('fonts', {
test: /\.(woff|woff2|ttf|eot|otf)$/,
loader: loaderName,
options: loaderOptions
});
}));
}

if (this.webpackConfig.useSassLoader) {
rules.push({
rules.push(applyRuleConfigurationCallback('sass', {
test: /\.s[ac]ss$/,
oneOf: [
{
Expand All @@ -316,11 +320,11 @@ class ConfigGenerator {
use: cssExtractLoaderUtil.prependLoaders(this.webpackConfig, sassLoaderUtil.getLoaders(this.webpackConfig))
}
]
});
}));
}

if (this.webpackConfig.useLessLoader) {
rules.push({
rules.push(applyRuleConfigurationCallback('less', {
test: /\.less/,
oneOf: [
{
Expand All @@ -331,11 +335,11 @@ class ConfigGenerator {
use: cssExtractLoaderUtil.prependLoaders(this.webpackConfig, lessLoaderUtil.getLoaders(this.webpackConfig))
}
]
});
}));
}

if (this.webpackConfig.useStylusLoader) {
rules.push({
rules.push(applyRuleConfigurationCallback('stylus', {
test: /\.styl/,
oneOf: [
{
Expand All @@ -346,39 +350,39 @@ class ConfigGenerator {
use: cssExtractLoaderUtil.prependLoaders(this.webpackConfig, stylusLoaderUtil.getLoaders(this.webpackConfig))
}
]
});
}));
}

if (this.webpackConfig.useVueLoader) {
rules.push({
rules.push(applyRuleConfigurationCallback('vue', {
test: /\.vue$/,
use: vueLoaderUtil.getLoaders(this.webpackConfig)
});
}));
}

if (this.webpackConfig.useEslintLoader) {
rules.push({
rules.push(applyRuleConfigurationCallback('eslint', {
test: /\.jsx?$/,
loader: 'eslint-loader',
exclude: /node_modules/,
enforce: 'pre',
options: eslintLoaderUtil.getOptions(this.webpackConfig)
});
}));
}

if (this.webpackConfig.useTypeScriptLoader) {
rules.push({
rules.push(applyRuleConfigurationCallback('typescript', {
test: /\.tsx?$/,
exclude: /node_modules/,
use: tsLoaderUtil.getLoaders(this.webpackConfig)
});
}));
}

if (this.webpackConfig.useHandlebarsLoader) {
rules.push({
rules.push(applyRuleConfigurationCallback('handlebars', {
test: /\.(handlebars|hbs)$/,
use: handlebarsLoaderUtil.getLoaders(this.webpackConfig)
});
}));
}

this.webpackConfig.loaders.forEach((loader) => {
Expand Down
32 changes: 32 additions & 0 deletions test/WebpackConfig.js
Expand Up @@ -1144,4 +1144,36 @@ describe('WebpackConfig object', () => {
}).to.throw('Argument 1 to configureWatchOptions() must be a callback function.');
});
});

describe('configureLoaderRule()', () => {
it('works properly', () => {
const config = createConfig();
const callback = (loader) => {};

expect(config.loaderConfigurationCallbacks['eslint']).to.not.equal(callback);

config.configureLoaderRule('eslint', callback);
expect(config.loaderConfigurationCallbacks['eslint']).to.equal(callback);
});

it('Call method with a not supported loader', () => {
const config = createConfig();

expect(() => {
config.configureLoaderRule('reason');
}).to.throw('Loader "reason" is not configurable. Valid loaders are "javascript", "css", "images", "fonts", "sass", "less", "stylus", "vue", "eslint", "typescript", "handlebars" and the aliases "js", "ts", "scss".');
});

it('Call method with not a valid callback', () => {
const config = createConfig();

expect(() => {
config.configureLoaderRule('eslint');
}).to.throw('Argument 2 to configureLoaderRule() must be a callback function.');

expect(() => {
config.configureLoaderRule('eslint', {});
}).to.throw('Argument 2 to configureLoaderRule() must be a callback function.');
});
});
});

0 comments on commit 112ec4b

Please sign in to comment.