From bb5c7b0ed4005d523572e69c6bc924fdb5cf7306 Mon Sep 17 00:00:00 2001 From: Anshuman Verma Date: Tue, 21 Jul 2020 18:33:20 +0530 Subject: [PATCH] fix: warn about merge config resolution cases (#1674) * 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 --- .../webpack-cli/lib/groups/ConfigGroup.js | 29 +++++++------- packages/webpack-cli/lib/utils/GroupHelper.js | 3 +- .../lib/utils/errors/MergeError.js | 10 +++++ .../webpack.config.js => config-absent/1.js} | 0 .../config-absent/merge-config-absent.test.js | 22 +++++++++++ test/merge/config-absent/some_entry.js | 1 + test/merge/config/1.js | 5 ++- test/merge/config/2.js | 1 + test/merge/config/merge-config.test.js | 9 +++++ test/merge/defaults/1.js | 3 -- test/merge/defaults/merge-defaults.test.js | 39 ------------------- test/merge/defaults/some_entry.js | 1 - test/merge/defaults/some_other_entry.js | 1 - test/merge/defaults/webpack.base.js | 5 --- 14 files changed, 63 insertions(+), 66 deletions(-) create mode 100644 packages/webpack-cli/lib/utils/errors/MergeError.js rename test/merge/{defaults/webpack.config.js => config-absent/1.js} (100%) create mode 100644 test/merge/config-absent/merge-config-absent.test.js create mode 100644 test/merge/config-absent/some_entry.js delete mode 100644 test/merge/defaults/1.js delete mode 100644 test/merge/defaults/merge-defaults.test.js delete mode 100644 test/merge/defaults/some_entry.js delete mode 100644 test/merge/defaults/some_other_entry.js delete mode 100644 test/merge/defaults/webpack.base.js diff --git a/packages/webpack-cli/lib/groups/ConfigGroup.js b/packages/webpack-cli/lib/groups/ConfigGroup.js index 1af92129b82..21471bc6785 100644 --- a/packages/webpack-cli/lib/groups/ConfigGroup.js +++ b/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 @@ -84,6 +86,7 @@ class ConfigGroup extends GroupHelper { const newOptionsObject = { outputOptions: {}, options: {}, + processingMessageBuffer: [], }; if (!moduleObj) { @@ -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); } } diff --git a/packages/webpack-cli/lib/utils/GroupHelper.js b/packages/webpack-cli/lib/utils/GroupHelper.js index 5c34a988b1f..e683128e8b1 100644 --- a/packages/webpack-cli/lib/utils/GroupHelper.js +++ b/packages/webpack-cli/lib/utils/GroupHelper.js @@ -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); } diff --git a/packages/webpack-cli/lib/utils/errors/MergeError.js b/packages/webpack-cli/lib/utils/errors/MergeError.js new file mode 100644 index 00000000000..74c39efd6e6 --- /dev/null +++ b/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; diff --git a/test/merge/defaults/webpack.config.js b/test/merge/config-absent/1.js similarity index 100% rename from test/merge/defaults/webpack.config.js rename to test/merge/config-absent/1.js diff --git a/test/merge/config-absent/merge-config-absent.test.js b/test/merge/config-absent/merge-config-absent.test.js new file mode 100644 index 00000000000..2bf2c57a729 --- /dev/null +++ b/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(); + }); +}); diff --git a/test/merge/config-absent/some_entry.js b/test/merge/config-absent/some_entry.js new file mode 100644 index 00000000000..cae68b1307a --- /dev/null +++ b/test/merge/config-absent/some_entry.js @@ -0,0 +1 @@ +console.log('Oikawa'); diff --git a/test/merge/config/1.js b/test/merge/config/1.js index 133fc6944be..5051ea4b9d9 100644 --- a/test/merge/config/1.js +++ b/test/merge/config/1.js @@ -1,3 +1,6 @@ module.exports = { - entry: './some_entry.js', + entry: './old_entry.js', + output: { + filename: 'badfile.js', + }, }; diff --git a/test/merge/config/2.js b/test/merge/config/2.js index d359c59d18e..2c99fa4d4b7 100644 --- a/test/merge/config/2.js +++ b/test/merge/config/2.js @@ -1,4 +1,5 @@ module.exports = { + entry: './some_entry.js', output: { filename: 'merged.js', }, diff --git a/test/merge/config/merge-config.test.js b/test/merge/config/merge-config.test.js index 80d2976865c..0f673e2e51e 100644 --- a/test/merge/config/merge-config.test.js +++ b/test/merge/config/merge-config.test.js @@ -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(); + }); + }); }); diff --git a/test/merge/defaults/1.js b/test/merge/defaults/1.js deleted file mode 100644 index 1a75177f966..00000000000 --- a/test/merge/defaults/1.js +++ /dev/null @@ -1,3 +0,0 @@ -module.exports = { - entry: './some_other_entry.js', -}; diff --git a/test/merge/defaults/merge-defaults.test.js b/test/merge/defaults/merge-defaults.test.js deleted file mode 100644 index 2b7a762c50c..00000000000 --- a/test/merge/defaults/merge-defaults.test.js +++ /dev/null @@ -1,39 +0,0 @@ -'use strict'; - -const { stat, readFile } = require('fs'); -const { resolve } = require('path'); - -const { run } = require('../../utils/test-utils'); - -describe('merge flag defaults', () => { - it('merges a default webpack.base.config with default config lookup', (done) => { - const { stdout } = run(__dirname, ['-m', './'], false); - expect(stdout).not.toContain('option has not been set, webpack will fallback to'); - - stat(resolve(__dirname, './dist/default.js'), (err, stats) => { - expect(err).toBe(null); - expect(stats.isFile()).toBe(true); - done(); - }); - readFile(resolve(__dirname, './dist/default.js'), 'utf-8', (err, data) => { - expect(err).toBe(null); - expect(data).toContain('some_entry'); - done(); - }); - }); - it('merges a configuration file with default base config', (done) => { - const { stdout } = run(__dirname, ['-c', './1.js'], false); - - expect(stdout).not.toContain('option has not been set, webpack will fallback to'); - stat(resolve(__dirname, './dist/default.js'), (err, stats) => { - expect(err).toBe(null); - expect(stats.isFile()).toBe(true); - done(); - }); - readFile(resolve(__dirname, './dist/default.js'), 'utf-8', (err, data) => { - expect(err).toBe(null); - expect(data).toContain('some_entry'); - done(); - }); - }); -}); diff --git a/test/merge/defaults/some_entry.js b/test/merge/defaults/some_entry.js deleted file mode 100644 index 95556096294..00000000000 --- a/test/merge/defaults/some_entry.js +++ /dev/null @@ -1 +0,0 @@ -console.log('some_entry'); diff --git a/test/merge/defaults/some_other_entry.js b/test/merge/defaults/some_other_entry.js deleted file mode 100644 index 289750fec7e..00000000000 --- a/test/merge/defaults/some_other_entry.js +++ /dev/null @@ -1 +0,0 @@ -console.log('some_other_entry'); diff --git a/test/merge/defaults/webpack.base.js b/test/merge/defaults/webpack.base.js deleted file mode 100644 index 17e3211fb1a..00000000000 --- a/test/merge/defaults/webpack.base.js +++ /dev/null @@ -1,5 +0,0 @@ -module.exports = { - output: { - filename: 'default.js', - }, -};