Skip to content

Commit

Permalink
fix: warn about merge config resolution cases (#1674)
Browse files Browse the repository at this point in the history
* fix: show warning about merge config resolution cases

* fix: do not fallback to base config in merge

* fix: do not fallback to base config in merge

* fix: remove fallback to base config

* fix: do not fallback to base config

* feat: add MergeError calss

* feat: add MergeError class

* tests: add tests for merge

* tests: add more merge tests
  • Loading branch information
anshumanv committed Jul 21, 2020
1 parent cc49df5 commit bb5c7b0
Show file tree
Hide file tree
Showing 14 changed files with 63 additions and 66 deletions.
29 changes: 14 additions & 15 deletions packages/webpack-cli/lib/groups/ConfigGroup.js
@@ -1,8 +1,10 @@
const { existsSync } = require('fs');
const { resolve, sep, dirname, parse } = require('path');
const webpackMerge = require('webpack-merge');
const { extensions, jsVariants } = require('interpret');
const GroupHelper = require('../utils/GroupHelper');
const rechoir = require('rechoir');
const MergeError = require('../utils/errors/MergeError');

// Order defines the priority, in increasing order
// example - config file lookup will be in order of .webpack/webpack.config.development.js -> webpack.config.development.js -> webpack.config.js
Expand Down Expand Up @@ -84,6 +86,7 @@ class ConfigGroup extends GroupHelper {
const newOptionsObject = {
outputOptions: {},
options: {},
processingMessageBuffer: [],
};

if (!moduleObj) {
Expand Down Expand Up @@ -161,22 +164,18 @@ class ConfigGroup extends GroupHelper {
if (Object.keys(this.args).some((arg) => arg === 'merge')) {
const { merge } = this.args;

const newConfigPath = this.resolveFilePath(merge, 'webpack.base.js');
if (newConfigPath) {
const configFiles = getConfigInfoFromFileName(newConfigPath);
if (!configFiles.length) {
this.opts.processingMessageBuffer.push({
lvl: 'warn',
msg: 'Could not find file to merge configuration with...',
});
return;
}
const foundConfig = configFiles[0];
const resolvedConfig = this.requireConfig(foundConfig);
const newConfigurationsObject = await this.finalize(resolvedConfig);
const webpackMerge = require('webpack-merge');
this.opts['options'] = webpackMerge(this.opts['options'], newConfigurationsObject.options);
// try resolving merge config
const newConfigPath = this.resolveFilePath(merge);

if (!newConfigPath) {
throw new MergeError("The supplied merge config doesn't exist.");
}

const configFiles = getConfigInfoFromFileName(newConfigPath);
const foundConfig = configFiles[0];
const resolvedConfig = this.requireConfig(foundConfig);
const newConfigurationsObject = await this.finalize(resolvedConfig);
this.opts['options'] = webpackMerge(this.opts['options'], newConfigurationsObject.options);
}
}

Expand Down
3 changes: 2 additions & 1 deletion packages/webpack-cli/lib/utils/GroupHelper.js
Expand Up @@ -47,7 +47,8 @@ class GroupHelper {
const configPathExists = predefinedConfigPath ? existsSync(predefinedConfigPath) : undefined;

if (!configPathExists) {
const LOOKUP_PATHS = [`${filename}`, `src/${filename}`, defaultValue, `src/${defaultValue}`];
const LOOKUP_PATHS = [`${filename}`, `src/${filename}`, ...(defaultValue ? [defaultValue, `src/${defaultValue}`] : [])];

if (filename) {
LOOKUP_PATHS.unshift(filename);
}
Expand Down
10 changes: 10 additions & 0 deletions packages/webpack-cli/lib/utils/errors/MergeError.js
@@ -0,0 +1,10 @@
class MergeError extends Error {
constructor(message) {
super(message);
this.name = 'MergeError';
// No need to show stack trace for known errors
this.stack = '';
}
}

module.exports = MergeError;
File renamed without changes.
22 changes: 22 additions & 0 deletions test/merge/config-absent/merge-config-absent.test.js
@@ -0,0 +1,22 @@
'use strict';

const fs = require('fs');
const { join } = require('path');

const { run } = require('../../utils/test-utils');

describe('merge flag configuration', () => {
it('Show warning message when the merge config is absent', () => {
// 2.js doesn't exist, let's try merging with it
const { stdout, stderr } = run(__dirname, ['--config', './1.js', '--merge', './2.js'], false);

// Since the process will exit, nothing on stdout
expect(stdout).toBeFalsy();
// Confirm that the user is notified
expect(stderr).toContain(`MergeError: The supplied merge config doesn't exist.`);
// Default config would be used
expect(fs.existsSync(join(__dirname, './dist/merged.js'))).toBeFalsy();
// Since the process will exit so no compilation will be done
expect(fs.existsSync(join(__dirname, './dist/main.js'))).toBeFalsy();
});
});
1 change: 1 addition & 0 deletions test/merge/config-absent/some_entry.js
@@ -0,0 +1 @@
console.log('Oikawa');
5 changes: 4 additions & 1 deletion test/merge/config/1.js
@@ -1,3 +1,6 @@
module.exports = {
entry: './some_entry.js',
entry: './old_entry.js',
output: {
filename: 'badfile.js',
},
};
1 change: 1 addition & 0 deletions test/merge/config/2.js
@@ -1,4 +1,5 @@
module.exports = {
entry: './some_entry.js',
output: {
filename: 'merged.js',
},
Expand Down
9 changes: 9 additions & 0 deletions test/merge/config/merge-config.test.js
Expand Up @@ -15,4 +15,13 @@ describe('merge flag configuration', () => {
done();
});
});
it('merges two configurations together with flag alias', (done) => {
const { stdout } = run(__dirname, ['--config', './1.js', '-m', './2.js'], false);
expect(stdout).toContain('merged.js');
stat(resolve(__dirname, './dist/merged.js'), (err, stats) => {
expect(err).toBe(null);
expect(stats.isFile()).toBe(true);
done();
});
});
});
3 changes: 0 additions & 3 deletions test/merge/defaults/1.js

This file was deleted.

39 changes: 0 additions & 39 deletions test/merge/defaults/merge-defaults.test.js

This file was deleted.

1 change: 0 additions & 1 deletion test/merge/defaults/some_entry.js

This file was deleted.

1 change: 0 additions & 1 deletion test/merge/defaults/some_other_entry.js

This file was deleted.

5 changes: 0 additions & 5 deletions test/merge/defaults/webpack.base.js

This file was deleted.

0 comments on commit bb5c7b0

Please sign in to comment.