From 242d2ec12dbc4f1d01a8c1ec19b5fbd1e2bdd4ed Mon Sep 17 00:00:00 2001 From: Anshuman Verma Date: Mon, 21 Sep 2020 19:49:29 +0530 Subject: [PATCH] refactor: config group to pure function and move away from group design (#1811) * chore: remove default value from config/entry flag * refactor: ConfigGroup remove from group design --- .../__snapshots__/parseArgs.test.ts.snap | 2 - .../__tests__/ConfigGroup/ConfigGroup.test.js | 15 +- .../webpack-cli/__tests__/GroupHelper.test.js | 9 - .../webpack-cli/lib/groups/ConfigGroup.js | 277 +++++++++--------- packages/webpack-cli/lib/utils/GroupHelper.js | 23 +- packages/webpack-cli/lib/utils/arg-utils.js | 23 ++ packages/webpack-cli/lib/utils/cli-flags.js | 2 - packages/webpack-cli/lib/webpack-cli.js | 35 ++- .../function-with-argv.test.js | 5 +- 9 files changed, 195 insertions(+), 196 deletions(-) delete mode 100644 packages/webpack-cli/__tests__/GroupHelper.test.js create mode 100644 packages/webpack-cli/lib/utils/arg-utils.js diff --git a/packages/serve/__tests__/__snapshots__/parseArgs.test.ts.snap b/packages/serve/__tests__/__snapshots__/parseArgs.test.ts.snap index e885c4c568c..4db7cd6b526 100644 --- a/packages/serve/__tests__/__snapshots__/parseArgs.test.ts.snap +++ b/packages/serve/__tests__/__snapshots__/parseArgs.test.ts.snap @@ -11,7 +11,6 @@ Object { }, "webpackArgs": Object { "color": true, - "config": Array [], "hot": true, "mode": "production", }, @@ -41,7 +40,6 @@ Object { }, "webpackArgs": Object { "color": true, - "config": Array [], "mode": "development", }, } diff --git a/packages/webpack-cli/__tests__/ConfigGroup/ConfigGroup.test.js b/packages/webpack-cli/__tests__/ConfigGroup/ConfigGroup.test.js index 5ce793d4295..8f2bc7318dc 100644 --- a/packages/webpack-cli/__tests__/ConfigGroup/ConfigGroup.test.js +++ b/packages/webpack-cli/__tests__/ConfigGroup/ConfigGroup.test.js @@ -7,7 +7,7 @@ const promiseConfig = require('./webpack.promise.config.cjs'); describe('ConfigGroup', function () { it('should handle merge properly', async () => { - const group = new ConfigGroup([ + const result = await ConfigGroup([ { merge: true, }, @@ -16,7 +16,6 @@ describe('ConfigGroup', function () { }, ]); - const result = await group.run(); const expectedOptions = { output: { filename: './dist-commonjs.js', libraryTarget: 'commonjs' }, entry: './a.js', @@ -30,35 +29,31 @@ describe('ConfigGroup', function () { }); it('should return array for multiple config', async () => { - const group = new ConfigGroup([ + const result = await ConfigGroup([ { config: [resolve(__dirname, './webpack.config1.cjs'), resolve(__dirname, './webpack.config2.cjs')] }, ]); - const result = await group.run(); const expectedOptions = [config1, config2]; expect(result.options).toEqual(expectedOptions); expect(result.outputOptions).toEqual({}); }); it('should return config object for single config', async () => { - const group = new ConfigGroup([{ config: [resolve(__dirname, './webpack.config1.cjs')] }]); - const result = await group.run(); + const result = await ConfigGroup([{ config: [resolve(__dirname, './webpack.config1.cjs')] }]); expect(result.options).toEqual(config1); expect(result.outputOptions).toEqual({}); }); it('should return resolved config object for promise config', async () => { - const group = new ConfigGroup([{ config: [resolve(__dirname, './webpack.promise.config.cjs')] }]); - const result = await group.run(); + const result = await ConfigGroup([{ config: [resolve(__dirname, './webpack.promise.config.cjs')] }]); const expectedOptions = await promiseConfig(); expect(result.options).toEqual(expectedOptions); expect(result.outputOptions).toEqual({}); }); it('should handle configs returning different types', async () => { - const group = new ConfigGroup([ + const result = await ConfigGroup([ { config: [resolve(__dirname, './webpack.promise.config.cjs'), resolve(__dirname, './webpack.config.cjs')] }, ]); - const result = await group.run(); const resolvedPromiseConfig = await promiseConfig(); const expectedOptions = [resolvedPromiseConfig, ...arrayConfig]; expect(result.options).toEqual(expectedOptions); diff --git a/packages/webpack-cli/__tests__/GroupHelper.test.js b/packages/webpack-cli/__tests__/GroupHelper.test.js deleted file mode 100644 index 151e9d6f0c8..00000000000 --- a/packages/webpack-cli/__tests__/GroupHelper.test.js +++ /dev/null @@ -1,9 +0,0 @@ -const GroupHelper = require('../lib/utils/GroupHelper'); - -describe('GroupHelper', function () { - it('should return undefined', () => { - const helper = new GroupHelper(); - - expect(helper.arrayToObject()).toBeUndefined(); - }); -}); diff --git a/packages/webpack-cli/lib/groups/ConfigGroup.js b/packages/webpack-cli/lib/groups/ConfigGroup.js index ed22885fce9..d88d7bc3c9c 100644 --- a/packages/webpack-cli/lib/groups/ConfigGroup.js +++ b/packages/webpack-cli/lib/groups/ConfigGroup.js @@ -2,8 +2,8 @@ const { existsSync } = require('fs'); const { resolve, sep, dirname, extname } = require('path'); const webpackMerge = require('webpack-merge'); const { extensions, jsVariants } = require('interpret'); -const GroupHelper = require('../utils/GroupHelper'); const rechoir = require('rechoir'); +const { arrayToObject } = require('../utils/arg-utils'); const ConfigError = require('../utils/errors/ConfigError'); const logger = require('../utils/logger'); @@ -29,6 +29,12 @@ const modeAlias = { development: 'dev', }; +let opts = { + outputOptions: {}, + options: {}, +}; + +// Return a list of default configs in various formats const getDefaultConfigFiles = () => { return DEFAULT_CONFIG_LOC.map((filename) => { // Since .cjs is not available on interpret side add it manually to default config extension list @@ -58,162 +64,163 @@ const getConfigInfoFromFileName = (filename) => { .filter((e) => existsSync(e.path)); }; -class ConfigGroup extends GroupHelper { - constructor(options) { - super(options); - } - - requireLoader(extension, path) { - rechoir.prepare(extensions, path, process.cwd()); - } - - requireConfig(configModule) { - const extension = Object.keys(jsVariants).find((t) => configModule.ext.endsWith(t)); +// Prepare rechoir environment to load multiple file formats +const requireLoader = (extension, path) => { + rechoir.prepare(extensions, path, process.cwd()); +}; - if (extension) { - this.requireLoader(extension, configModule.path); - } +// Reads a config file given the config metadata +const requireConfig = (configModule) => { + const extension = Object.keys(jsVariants).find((t) => configModule.ext.endsWith(t)); - let config = require(configModule.path); - if (config.default) { - config = config.default; - } + if (extension) { + requireLoader(extension, configModule.path); + } - return { - content: config, - path: configModule.path, - }; + let config = require(configModule.path); + if (config.default) { + config = config.default; } - async finalize(moduleObj) { - const { argv } = this.args; - const newOptionsObject = { - outputOptions: {}, - options: {}, - }; + return { + content: config, + path: configModule.path, + }; +}; - if (!moduleObj) { - return newOptionsObject; - } - const configPath = moduleObj.path; - const configOptions = moduleObj.content; - if (typeof configOptions === 'function') { - // when config is a function, pass the env from args to the config function - let formattedEnv; - if (Array.isArray(this.args.env)) { - formattedEnv = this.args.env.reduce((envObject, envOption) => { - envObject[envOption] = true; - return envObject; - }, {}); +// Responsible for reading user configuration files +// else does a default config lookup and resolves it. +const resolveConfigFiles = async (args) => { + const { config, mode } = args; + if (config && config.length > 0) { + const resolvedOptions = []; + const finalizedConfigs = config.map(async (webpackConfig) => { + const configPath = resolve(webpackConfig); + const configFiles = getConfigInfoFromFileName(configPath); + if (!configFiles.length) { + throw new ConfigError(`The specified config file doesn't exist in ${configPath}`); } - const newOptions = configOptions(formattedEnv, argv); - // When config function returns a promise, resolve it, if not it's resolved by default - newOptionsObject['options'] = await Promise.resolve(newOptions); - } else if (Array.isArray(configOptions) && this.args.configName) { - // In case of exporting multiple configurations, If you pass a name to --config-name flag, - // webpack will only build that specific configuration. - const namedOptions = configOptions.filter((opt) => this.args.configName.includes(opt.name)); - if (namedOptions.length === 0) { - logger.error(`Configuration with name "${this.args.configName}" was not found.`); - process.exit(2); + const foundConfig = configFiles[0]; + const resolvedConfig = requireConfig(foundConfig); + return finalize(resolvedConfig, args); + }); + // resolve all the configs + for await (const resolvedOption of finalizedConfigs) { + if (Array.isArray(resolvedOption.options)) { + resolvedOptions.push(...resolvedOption.options); } else { - newOptionsObject['options'] = namedOptions; + resolvedOptions.push(resolvedOption.options); } - } else { - if (Array.isArray(configOptions) && !configOptions.length) { - newOptionsObject['options'] = {}; - return newOptionsObject; - } - newOptionsObject['options'] = configOptions; } + // When the resolved configs are more than 1, then pass them as Array [{...}, {...}] else pass the first config object {...} + const finalOptions = resolvedOptions.length > 1 ? resolvedOptions : resolvedOptions[0] || {}; - if (configOptions && configPath.includes('.webpack')) { - const currentPath = configPath; - const parentContext = dirname(currentPath).split(sep).slice(0, -1).join(sep); - if (Array.isArray(configOptions)) { - configOptions.forEach((config) => { - config.context = config.context || parentContext; - }); - } else { - configOptions.context = configOptions.context || parentContext; - } - newOptionsObject['options'] = configOptions; - } - return newOptionsObject; + opts['options'] = finalOptions; + return; } - async resolveConfigFiles() { - const { config, mode } = this.args; - if (config.length > 0) { - const resolvedOptions = []; - const finalizedConfigs = config.map(async (webpackConfig) => { - const configPath = resolve(webpackConfig); - const configFiles = getConfigInfoFromFileName(configPath); - if (!configFiles.length) { - throw new ConfigError(`The specified config file doesn't exist in ${configPath}`); - } - const foundConfig = configFiles[0]; - const resolvedConfig = this.requireConfig(foundConfig); - return this.finalize(resolvedConfig); - }); - // resolve all the configs - for await (const resolvedOption of finalizedConfigs) { - if (Array.isArray(resolvedOption.options)) { - resolvedOptions.push(...resolvedOption.options); - } else { - resolvedOptions.push(resolvedOption.options); - } - } - // When the resolved configs are more than 1, then pass them as Array [{...}, {...}] else pass the first config object {...} - const finalOptions = resolvedOptions.length > 1 ? resolvedOptions : resolvedOptions[0] || {}; - - this.opts['options'] = finalOptions; + // When no config is supplied, lookup for default configs + const defaultConfigFiles = getDefaultConfigFiles(); + const tmpConfigFiles = defaultConfigFiles.filter((file) => { + return existsSync(file.path); + }); + + const configFiles = tmpConfigFiles.map(requireConfig); + if (configFiles.length) { + const defaultConfig = configFiles.find((p) => p.path.includes(mode) || p.path.includes(modeAlias[mode])); + if (defaultConfig) { + opts = await finalize(defaultConfig, args); return; } + const foundConfig = configFiles.pop(); + opts = await finalize(foundConfig, args); + return; + } +}; - // When no config is supplied, lookup for default configs - const defaultConfigFiles = getDefaultConfigFiles(); - const tmpConfigFiles = defaultConfigFiles.filter((file) => { - return existsSync(file.path); - }); +// Given config data, determines the type of config and +// returns final config +const finalize = async (moduleObj, args) => { + const { argv, env, configName } = args; + const newOptionsObject = { + outputOptions: {}, + options: {}, + }; - const configFiles = tmpConfigFiles.map(this.requireConfig.bind(this)); - if (configFiles.length) { - const defaultConfig = configFiles.find((p) => p.path.includes(mode) || p.path.includes(modeAlias[mode])); - if (defaultConfig) { - this.opts = await this.finalize(defaultConfig); - return; - } - const foundConfig = configFiles.pop(); - this.opts = await this.finalize(foundConfig); - return; + if (!moduleObj) { + return newOptionsObject; + } + const configPath = moduleObj.path; + const configOptions = moduleObj.content; + if (typeof configOptions === 'function') { + // when config is a function, pass the env from args to the config function + let formattedEnv; + if (Array.isArray(env)) { + formattedEnv = env.reduce((envObject, envOption) => { + envObject[envOption] = true; + return envObject; + }, {}); + } + const newOptions = configOptions(formattedEnv, argv); + // When config function returns a promise, resolve it, if not it's resolved by default + newOptionsObject['options'] = await Promise.resolve(newOptions); + } else if (Array.isArray(configOptions) && configName) { + // In case of exporting multiple configurations, If you pass a name to --config-name flag, + // webpack will only build that specific configuration. + const namedOptions = configOptions.filter((opt) => configName.includes(opt.name)); + if (namedOptions.length === 0) { + logger.error(`Configuration with name "${configName}" was not found.`); + process.exit(2); + } else { + newOptionsObject['options'] = namedOptions; + } + } else { + if (Array.isArray(configOptions) && !configOptions.length) { + newOptionsObject['options'] = {}; + return newOptionsObject; } + newOptionsObject['options'] = configOptions; } - async resolveConfigMerging() { - const { merge } = this.args; - if (merge) { - // Get the current configuration options - const { options: configOptions } = this.opts; - - // we can only merge when there are multiple configurations - // either by passing multiple configs by flags or passing a - // single config exporting an array - if (!Array.isArray(configOptions)) { - throw new ConfigError('Atleast two configurations are required for merge.', 'MergeError'); - } - - // We return a single config object which is passed to the compiler - const mergedOptions = configOptions.reduce((currentConfig, mergedConfig) => webpackMerge(currentConfig, mergedConfig), {}); - this.opts['options'] = mergedOptions; + if (configOptions && configPath.includes('.webpack')) { + const currentPath = configPath; + const parentContext = dirname(currentPath).split(sep).slice(0, -1).join(sep); + if (Array.isArray(configOptions)) { + configOptions.forEach((config) => { + config.context = config.context || parentContext; + }); + } else { + configOptions.context = configOptions.context || parentContext; } + newOptionsObject['options'] = configOptions; } + return newOptionsObject; +}; - async run() { - await this.resolveConfigFiles(); - await this.resolveConfigMerging(); - return this.opts; +const resolveConfigMerging = async (args) => { + const { merge } = args; + if (merge) { + // Get the current configuration options + const { options: configOptions } = opts; + + // we can only merge when there are multiple configurations + // either by passing multiple configs by flags or passing a + // single config exporting an array + if (!Array.isArray(configOptions)) { + throw new ConfigError('Atleast two configurations are required for merge.', 'MergeError'); + } + + // We return a single config object which is passed to the compiler + const mergedOptions = configOptions.reduce((currentConfig, mergedConfig) => webpackMerge(currentConfig, mergedConfig), {}); + opts['options'] = mergedOptions; } -} +}; + +const handleConfigResolution = async (args) => { + const parsedArgs = arrayToObject(args); + await resolveConfigFiles(parsedArgs); + await resolveConfigMerging(parsedArgs); + return opts; +}; -module.exports = ConfigGroup; +module.exports = handleConfigResolution; diff --git a/packages/webpack-cli/lib/utils/GroupHelper.js b/packages/webpack-cli/lib/utils/GroupHelper.js index f7e539533eb..18d1acb533d 100644 --- a/packages/webpack-cli/lib/utils/GroupHelper.js +++ b/packages/webpack-cli/lib/utils/GroupHelper.js @@ -1,9 +1,10 @@ const { resolve, join } = require('path'); const { existsSync } = require('fs'); +const { arrayToObject } = require('../utils/arg-utils'); class GroupHelper { constructor(options) { - this.args = this.arrayToObject(options); + this.args = arrayToObject(options); this.opts = { outputOptions: {}, options: {}, @@ -11,26 +12,6 @@ class GroupHelper { this.strategy = undefined; } - hyphenToUpperCase(name) { - if (!name) { - return name; - } - return name.replace(/-([a-z])/g, function (g) { - return g[1].toUpperCase(); - }); - } - - arrayToObject(arr) { - if (!arr) { - return; - } - return arr.reduce((result, currentItem) => { - const key = Object.keys(currentItem)[0]; - result[this.hyphenToUpperCase(key)] = currentItem[key]; - return result; - }, {}); - } - resolveFilePath(filename = '', defaultValue) { if (filename && Array.isArray(filename)) { return filename.map((fp) => this.resolveFilePath(fp, defaultValue)).filter((e) => e); diff --git a/packages/webpack-cli/lib/utils/arg-utils.js b/packages/webpack-cli/lib/utils/arg-utils.js new file mode 100644 index 00000000000..d403b9087cd --- /dev/null +++ b/packages/webpack-cli/lib/utils/arg-utils.js @@ -0,0 +1,23 @@ +function hyphenToUpperCase(name) { + if (!name) { + return name; + } + return name.replace(/-([a-z])/g, function (g) { + return g[1].toUpperCase(); + }); +} + +function arrayToObject(arr) { + if (!arr) { + return; + } + return arr.reduce((result, currentItem) => { + const key = Object.keys(currentItem)[0]; + result[hyphenToUpperCase(key)] = currentItem[key]; + return result; + }, {}); +} + +module.exports = { + arrayToObject, +}; diff --git a/packages/webpack-cli/lib/utils/cli-flags.js b/packages/webpack-cli/lib/utils/cli-flags.js index b82eca34b01..11652e08519 100644 --- a/packages/webpack-cli/lib/utils/cli-flags.js +++ b/packages/webpack-cli/lib/utils/cli-flags.js @@ -81,7 +81,6 @@ const core = [ usage: '--entry | --entry --entry ', type: String, multiple: true, - defaultOption: true, group: BASIC_GROUP, description: 'The entry point(s) of your application e.g. ./src/main.js', link: 'https://webpack.js.org/concepts/#entry', @@ -91,7 +90,6 @@ const core = [ usage: '--config ', alias: 'c', type: String, - defaultValue: [], group: CONFIG_GROUP, multiple: true, description: 'Provide path to a webpack configuration file e.g. ./webpack.config.js', diff --git a/packages/webpack-cli/lib/webpack-cli.js b/packages/webpack-cli/lib/webpack-cli.js index 26e5cb5929d..a8fc28a92c6 100644 --- a/packages/webpack-cli/lib/webpack-cli.js +++ b/packages/webpack-cli/lib/webpack-cli.js @@ -1,5 +1,6 @@ const { options } = require('colorette'); const GroupHelper = require('./utils/GroupHelper'); +const handleConfigResolution = require('./groups/ConfigGroup'); const { Compiler } = require('./utils/Compiler'); const { groups, core } = require('./utils/cli-flags'); const webpackMerge = require('webpack-merge'); @@ -63,6 +64,21 @@ class WebpackCLI extends GroupHelper { coreCliHelper.processArguments(coreCliArgs, this.compilerConfiguration, coreConfig); } + /** + * Responsible for resolving config + * @private\ + * @param {Object} processed args + * @returns {void} + */ + async _handleConfig(parsedArgs) { + const { mode } = parsedArgs; + // get config group values from the map + const value = this.groupMap.get(groups.CONFIG_GROUP) || []; + const resolvedConfig = await handleConfigResolution([...value, { mode }, { argv: parsedArgs }]); + this._mergeOptionsToConfiguration(resolvedConfig.options); + this._mergeOptionsToOutputConfiguration(resolvedConfig.outputOptions); + } + /** * Expose commander argParser * @param {...any} args args for argParser @@ -81,13 +97,7 @@ class WebpackCLI extends GroupHelper { * * @returns {void} */ - resolveGroups(parsedArgs) { - let mode; - // determine the passed mode for ConfigGroup - if (this.groupMap.has(groups.ZERO_CONFIG_GROUP)) { - const modePresent = this.groupMap.get(groups.ZERO_CONFIG_GROUP).find((t) => !!t.mode); - if (modePresent) mode = modePresent.mode; - } + resolveGroups() { for (const [key, value] of this.groupMap.entries()) { switch (key) { case groups.ZERO_CONFIG_GROUP: { @@ -105,11 +115,6 @@ class WebpackCLI extends GroupHelper { this.advancedGroup = new AdvancedGroup(value); break; } - case groups.CONFIG_GROUP: { - const ConfigGroup = require('./groups/ConfigGroup'); - this.configGroup = new ConfigGroup([...value, { mode }, { argv: parsedArgs }]); - break; - } case groups.DISPLAY_GROUP: { const StatsGroup = require('./groups/StatsGroup'); this.statsGroup = new StatsGroup(value); @@ -223,11 +228,11 @@ class WebpackCLI extends GroupHelper { * The next groups will override existing parameters * @returns {Promise} A Promise */ - async runOptionGroups() { + async runOptionGroups(parsedArgs) { await Promise.resolve() .then(() => this._handleGroupHelper(this.zeroConfigGroup)) .then(() => this._handleDefaultEntry()) - .then(() => this._handleGroupHelper(this.configGroup)) + .then(() => this._handleConfig(parsedArgs)) .then(() => this._handleGroupHelper(this.outputGroup)) .then(() => this._handleCoreFlags()) .then(() => this._handleGroupHelper(this.basicGroup)) @@ -239,7 +244,7 @@ class WebpackCLI extends GroupHelper { async processArgs(args, cliOptions) { this.setMappedGroups(args, cliOptions); this.resolveGroups(args); - const groupResult = await this.runOptionGroups(); + const groupResult = await this.runOptionGroups(args); return groupResult; } diff --git a/test/config/type/function-with-argv/function-with-argv.test.js b/test/config/type/function-with-argv/function-with-argv.test.js index 06498c156f5..26d7cb94240 100644 --- a/test/config/type/function-with-argv/function-with-argv.test.js +++ b/test/config/type/function-with-argv/function-with-argv.test.js @@ -5,10 +5,11 @@ const { run } = require('../../../utils/test-utils'); describe('function configuration', () => { it('is able to understand a configuration file as a function', () => { - const { stderr, stdout } = run(__dirname, ['--mode', 'development'], false); + const { stderr, stdout, exitCode } = run(__dirname, ['--mode', 'development'], false); expect(stderr).toBeFalsy(); expect(stdout).toBeTruthy(); - expect(stdout).toContain("argv: { config: [], color: true, mode: 'development' }"); + expect(exitCode).toBe(0); + expect(stdout).toContain("argv: { color: true, mode: 'development' }"); // Should generate the appropriate files expect(existsSync(resolve(__dirname, './dist/dev.js'))).toBeTruthy(); });