Skip to content

Commit f6ae0e1

Browse files
snitin315snitin315
authored
fix: parsing built-in arguments (#2455)
1 parent f010690 commit f6ae0e1

14 files changed

+430
-227
lines changed

‎.github/workflows/nodejs.yml

+2-2
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,9 @@ jobs:
7171
path: |
7272
node_modules
7373
*/*/node_modules
74-
key: ${{ runner.os }}-${{ matrix.webpack-version }}-yarn-${{ hashFiles('**/yarn.lock', './yarn.lock') }}
74+
key: a-${{ runner.os }}-${{ matrix.webpack-version }}-yarn-${{ hashFiles('**/yarn.lock', './yarn.lock') }}
7575
restore-keys: |
76-
${{ runner.os }}-${{ matrix.webpack-version }}-yarn-
76+
a-${{ runner.os }}-${{ matrix.webpack-version }}-yarn-
7777
7878
- name: Install dependencies
7979
run: yarn

‎OPTIONS.md

+107-54
Large diffs are not rendered by default.

‎packages/webpack-cli/lib/webpack-cli.js

+150-84
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ const path = require('path');
33
const { pathToFileURL } = require('url');
44
const Module = require('module');
55

6-
const { program } = require('commander');
6+
const { program, Option } = require('commander');
77
const utils = require('./utils');
88

99
class WebpackCLI {
@@ -116,107 +116,182 @@ class WebpackCLI {
116116
}
117117

118118
makeOption(command, option) {
119-
let type = option.type;
120-
let isMultipleTypes = Array.isArray(type);
121-
let isOptional = false;
122-
123-
if (isMultipleTypes) {
124-
if (type.length === 1) {
125-
type = type[0];
126-
isMultipleTypes = false;
127-
} else {
128-
isOptional = type.includes(Boolean);
119+
let mainOption;
120+
let negativeOption;
121+
122+
if (option.configs) {
123+
let needNegativeOption = false;
124+
let mainOptionType = new Set();
125+
126+
option.configs.forEach((config) => {
127+
// Possible value: "enum" | "string" | "path" | "number" | "boolean" | "RegExp" | "reset"
128+
switch (config.type) {
129+
case 'reset':
130+
mainOptionType.add(Boolean);
131+
break;
132+
case 'boolean':
133+
if (!needNegativeOption) {
134+
needNegativeOption = true;
135+
}
136+
137+
mainOptionType.add(Boolean);
138+
break;
139+
case 'number':
140+
mainOptionType.add(Number);
141+
break;
142+
case 'string':
143+
case 'path':
144+
case 'RegExp':
145+
mainOptionType.add(String);
146+
break;
147+
case 'enum': {
148+
let hasFalseEnum = false;
149+
150+
const enumTypes = config.values.map((value) => {
151+
switch (typeof value) {
152+
case 'string':
153+
mainOptionType.add(String);
154+
break;
155+
case 'number':
156+
mainOptionType.add(Number);
157+
break;
158+
case 'boolean':
159+
if (!hasFalseEnum && value === false) {
160+
hasFalseEnum = true;
161+
break;
162+
}
163+
164+
mainOptionType.add(Boolean);
165+
break;
166+
}
167+
});
168+
169+
if (!needNegativeOption) {
170+
needNegativeOption = hasFalseEnum;
171+
}
172+
173+
return enumTypes;
174+
}
175+
}
176+
});
177+
178+
mainOption = {
179+
flags: option.alias ? `-${option.alias}, --${option.name}` : `--${option.name}`,
180+
description: option.description || '',
181+
type: mainOptionType,
182+
multiple: option.multiple,
183+
};
184+
185+
if (needNegativeOption) {
186+
negativeOption = {
187+
flags: `--no-${option.name}`,
188+
description: option.negatedDescription ? option.negatedDescription : `Negative '${option.name}' option.`,
189+
};
190+
}
191+
} else {
192+
mainOption = {
193+
flags: option.alias ? `-${option.alias}, --${option.name}` : `--${option.name}`,
194+
// TODO `describe` used by `webpack-dev-server@3`
195+
description: option.description || option.describe || '',
196+
type: option.type ? new Set(Array.isArray(option.type) ? option.type : [option.type]) : new Set([Boolean]),
197+
multiple: option.multiple,
198+
defaultValue: option.defaultValue,
199+
};
200+
201+
if (option.negative) {
202+
negativeOption = {
203+
flags: `--no-${option.name}`,
204+
description: option.negatedDescription ? option.negatedDescription : `Negative '${option.name}' option.`,
205+
};
129206
}
130207
}
131208

132-
const isMultiple = option.multiple;
133-
const isRequired = type !== Boolean && typeof type !== 'undefined';
209+
if (mainOption.type.size > 1 && mainOption.type.has(Boolean)) {
210+
mainOption.flags = `${mainOption.flags} [value${mainOption.multiple ? '...' : ''}]`;
211+
} else if (mainOption.type.size > 0 && !mainOption.type.has(Boolean)) {
212+
mainOption.flags = `${mainOption.flags} <value${mainOption.multiple ? '...' : ''}>`;
213+
}
134214

135-
let flags = option.alias ? `-${option.alias}, --${option.name}` : `--${option.name}`;
215+
if (mainOption.type.size === 1) {
216+
if (mainOption.type.has(Number)) {
217+
let skipDefault = true;
136218

137-
if (isOptional) {
138-
// `commander.js` recognizes [value] as an optional placeholder, making this flag work either as a string or a boolean
139-
flags = `${flags} [value${isMultiple ? '...' : ''}]`;
140-
} else if (isRequired) {
141-
// <value> is a required placeholder for any non-Boolean types
142-
flags = `${flags} <value${isMultiple ? '...' : ''}>`;
143-
}
219+
const optionForCommand = new Option(mainOption.flags, mainOption.description)
220+
.argParser((value, prev = []) => {
221+
if (mainOption.defaultValue && mainOption.multiple && skipDefault) {
222+
prev = [];
223+
skipDefault = false;
224+
}
144225

145-
// TODO `describe` used by `webpack-dev-server@3`
146-
const description = option.description || option.describe || '';
147-
const defaultValue = option.defaultValue;
226+
return mainOption.multiple ? [].concat(prev).concat(Number(value)) : Number(value);
227+
})
228+
.default(mainOption.defaultValue);
148229

149-
if (type === Boolean) {
150-
command.option(flags, description, defaultValue);
151-
} else if (type === Number) {
152-
let skipDefault = true;
230+
command.addOption(optionForCommand);
231+
} else if (mainOption.type.has(String)) {
232+
let skipDefault = true;
153233

154-
command.option(
155-
flags,
156-
description,
157-
(value, prev = []) => {
158-
if (defaultValue && isMultiple && skipDefault) {
159-
prev = [];
160-
skipDefault = false;
161-
}
234+
const optionForCommand = new Option(mainOption.flags, mainOption.description)
235+
.argParser((value, prev = []) => {
236+
if (mainOption.defaultValue && mainOption.multiple && skipDefault) {
237+
prev = [];
238+
skipDefault = false;
239+
}
162240

163-
return isMultiple ? [].concat(prev).concat(Number(value)) : Number(value);
164-
},
165-
defaultValue,
166-
);
167-
} else if (type === String) {
168-
let skipDefault = true;
241+
return mainOption.multiple ? [].concat(prev).concat(value) : value;
242+
})
243+
.default(mainOption.defaultValue);
169244

170-
command.option(
171-
flags,
172-
description,
173-
(value, prev = []) => {
174-
if (defaultValue && isMultiple && skipDefault) {
175-
prev = [];
176-
skipDefault = false;
177-
}
245+
command.addOption(optionForCommand);
246+
} else if (mainOption.type.has(Boolean)) {
247+
const optionForCommand = new Option(mainOption.flags, mainOption.description).default(mainOption.defaultValue);
178248

179-
return isMultiple ? [].concat(prev).concat(value) : value;
180-
},
181-
defaultValue,
182-
);
183-
} else if (isMultipleTypes) {
249+
command.addOption(optionForCommand);
250+
} else {
251+
const optionForCommand = new Option(mainOption.flags, mainOption.description)
252+
.argParser(Array.from(mainOption.type)[0])
253+
.default(mainOption.defaultValue);
254+
255+
command.addOption(optionForCommand);
256+
}
257+
} else if (mainOption.type.size > 1) {
184258
let skipDefault = true;
185259

186-
command.option(
187-
flags,
188-
description,
189-
(value, prev = []) => {
190-
if (defaultValue && isMultiple && skipDefault) {
260+
const optionForCommand = new Option(mainOption.flags, mainOption.description, mainOption.defaultValue)
261+
.argParser((value, prev = []) => {
262+
if (mainOption.defaultValue && mainOption.multiple && skipDefault) {
191263
prev = [];
192264
skipDefault = false;
193265
}
194266

195-
if (type.includes(Number)) {
267+
if (mainOption.type.has(Number)) {
196268
const numberValue = Number(value);
197269

198270
if (!isNaN(numberValue)) {
199-
return isMultiple ? [].concat(prev).concat(numberValue) : numberValue;
271+
return mainOption.multiple ? [].concat(prev).concat(numberValue) : numberValue;
200272
}
201273
}
202274

203-
if (type.includes(String)) {
204-
return isMultiple ? [].concat(prev).concat(value) : value;
275+
if (mainOption.type.has(String)) {
276+
return mainOption.multiple ? [].concat(prev).concat(value) : value;
205277
}
206278

207279
return value;
208-
},
209-
defaultValue,
210-
);
211-
} else {
212-
command.option(flags, description, type, defaultValue);
213-
}
280+
})
281+
.default(mainOption.defaultValue);
282+
283+
command.addOption(optionForCommand);
284+
} else if (mainOption.type.size === 0 && negativeOption) {
285+
const optionForCommand = new Option(mainOption.flags, mainOption.description);
286+
287+
// Hide stub option
288+
optionForCommand.hideHelp();
214289

215-
if (option.negative) {
216-
// commander requires explicitly adding the negated version of boolean flags
217-
const negatedFlag = `--no-${option.name}`;
290+
command.addOption(optionForCommand);
291+
}
218292

219-
command.option(negatedFlag, option.negatedDescription ? option.negatedDescription : `Negative '${option.name}' option.`);
293+
if (negativeOption) {
294+
command.addOption(new Option(negativeOption.flags, negativeOption.description));
220295
}
221296
}
222297

@@ -403,15 +478,6 @@ class WebpackCLI {
403478
// A list of cli flags generated by core can be found here https://github.com/webpack/webpack/blob/master/test/__snapshots__/Cli.test.js.snap
404479
const coreFlags = this.webpack.cli
405480
? Object.entries(this.webpack.cli.getArguments()).map(([flag, meta]) => {
406-
if (meta.simpleType === 'string') {
407-
meta.type = String;
408-
} else if (meta.simpleType === 'number') {
409-
meta.type = Number;
410-
} else {
411-
meta.type = Boolean;
412-
meta.negative = !flag.endsWith('-reset');
413-
}
414-
415481
const inBuiltIn = builtInFlags.find((builtInFlag) => builtInFlag.name === flag);
416482

417483
if (inBuiltIn) {
@@ -670,7 +736,7 @@ class WebpackCLI {
670736
}
671737

672738
command.options.forEach((option) => {
673-
if (this.utils.levenshtein.distance(name, option.long.slice(2)) < 3) {
739+
if (!option.hidden && this.utils.levenshtein.distance(name, option.long.slice(2)) < 3) {
674740
this.logger.error(`Did you mean '--${option.name()}'?`);
675741
}
676742
});

‎test/core-flags/experiments-flag.test.js

+5-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,11 @@ describe('experiments option related flag', () => {
1919

2020
const propName = hyphenToUpperCase(property);
2121

22-
if (flag.type === Boolean) {
22+
if (propName === 'client' || propName === 'test') {
23+
return false;
24+
}
25+
26+
if (flag.configs.filter((config) => config.type === 'boolean').length > 0) {
2327
it(`should config --${flag.name} correctly`, () => {
2428
const { exitCode, stderr, stdout } = run(__dirname, [`--${flag.name}`]);
2529

‎test/core-flags/mock/mock.js

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
console.log('MOCK');

‎test/core-flags/module-flags.test.js

+27-3
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,11 @@ describe('module config related flag', () => {
1717

1818
const propName = hyphenToUpperCase(property);
1919

20-
if (flag.type === Boolean && !flag.name.includes('module-no-parse') && !flag.name.includes('module-parser-')) {
20+
if (
21+
flag.configs.filter((config) => config.type === 'boolean').length > 0 &&
22+
!flag.name.includes('module-no-parse') &&
23+
!flag.name.includes('module-parser-')
24+
) {
2125
it(`should config --${flag.name} correctly`, () => {
2226
if (flag.name.includes('-reset')) {
2327
const { stderr, stdout } = run(__dirname, [`--${flag.name}`]);
@@ -67,7 +71,10 @@ describe('module config related flag', () => {
6771
}
6872
}
6973

70-
if (flag.type === String && !(flag.name.includes('module-parser-') || flag.name.startsWith('module-generator'))) {
74+
if (
75+
flag.configs.filter((config) => config.type === 'string').length > 0 &&
76+
!(flag.name.includes('module-parser-') || flag.name.startsWith('module-generator'))
77+
) {
7178
it(`should config --${flag.name} correctly`, () => {
7279
if (flag.name === 'module-no-parse') {
7380
let { stderr, stdout, exitCode } = run(__dirname, [`--${flag.name}`, 'value']);
@@ -120,7 +127,7 @@ describe('module config related flag', () => {
120127
expect(stdout).toContain(`generator: { asset: { dataUrl: [Object] } }`);
121128
});
122129

123-
it('should config module.parser flags coorectly', () => {
130+
it('should config module.parser flags correctly', () => {
124131
const { exitCode, stderr, stdout } = run(__dirname, [
125132
'--module-parser-javascript-browserify',
126133
'--module-parser-javascript-commonjs',
@@ -138,4 +145,21 @@ describe('module config related flag', () => {
138145
expect(stdout).toContain('import: true');
139146
expect(stdout).toContain('node: false');
140147
});
148+
149+
it('should accept --module-parser-javascript-url=relative', () => {
150+
const { exitCode, stderr, stdout } = run(__dirname, ['--module-parser-javascript-url', 'relative']);
151+
152+
expect(exitCode).toBe(0);
153+
expect(stderr).toBeFalsy();
154+
expect(stdout).toContain(`url: 'relative'`);
155+
});
156+
157+
it('should throw an error for an invalid value of --module-parser-javascript-url', () => {
158+
const { exitCode, stderr, stdout } = run(__dirname, ['--module-parser-javascript-url', 'test']);
159+
160+
expect(exitCode).toBe(2);
161+
expect(stderr).toContain(`Invalid value 'test' for the '--module-parser-javascript-url' option`);
162+
expect(stderr).toContain(`Expected: 'relative'`);
163+
expect(stdout).toBeFalsy();
164+
});
141165
});

0 commit comments

Comments
 (0)