From 9e9c70bc1f30d90cebd91341e865abb46f9c269e Mon Sep 17 00:00:00 2001 From: Anshuman Verma Date: Thu, 24 Sep 2020 17:31:33 +0530 Subject: [PATCH] fix: mode behaviour (#1824) * fix: mode behaviour * fix: mode behaviour --- .../webpack-cli/__tests__/resolveMode.test.js | 66 +++++++++++++++---- .../webpack-cli/lib/groups/resolveMode.js | 47 +++++++++---- packages/webpack-cli/lib/webpack-cli.js | 21 +++++- .../mode-single-arg/mode-single-arg.test.js | 6 ++ .../mode-with-config/mode-with-config.test.js | 45 ++++++++++++- test/mode/mode-with-config/webpack.config3.js | 6 ++ test/utils/test-utils.js | 3 +- 7 files changed, 164 insertions(+), 30 deletions(-) create mode 100644 test/mode/mode-with-config/webpack.config3.js diff --git a/packages/webpack-cli/__tests__/resolveMode.test.js b/packages/webpack-cli/__tests__/resolveMode.test.js index ca9d9910b74..3db6f889273 100644 --- a/packages/webpack-cli/__tests__/resolveMode.test.js +++ b/packages/webpack-cli/__tests__/resolveMode.test.js @@ -2,18 +2,24 @@ const resolveMode = require('../lib/groups/resolveMode'); describe('resolveMode', function () { it('should handle the mode option [production]', () => { - const result = resolveMode({ - mode: 'production', - }); + const result = resolveMode( + { + mode: 'production', + }, + {}, + ); // ensure no other properties are added expect(result.options).toMatchObject({ mode: 'production' }); expect(result.options.mode).toEqual('production'); }); it('should handle the mode option [development]', () => { - const result = resolveMode({ - mode: 'development', - }); + const result = resolveMode( + { + mode: 'development', + }, + {}, + ); // ensure no other properties are added expect(result.options).toMatchObject({ mode: 'development' }); @@ -21,22 +27,56 @@ describe('resolveMode', function () { }); it('should handle the mode option [none]', () => { - const result = resolveMode({ - mode: 'none', - }); + const result = resolveMode( + { + mode: 'none', + }, + {}, + ); // ensure no other properties are added expect(result.options).toMatchObject({ mode: 'none' }); expect(result.options.mode).toEqual('none'); }); - it('should prefer supplied move over NODE_ENV', () => { + it('should prefer supplied move flag over NODE_ENV', () => { process.env.NODE_ENV = 'production'; - const result = resolveMode({ - mode: 'development', - }); + const result = resolveMode( + { + mode: 'development', + }, + {}, + ); // ensure no other properties are added expect(result.options).toMatchObject({ mode: 'development' }); }); + + it('should prefer supplied move flag over mode from config', () => { + const result = resolveMode( + { + mode: 'development', + }, + { mode: 'production' }, + ); + + // ensure no other properties are added + expect(result.options).toMatchObject({ mode: 'development' }); + }); + + it('should prefer mode form config over NODE_ENV', () => { + process.env.NODE_ENV = 'development'; + const result = resolveMode({}, { mode: 'production' }); + + // ensure no other properties are added + expect(result.options).toMatchObject({ mode: 'production' }); + }); + + it('should prefer mode form flag over NODE_ENV and config', () => { + process.env.NODE_ENV = 'development'; + const result = resolveMode({ mode: 'none' }, { mode: 'production' }); + + // ensure no other properties are added + expect(result.options).toMatchObject({ mode: 'none' }); + }); }); diff --git a/packages/webpack-cli/lib/groups/resolveMode.js b/packages/webpack-cli/lib/groups/resolveMode.js index 20052ae4200..2e3a8a15acd 100644 --- a/packages/webpack-cli/lib/groups/resolveMode.js +++ b/packages/webpack-cli/lib/groups/resolveMode.js @@ -1,26 +1,49 @@ const PRODUCTION = 'production'; const DEVELOPMENT = 'development'; -const resolveMode = (args) => { +/* +Mode priority: + - Mode flag + - Mode from config + - Mode form NODE_ENV +*/ + +/** + * + * @param {string} mode - mode flag value + * @param {Object} configObject - contains relevant loaded config + */ +const assignMode = (mode, configObject) => { const { env: { NODE_ENV }, } = process; - const { mode } = args; - + const { mode: configMode } = configObject; let finalMode; - /** - * It determines the mode to pass to webpack compiler - * @returns {string} The mode - */ - if (!mode && NODE_ENV && (NODE_ENV === PRODUCTION || NODE_ENV === DEVELOPMENT)) { + if (mode) { + finalMode = mode; + } else if (configMode) { + finalMode = configMode; + } else if (NODE_ENV && (NODE_ENV === PRODUCTION || NODE_ENV === DEVELOPMENT)) { finalMode = NODE_ENV; } else { - finalMode = mode || PRODUCTION; + finalMode = PRODUCTION; } + return { mode: finalMode }; +}; + +/** + * + * @param {Object} args - parsedArgs from the CLI + * @param {Object | Array} configOptions - Contains loaded config or array of configs + */ +const resolveMode = (args, configOptions) => { + const { mode } = args; + let resolvedMode; + if (Array.isArray(configOptions)) { + resolvedMode = configOptions.map((configObject) => assignMode(mode, configObject)); + } else resolvedMode = assignMode(mode, configOptions); - return { - options: { mode: finalMode }, - }; + return { options: resolvedMode }; }; module.exports = resolveMode; diff --git a/packages/webpack-cli/lib/webpack-cli.js b/packages/webpack-cli/lib/webpack-cli.js index b815f5a1f4e..de3320550e0 100644 --- a/packages/webpack-cli/lib/webpack-cli.js +++ b/packages/webpack-cli/lib/webpack-cli.js @@ -77,8 +77,8 @@ class WebpackCLI extends GroupHelper { this._mergeOptionsToOutputConfiguration(resolvedConfig.outputOptions); } - async _baseResolver(cb, parsedArgs) { - const resolvedConfig = cb(parsedArgs); + async _baseResolver(cb, parsedArgs, configOptions) { + const resolvedConfig = cb(parsedArgs, configOptions); this._mergeOptionsToConfiguration(resolvedConfig.options); this._mergeOptionsToOutputConfiguration(resolvedConfig.outputOptions); } @@ -143,6 +143,21 @@ class WebpackCLI extends GroupHelper { * @returns {void} */ _mergeOptionsToConfiguration(options, strategy) { + /** + * options where they differ per config use this method to apply relevant option to relevant config + * eg mode flag applies per config + */ + if (Array.isArray(options) && Array.isArray(this.compilerConfiguration)) { + this.compilerConfiguration = options.map((option, index) => { + const compilerConfig = this.compilerConfiguration[index]; + if (strategy) { + return webpackMerge.strategy(strategy)(compilerConfig, option); + } + return webpackMerge(compilerConfig, option); + }); + return; + } + /** * options is an array (multiple configuration) so we create a new * configuration where each element is individually merged @@ -229,9 +244,9 @@ class WebpackCLI extends GroupHelper { */ async runOptionGroups(parsedArgs) { await Promise.resolve() - .then(() => this._baseResolver(resolveMode, parsedArgs)) .then(() => this._handleDefaultEntry()) .then(() => this._handleConfig(parsedArgs)) + .then(() => this._baseResolver(resolveMode, parsedArgs, this.compilerConfiguration)) .then(() => this._handleGroupHelper(this.outputGroup)) .then(() => this._handleCoreFlags()) .then(() => this._handleGroupHelper(this.basicGroup)) diff --git a/test/mode/mode-single-arg/mode-single-arg.test.js b/test/mode/mode-single-arg/mode-single-arg.test.js index d995308080a..93dcc74cdc8 100644 --- a/test/mode/mode-single-arg/mode-single-arg.test.js +++ b/test/mode/mode-single-arg/mode-single-arg.test.js @@ -26,6 +26,12 @@ describe('mode flags', () => { expect(stdout).toContain(`mode: 'none'`); }); + it('should pick mode form NODE_ENV', () => { + const { stderr, stdout } = run(__dirname, [], false, [], { NODE_ENV: 'development' }); + expect(stderr).toBeFalsy(); + expect(stdout).toContain(`mode: 'development'`); + }); + it('should throw error when --mode=abcd is passed', () => { const { stderr } = run(__dirname, ['--mode', 'abcd']); expect(stderr).toContain('configuration.mode should be one of these'); diff --git a/test/mode/mode-with-config/mode-with-config.test.js b/test/mode/mode-with-config/mode-with-config.test.js index 45900f83205..0609608f14d 100644 --- a/test/mode/mode-with-config/mode-with-config.test.js +++ b/test/mode/mode-with-config/mode-with-config.test.js @@ -92,10 +92,53 @@ describe('mode flags with config', () => { }); }); - it('should use mode from config over flags', () => { + it('should use mode flag over config', () => { const { stdout, stderr, exitCode } = run(__dirname, ['--mode', 'production', '-c', 'webpack.config2.js']); expect(stderr).toBeFalsy(); expect(exitCode).toEqual(0); + expect(stdout).toContain(`mode: 'production'`); + }); + + it('should use mode from flag over NODE_ENV', () => { + const { stdout, stderr, exitCode } = run(__dirname, ['--mode', 'none', '-c', 'webpack.config2.js'], false, [], { + NODE_ENV: 'production', + }); + expect(stderr).toBeFalsy(); + expect(exitCode).toEqual(0); + expect(stdout).toContain(`mode: 'none'`); + }); + + it('should use mode from config over NODE_ENV', () => { + const { stdout, stderr, exitCode } = run(__dirname, ['-c', 'webpack.config2.js']); + expect(stderr).toBeFalsy(); + expect(exitCode).toEqual(0); + expect(stdout).toContain(`mode: 'development'`); + }); + + it('should use mode from config when multiple config are supplied', () => { + const { stdout, stderr } = run(__dirname, ['-c', 'webpack.config3.js', '-c', 'webpack.config2.js']); + expect(stderr).toBeFalsy(); + expect(stdout).toContain(`mode: 'development'`); + expect(stdout.match(new RegExp("mode: 'development'", 'g')).length).toEqual(1); + }); + + it('mode flag should apply to all configs', () => { + const { stdout, stderr, exitCode } = run(__dirname, ['--mode', 'none', '-c', './webpack.config3.js', '-c', './webpack.config2.js']); + expect(stderr).toBeFalsy(); + expect(exitCode).toEqual(0); + expect(stdout).toContain(`mode: 'none'`); + expect(stdout.match(new RegExp("mode: 'none'", 'g')).length).toEqual(2); + }); + + it('only config where mode is absent pick up from NODE_ENV', () => { + const { stdout, stderr, exitCode } = run(__dirname, ['-c', './webpack.config3.js', '-c', './webpack.config2.js'], false, [], { + NODE_ENV: 'production', + }); + expect(stderr).toBeFalsy(); + expect(exitCode).toEqual(0); + expect(stdout).toContain(`mode: 'production'`); expect(stdout).toContain(`mode: 'development'`); + expect(stdout.match(new RegExp("mode: 'production'", 'g')).length).toEqual(1); + expect(stdout.match(new RegExp("mode: 'development'", 'g')).length).toEqual(1); }); }); diff --git a/test/mode/mode-with-config/webpack.config3.js b/test/mode/mode-with-config/webpack.config3.js new file mode 100644 index 00000000000..841f518c92d --- /dev/null +++ b/test/mode/mode-with-config/webpack.config3.js @@ -0,0 +1,6 @@ +// eslint-disable-next-line node/no-unpublished-require +const WebpackCLITestPlugin = require('../../utils/webpack-cli-test-plugin'); + +module.exports = { + plugins: [new WebpackCLITestPlugin()], +}; diff --git a/test/utils/test-utils.js b/test/utils/test-utils.js index 032f15de886..a4060b46405 100644 --- a/test/utils/test-utils.js +++ b/test/utils/test-utils.js @@ -19,7 +19,7 @@ const isWebpack5 = version.startsWith('5'); * @param {Boolean} setOutput Boolean that decides if a default output path will be set or not * @returns {Object} The webpack output or Promise when nodeOptions are present */ -function run(testCase, args = [], setOutput = true, nodeArgs = []) { +function run(testCase, args = [], setOutput = true, nodeArgs = [], env) { const cwd = path.resolve(testCase); const outputPath = path.resolve(testCase, 'bin'); @@ -29,6 +29,7 @@ function run(testCase, args = [], setOutput = true, nodeArgs = []) { cwd, reject: false, nodeOptions: nodeArgs, + env, stdio: ENABLE_LOG_COMPILATION ? 'inherit' : 'pipe', });