Skip to content

Commit

Permalink
fix: prevent info from running unnecessarily (#1650)
Browse files Browse the repository at this point in the history 
* refactor: leverage arg-parser lib to parse commands

* chore: ignore unknown commands

* fix: prevent default behavior of arg-parser lib

* chore: implement flags for info subcommand

* chore: fall back to info --output

* refactor: remove debug snippets

* tests: test case to ensure info is not invoked unnecessarily
  • Loading branch information
@jamesgeorge007
jamesgeorge007 authored Jun 30, 2020
1 parent 68bd892 commit ddee5ad
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 92 deletions.
136 changes: 63 additions & 73 deletions packages/webpack-cli/lib/bootstrap.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,6 @@ require('./utils/process-log');

process.title = 'webpack-cli';

// const isFlagPresent = (args, flag) => args.find((arg) => [flag, `--${flag}`].includes(arg));
const isArgCommandName = (arg, cmd) => arg === cmd.name || arg === cmd.alias;
const removeCmdFromArgs = (args, cmd) => args.filter((arg) => !isArgCommandName(arg, cmd));
const normalizeFlags = (args, cmd) => {
const slicedArgs = args.slice(2);
return removeCmdFromArgs(slicedArgs, cmd);
};

const isCommandUsed = (commands) =>
commands.find((cmd) => {
return process.argv.includes(cmd.name) || process.argv.includes(cmd.alias);
Expand All @@ -25,7 +17,7 @@ async function runCLI(cli, commandIsUsed) {
const runVersion = () => {
cli.runVersion(process.argv, commandIsUsed);
};
const parsedArgs = argParser(core, process.argv, false, process.title, cli.runHelp, runVersion);
const parsedArgs = argParser(core, process.argv, false, process.title, cli.runHelp, runVersion, commands);

if (parsedArgs.unknownArgs.includes('help')) {
cli.runHelp(process.argv);
Expand All @@ -38,74 +30,72 @@ async function runCLI(cli, commandIsUsed) {
}

if (commandIsUsed) {
commandIsUsed.defaultOption = true;
args = normalizeFlags(process.argv, commandIsUsed);
return await cli.runCommand(commandIsUsed, ...args);
} else {
try {
// handle the default webpack entry CLI argument, where instead
// of doing 'webpack-cli --entry ./index.js' you can simply do
// 'webpack-cli ./index.js'
// if the unknown arg starts with a '-', it will be considered
// an unknown flag rather than an entry
let entry;
if (parsedArgs.unknownArgs.length > 0 && !parsedArgs.unknownArgs[0].startsWith('-')) {
if (parsedArgs.unknownArgs.length === 1) {
entry = parsedArgs.unknownArgs[0];
} else {
entry = [];
parsedArgs.unknownArgs.forEach((unknown) => {
if (!unknown.startsWith('-')) {
entry.push(unknown);
}
});
}
} else if (parsedArgs.unknownArgs.length > 0) {
return;
}

try {
// handle the default webpack entry CLI argument, where instead
// of doing 'webpack-cli --entry ./index.js' you can simply do
// 'webpack-cli ./index.js'
// if the unknown arg starts with a '-', it will be considered
// an unknown flag rather than an entry
let entry;
if (parsedArgs.unknownArgs.length > 0 && !parsedArgs.unknownArgs[0].startsWith('-')) {
if (parsedArgs.unknownArgs.length === 1) {
entry = parsedArgs.unknownArgs[0];
} else {
entry = [];
parsedArgs.unknownArgs.forEach((unknown) => {
logger.warn('Unknown argument:', unknown);
});
cliExecuter();
return;
}
const parsedArgsOpts = parsedArgs.opts;
if (entry) {
parsedArgsOpts.entry = entry;
}
const result = await cli.run(parsedArgsOpts, core);
if (!result) {
return;
}
} catch (err) {
if (err.name === 'UNKNOWN_VALUE') {
logger.error(`Parse Error (unknown argument): ${err.value}`);
return;
} else if (err.name === 'ALREADY_SET') {
const argsMap = {};
const keysToDelete = [];
process.argv.forEach((arg, idx) => {
const oldMapValue = argsMap[arg];
argsMap[arg] = {
value: process.argv[idx],
pos: idx,
};
// Swap idx of overridden value
if (oldMapValue) {
argsMap[arg].pos = oldMapValue.pos;
keysToDelete.push(idx + 1);
if (!unknown.startsWith('-')) {
entry.push(unknown);
}
});
// Filter out the value for the overridden key
const newArgKeys = Object.keys(argsMap).filter((arg) => !keysToDelete.includes(argsMap[arg].pos));
// eslint-disable-next-line require-atomic-updates
process.argv = newArgKeys;
args = argParser('', core, process.argv);
await cli.run(args.opts, core);
process.stdout.write('\n');
logger.warn('Duplicate flags found, defaulting to last set value');
} else {
logger.error(err);
return;
}
} else if (parsedArgs.unknownArgs.length > 0) {
parsedArgs.unknownArgs.forEach((unknown) => {
logger.warn('Unknown argument:', unknown);
});
cliExecuter();
return;
}
const parsedArgsOpts = parsedArgs.opts;
if (entry) {
parsedArgsOpts.entry = entry;
}
const result = await cli.run(parsedArgsOpts, core);
if (!result) {
return;
}
} catch (err) {
if (err.name === 'UNKNOWN_VALUE') {
logger.error(`Parse Error (unknown argument): ${err.value}`);
return;
} else if (err.name === 'ALREADY_SET') {
const argsMap = {};
const keysToDelete = [];
process.argv.forEach((arg, idx) => {
const oldMapValue = argsMap[arg];
argsMap[arg] = {
value: process.argv[idx],
pos: idx,
};
// Swap idx of overridden value
if (oldMapValue) {
argsMap[arg].pos = oldMapValue.pos;
keysToDelete.push(idx + 1);
}
});
// Filter out the value for the overridden key
const newArgKeys = Object.keys(argsMap).filter((arg) => !keysToDelete.includes(argsMap[arg].pos));
// eslint-disable-next-line require-atomic-updates
process.argv = newArgKeys;
args = argParser('', core, process.argv);
await cli.run(args.opts, core);
process.stdout.write('\n');
logger.warn('Duplicate flags found, defaulting to last set value');
} else {
logger.error(err);
return;
}
}
}
Expand Down
39 changes: 34 additions & 5 deletions packages/webpack-cli/lib/utils/arg-parser.js
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,15 @@
const commander = require('commander');
const logger = require('./logger');

const defaultCommands = {
init: 'init',
loader: 'generate-loader',
plugin: 'generate-plugin',
info: 'info',
migrate: 'migrate',
serve: 'serve',
};

/**
* Creates Argument parser corresponding to the supplied options
* parse the args and return the result
Expand All @@ -10,11 +19,30 @@ const logger = require('./logger');
* @param {boolean} argsOnly false if all of process.argv has been provided, true if
* args is only a subset of process.argv that removes the first couple elements
*/
function argParser(options, args, argsOnly = false, name = '', helpFunction = undefined, versionFunction = undefined) {
function argParser(options, args, argsOnly = false, name = '', helpFunction = undefined, versionFunction = undefined, commands) {
const parser = new commander.Command();
// Set parser name
parser.name(name);

if (commands) {
commands.reduce((parserInstance, cmd) => {
parser
.command(cmd.name)
.alias(cmd.alias)
.description(cmd.description)
.usage(cmd.usage)
.allowUnknownOption(true)
.action(async () => {
const cliArgs = args.slice(args.indexOf(cmd.name) + 1 || args.indexOf(cmd.alias) + 1);
return await require('../commands/ExternalCommand').run(defaultCommands[cmd.name], ...cliArgs);
});
return parser;
}, parser);

// Prevent default behavior
parser.on('command:*', () => {});
}

// Use customized version output if available
if (versionFunction) {
parser.on('option:version', () => {
Expand All @@ -40,20 +68,21 @@ function argParser(options, args, argsOnly = false, name = '', helpFunction = un
let flagsWithType = option.type !== Boolean ? flags + ' <value>' : flags;
if (option.type === Boolean || option.type === String) {
if (!option.multiple) {
parserInstance.option(flagsWithType, option.description, option.defaultValue);
// Prevent default behavior for standalone options
parserInstance.option(flagsWithType, option.description, option.defaultValue).action(() => {});
} else {
const multiArg = (value, previous = []) => previous.concat([value]);
parserInstance.option(flagsWithType, option.description, multiArg, option.defaultValue);
parserInstance.option(flagsWithType, option.description, multiArg, option.defaultValue).action(() => {});
}
} else if (option.type === Number) {
parserInstance.option(flagsWithType, option.description, Number, option.defaultValue);
} else {
// in this case the type is a parsing function
if (option.type.length > 1) {
flagsWithType = flags + ' [value]';
parserInstance.option(flagsWithType, option.description, option.type[0], option.defaultValue);
parserInstance.option(flagsWithType, option.description, option.type[0], option.defaultValue).action(() => {});
} else {
parserInstance.option(flagsWithType, option.description, option.type, option.defaultValue);
parserInstance.option(flagsWithType, option.description, option.type, option.defaultValue).action(() => {});
}
}

Expand Down
14 changes: 0 additions & 14 deletions packages/webpack-cli/lib/webpack-cli.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,6 @@ const webpackMerge = require('webpack-merge');
const { toKebabCase } = require('./utils/helpers');
const argParser = require('./utils/arg-parser');

const defaultCommands = {
init: 'init',
loader: 'generate-loader',
plugin: 'generate-plugin',
info: 'info',
migrate: 'migrate',
serve: 'serve',
};

class WebpackCLI extends GroupHelper {
constructor() {
super();
Expand Down Expand Up @@ -256,11 +247,6 @@ class WebpackCLI extends GroupHelper {
return webpack;
}

async runCommand(command, ...args) {
// TODO: rename and depreciate init
return await require('./commands/ExternalCommand').run(defaultCommands[command.name], ...args);
}

runHelp(args) {
const HelpGroup = require('./groups/HelpGroup');
const { commands, allNames, hasUnknownArgs } = require('./utils/unknown-args');
Expand Down
7 changes: 7 additions & 0 deletions test/serve/basic/serve-basic.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,13 @@ describe('basic serve usage', () => {
console.warn('TODO: fix `serve` test on windows');
});
} else {
it('should not invoke info subcommand', async () => {
const { stdout, stderr } = await runServe(['--client-log-level', 'info'], testPath);
expect(stdout).toContain('main.js');
expect(stdout).not.toContain('hot/dev-server.js');
expect(stderr).toHaveLength(0);
});

it('compiles without flags', async () => {
const { stdout, stderr } = await runServe(['--port', port], testPath);
expect(stdout).toContain('main.js');
Expand Down

0 comments on commit ddee5ad

Please sign in to comment.