From bb1483707f3af9bedae115fb51c6084888f0e950 Mon Sep 17 00:00:00 2001 From: Nitin Kumar Date: Sun, 18 Apr 2021 07:31:00 +0530 Subject: [PATCH 1/4] fix: improve parsing of `--env` flag --- packages/webpack-cli/lib/webpack-cli.js | 11 +++++++++-- .../type/function-with-env/function-with-env.test.js | 10 ++++++++++ .../config/type/function-with-env/webpack.config.js | 8 ++++++++ 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/packages/webpack-cli/lib/webpack-cli.js b/packages/webpack-cli/lib/webpack-cli.js index c5ba9ff9e15..b8b0db06b69 100644 --- a/packages/webpack-cli/lib/webpack-cli.js +++ b/packages/webpack-cli/lib/webpack-cli.js @@ -373,8 +373,11 @@ class WebpackCLI { { name: 'env', type: (value, previous = {}) => { + // for https://github.com/webpack/webpack-cli/issues/2642 + const regExpForSplitting = (value.match(/=/g) || []).length === 1 ? /=/ : /=(.+)/; + // This ensures we're only splitting by the first `=` - const [allKeys, val] = value.split(/=(.+)/, 2); + const [allKeys, val] = value.split(regExpForSplitting, 2); const splitKeys = allKeys.split(/\.(?!$)/); let prevRef = previous; @@ -389,7 +392,11 @@ class WebpackCLI { } if (index === splitKeys.length - 1) { - prevRef[someKey] = val || true; + if (typeof val === 'string') { + prevRef[someKey] = val; + } else { + prevRef[someKey] = true; + } } prevRef = prevRef[someKey]; diff --git a/test/build/config/type/function-with-env/function-with-env.test.js b/test/build/config/type/function-with-env/function-with-env.test.js index b31b2e546ad..2c07fb01697 100644 --- a/test/build/config/type/function-with-env/function-with-env.test.js +++ b/test/build/config/type/function-with-env/function-with-env.test.js @@ -117,6 +117,16 @@ describe('function configuration', () => { expect(existsSync(resolve(__dirname, './dist/true.js'))).toBeTruthy(); }); + it('Supports empty string', async () => { + const { exitCode, stderr, stdout } = await run(__dirname, ['--env', `foo=''`]); + + expect(exitCode).toBe(0); + expect(stderr).toBeFalsy(); + expect(stdout).toBeTruthy(); + // Should generate the appropriate files + expect(existsSync(resolve(__dirname, './dist/empty-string.js'))).toBeTruthy(); + }); + it('is able to understand multiple env flags', async () => { const { exitCode, stderr, stdout } = await run(__dirname, ['--env', 'isDev', '--env', 'verboseStats', '--env', 'envMessage']); diff --git a/test/build/config/type/function-with-env/webpack.config.js b/test/build/config/type/function-with-env/webpack.config.js index 35efedbbb0d..8fe8deb18e8 100644 --- a/test/build/config/type/function-with-env/webpack.config.js +++ b/test/build/config/type/function-with-env/webpack.config.js @@ -9,6 +9,14 @@ module.exports = (env) => { }, }; } + if (env.foo === `''`) { + return { + entry: './a.js', + output: { + filename: 'empty-string.js', + }, + }; + } return { entry: './a.js', mode: 'development', From 00ed5a9eca2ded49868cae62ff7690cca28164a2 Mon Sep 17 00:00:00 2001 From: Nitin Kumar Date: Mon, 19 Apr 2021 07:02:30 +0530 Subject: [PATCH 2/4] refactor: code --- packages/webpack-cli/lib/webpack-cli.js | 6 ++++-- .../type/function-with-env/function-with-env.test.js | 10 ++++++++++ .../config/type/function-with-env/webpack.config.js | 8 ++++++++ 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/packages/webpack-cli/lib/webpack-cli.js b/packages/webpack-cli/lib/webpack-cli.js index b8b0db06b69..2a7f54f98a2 100644 --- a/packages/webpack-cli/lib/webpack-cli.js +++ b/packages/webpack-cli/lib/webpack-cli.js @@ -374,10 +374,12 @@ class WebpackCLI { name: 'env', type: (value, previous = {}) => { // for https://github.com/webpack/webpack-cli/issues/2642 - const regExpForSplitting = (value.match(/=/g) || []).length === 1 ? /=/ : /=(.+)/; + if (value.endsWith('=')) { + value.concat(`''`); + } // This ensures we're only splitting by the first `=` - const [allKeys, val] = value.split(regExpForSplitting, 2); + const [allKeys, val] = value.split(/=(.+)/, 2); const splitKeys = allKeys.split(/\.(?!$)/); let prevRef = previous; diff --git a/test/build/config/type/function-with-env/function-with-env.test.js b/test/build/config/type/function-with-env/function-with-env.test.js index 2c07fb01697..cdb80700457 100644 --- a/test/build/config/type/function-with-env/function-with-env.test.js +++ b/test/build/config/type/function-with-env/function-with-env.test.js @@ -127,6 +127,16 @@ describe('function configuration', () => { expect(existsSync(resolve(__dirname, './dist/empty-string.js'))).toBeTruthy(); }); + it('Supports empty string with multiple "="', async () => { + const { exitCode, stderr, stdout } = await run(__dirname, ['--env', `foo=bar=''`]); + + expect(exitCode).toBe(0); + expect(stderr).toBeFalsy(); + expect(stdout).toBeTruthy(); + // Should generate the appropriate files + expect(existsSync(resolve(__dirname, './dist/new-empty-string.js'))).toBeTruthy(); + }); + it('is able to understand multiple env flags', async () => { const { exitCode, stderr, stdout } = await run(__dirname, ['--env', 'isDev', '--env', 'verboseStats', '--env', 'envMessage']); diff --git a/test/build/config/type/function-with-env/webpack.config.js b/test/build/config/type/function-with-env/webpack.config.js index 8fe8deb18e8..9114623d545 100644 --- a/test/build/config/type/function-with-env/webpack.config.js +++ b/test/build/config/type/function-with-env/webpack.config.js @@ -17,6 +17,14 @@ module.exports = (env) => { }, }; } + if (env.foo === `bar=''`) { + return { + entry: './a.js', + output: { + filename: 'new-empty-string.js', + }, + }; + } return { entry: './a.js', mode: 'development', From 2220b3e9953f87638b4f22de3ccb36057e0c54fb Mon Sep 17 00:00:00 2001 From: Nitin Kumar Date: Mon, 19 Apr 2021 07:06:05 +0530 Subject: [PATCH 3/4] fix: lint --- packages/webpack-cli/lib/webpack-cli.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/webpack-cli/lib/webpack-cli.js b/packages/webpack-cli/lib/webpack-cli.js index 2a7f54f98a2..522a6e1259f 100644 --- a/packages/webpack-cli/lib/webpack-cli.js +++ b/packages/webpack-cli/lib/webpack-cli.js @@ -375,7 +375,7 @@ class WebpackCLI { type: (value, previous = {}) => { // for https://github.com/webpack/webpack-cli/issues/2642 if (value.endsWith('=')) { - value.concat(`''`); + value.concat('""'); } // This ensures we're only splitting by the first `=` From b7447c9ba9ea31faf56d4d02aa12848ce06eb612 Mon Sep 17 00:00:00 2001 From: evilebottnawi Date: Mon, 19 Apr 2021 16:43:18 +0300 Subject: [PATCH 4/4] fix: some edge cases --- packages/webpack-cli/lib/webpack-cli.js | 15 +++++++++------ .../function-with-env/function-with-env.test.js | 10 ++++++++++ .../type/function-with-env/webpack.config.js | 8 ++++++++ .../help/__snapshots__/help.test.js.snap.webpack4 | 2 ++ .../help/__snapshots__/help.test.js.snap.webpack5 | 2 ++ test/help/help.test.js | 8 ++++++++ 6 files changed, 39 insertions(+), 6 deletions(-) diff --git a/packages/webpack-cli/lib/webpack-cli.js b/packages/webpack-cli/lib/webpack-cli.js index 522a6e1259f..449aa9b4ba6 100644 --- a/packages/webpack-cli/lib/webpack-cli.js +++ b/packages/webpack-cli/lib/webpack-cli.js @@ -1221,13 +1221,13 @@ class WebpackCLI { const defaultCommandToRun = getCommandName(buildCommandOptions.name); const hasOperand = typeof operands[0] !== 'undefined'; const operand = hasOperand ? operands[0] : defaultCommandToRun; - + const isHelpOption = typeof options.help !== 'undefined'; const isHelpCommandSyntax = isCommand(operand, helpCommandOptions); - if (options.help || isHelpCommandSyntax) { + if (isHelpOption || isHelpCommandSyntax) { let isVerbose = false; - if (options.help) { + if (isHelpOption) { if (typeof options.help === 'string') { if (options.help !== 'verbose') { this.logger.error("Unknown value for '--help' option, please use '--help=verbose'"); @@ -1241,7 +1241,7 @@ class WebpackCLI { this.program.forHelp = true; const optionsForHelp = [] - .concat(options.help && hasOperand ? [operand] : []) + .concat(isHelpOption && hasOperand ? [operand] : []) // Syntax `webpack help [command]` .concat(operands.slice(1)) // Syntax `webpack help [option]` @@ -1252,9 +1252,12 @@ class WebpackCLI { await outputHelp(optionsForHelp, isVerbose, isHelpCommandSyntax, program); } - if (options.version || isCommand(operand, versionCommandOptions)) { + const isVersionOption = typeof options.version !== 'undefined'; + const isVersionCommandSyntax = isCommand(operand, versionCommandOptions); + + if (isVersionOption || isVersionCommandSyntax) { const optionsForVersion = [] - .concat(options.version ? [operand] : []) + .concat(isVersionOption ? [operand] : []) .concat(operands.slice(1)) .concat(unknown); diff --git a/test/build/config/type/function-with-env/function-with-env.test.js b/test/build/config/type/function-with-env/function-with-env.test.js index cdb80700457..539cc160684 100644 --- a/test/build/config/type/function-with-env/function-with-env.test.js +++ b/test/build/config/type/function-with-env/function-with-env.test.js @@ -137,6 +137,16 @@ describe('function configuration', () => { expect(existsSync(resolve(__dirname, './dist/new-empty-string.js'))).toBeTruthy(); }); + it('Supports env variable with "=" at the end', async () => { + const { exitCode, stderr, stdout } = await run(__dirname, ['--env', `foo=`]); + + expect(exitCode).toBe(0); + expect(stderr).toBeFalsy(); + expect(stdout).toBeTruthy(); + // Should generate the appropriate files + expect(existsSync(resolve(__dirname, './dist/equal-at-the-end.js'))).toBeTruthy(); + }); + it('is able to understand multiple env flags', async () => { const { exitCode, stderr, stdout } = await run(__dirname, ['--env', 'isDev', '--env', 'verboseStats', '--env', 'envMessage']); diff --git a/test/build/config/type/function-with-env/webpack.config.js b/test/build/config/type/function-with-env/webpack.config.js index 9114623d545..5a726c711f9 100644 --- a/test/build/config/type/function-with-env/webpack.config.js +++ b/test/build/config/type/function-with-env/webpack.config.js @@ -25,6 +25,14 @@ module.exports = (env) => { }, }; } + if (env['foo=']) { + return { + entry: './a.js', + output: { + filename: 'equal-at-the-end.js', + }, + }; + } return { entry: './a.js', mode: 'development', diff --git a/test/help/__snapshots__/help.test.js.snap.webpack4 b/test/help/__snapshots__/help.test.js.snap.webpack4 index a17dfec40c0..238420633fd 100644 --- a/test/help/__snapshots__/help.test.js.snap.webpack4 +++ b/test/help/__snapshots__/help.test.js.snap.webpack4 @@ -28,6 +28,8 @@ exports[`help should log error for invalid command using the "--help" option 1`] exports[`help should log error for invalid flag with the "--help" option #2 1`] = `"[webpack-cli] Unknown value for '--help' option, please use '--help=verbose'"`; +exports[`help should log error for invalid flag with the "--help" option #2 2`] = `"[webpack-cli] Unknown value for '--help' option, please use '--help=verbose'"`; + exports[`help should log error for invalid flag with the "--help" option 1`] = ` "[webpack-cli] Incorrect use of help [webpack-cli] Please use: 'webpack help [command] [option]' | 'webpack [command] --help' diff --git a/test/help/__snapshots__/help.test.js.snap.webpack5 b/test/help/__snapshots__/help.test.js.snap.webpack5 index 7297f62c0f0..444cffabc89 100644 --- a/test/help/__snapshots__/help.test.js.snap.webpack5 +++ b/test/help/__snapshots__/help.test.js.snap.webpack5 @@ -28,6 +28,8 @@ exports[`help should log error for invalid command using the "--help" option 1`] exports[`help should log error for invalid flag with the "--help" option #2 1`] = `"[webpack-cli] Unknown value for '--help' option, please use '--help=verbose'"`; +exports[`help should log error for invalid flag with the "--help" option #2 2`] = `"[webpack-cli] Unknown value for '--help' option, please use '--help=verbose'"`; + exports[`help should log error for invalid flag with the "--help" option 1`] = ` "[webpack-cli] Incorrect use of help [webpack-cli] Please use: 'webpack help [command] [option]' | 'webpack [command] --help' diff --git a/test/help/help.test.js b/test/help/help.test.js index 189fcf095d7..e3dbba1cf1d 100644 --- a/test/help/help.test.js +++ b/test/help/help.test.js @@ -402,4 +402,12 @@ describe('help', () => { expect(stderr).toMatchSnapshot(); expect(stdout).toBeFalsy(); }); + + it('should log error for invalid flag with the "--help" option #2', async () => { + const { exitCode, stderr, stdout } = await run(__dirname, ['--help=']); + + expect(exitCode).toBe(2); + expect(stderr).toMatchSnapshot(); + expect(stdout).toBeFalsy(); + }); });