Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

migrate to commander #1481

Merged
merged 44 commits into from
Apr 25, 2020
Merged
Show file tree
Hide file tree
Changes from 43 commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
e663d10
chore: use commander
rishabh3112 Mar 18, 2020
87403bf
chore: add support for help and version command
rishabh3112 Mar 18, 2020
ec8c474
chore: remove duplicate code for no-mode
rishabh3112 Mar 18, 2020
f7009ae
chore: create helpers
rishabh3112 Mar 18, 2020
af62829
chore: remove global flag
rishabh3112 Mar 19, 2020
eab3500
chore: improve arg parsing and tests
rishabh3112 Mar 19, 2020
ab17bf0
chore: remove command-line-args
rishabh3112 Mar 21, 2020
327fc7d
chore: remove instance of command-line-args
rishabh3112 Mar 21, 2020
05173d8
chore: add explaination to the change
rishabh3112 Mar 21, 2020
5244654
chore: fix
rishabh3112 Mar 22, 2020
777f8e5
chore: migrate serve package
rishabh3112 Mar 24, 2020
b9a4932
chore: handle error
rishabh3112 Mar 24, 2020
ad0cc8f
chore: reactor argparser
rishabh3112 Mar 26, 2020
1b3212d
chore: get serve args from process.argv
rishabh3112 Mar 26, 2020
503a1f7
chore: sync message with test
rishabh3112 Mar 26, 2020
7cf46bf
chore: fix styling
knagaitsev Apr 17, 2020
231f181
chore: fix problem with no mode flag
knagaitsev Apr 18, 2020
0e1db3b
chore: started adding info parsing
knagaitsev Apr 18, 2020
24cbc85
chore: parsing without first few parts of process argv
knagaitsev Apr 18, 2020
cbbed32
chore: fixed args calling in serve
knagaitsev Apr 18, 2020
edf38f2
chore: fixing serve
knagaitsev Apr 18, 2020
a7fa090
chore: fix no-mode tests
knagaitsev Apr 18, 2020
ab8e885
chore: fix output defaults tests
knagaitsev Apr 18, 2020
c4a4d69
chore: handle default webpack entry arg
knagaitsev Apr 18, 2020
4eccb84
chore: fix basic config test
knagaitsev Apr 18, 2020
26c65cc
chore: fix version and help commands
knagaitsev Apr 18, 2020
783f456
chore: change help, version, and fix entry checking
knagaitsev Apr 18, 2020
29aa5c0
chore: add some negated flag handling
knagaitsev Apr 23, 2020
f1888cf
chore: fix serve test
knagaitsev Apr 24, 2020
793e51e
chore: rename misleading parse-args to parse-node-args
knagaitsev Apr 24, 2020
03b7618
chore: added arg-parser tests
knagaitsev Apr 24, 2020
575fbe8
chore: use args passed to serve
knagaitsev Apr 24, 2020
fa0f145
chore: fix stats flag handling
knagaitsev Apr 24, 2020
91ee8f5
chore: fix function as a custom type
knagaitsev Apr 24, 2020
9240879
chore: change boolean logic in arg parser
knagaitsev Apr 24, 2020
f4c94da
chore: fixed stats test and removed stats custom type
knagaitsev Apr 24, 2020
11282ee
chore: removed output todo and added tests for warning
knagaitsev Apr 24, 2020
8339c7b
chore: changed invalid mode message and updated test
knagaitsev Apr 24, 2020
426c666
chore: add comment about commander negation
knagaitsev Apr 24, 2020
2deb7e3
chore: made output tests sync
knagaitsev Apr 24, 2020
dfb6f6c
chore: fix spelling in comment
knagaitsev Apr 24, 2020
c068011
chore: remove unnecessary log
knagaitsev Apr 25, 2020
9351796
chore: add runPromptWithAnswers changes for stability
knagaitsev Apr 25, 2020
d160106
Merge branch 'next' into chore/migrate-commander
jamesgeorge007 Apr 25, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/info/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const DEFAULT_DETAILS: Information = {

export default async function info(...args): Promise<string[]> {
const cli = new WebpackCLI();
const infoArgs = cli.commandLineArgs(options, { argv: args, stopAtFirstUnknown: false });
const infoArgs = cli.argParser(options, args, true).opts;
const envinfoConfig = {};

if (infoArgs._unknown && infoArgs._unknown.length > 0) {
Expand Down
1 change: 1 addition & 0 deletions packages/info/src/options.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
export default [
{
name: 'output',
usage: '--output <json | markdown>',
type: String,
description: 'To get the output in specified format (accept json or markdown)',
},
Expand Down
33 changes: 20 additions & 13 deletions packages/serve/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { devServer } from 'webpack-dev-server/bin/cli-flags';
import WebpackCLI from 'webpack-cli';
import logger from 'webpack-cli/lib/utils/logger';
import startDevServer from './startDevServer';
import argsToCamelCase from './args-to-camel-case';

/**
*
Expand All @@ -10,23 +10,30 @@ import argsToCamelCase from './args-to-camel-case';
* @param {String[]} args - args processed from the CLI
* @returns {Function} invokes the devServer API
*/
export default function serve(...args): void {
export default function serve(...args: string[]): void {
const cli = new WebpackCLI();
const core = cli.getCoreFlags();
// partial parsing usage: https://github.com/75lb/command-line-args/wiki/Partial-parsing

// since the webpack flags have the 'entry' option set as it's default option,
// we need to parse the dev server args first. Otherwise, the webpack parsing could snatch
// one of the dev server's options and set it to this 'entry' option.
// see: https://github.com/75lb/command-line-args/blob/master/doc/option-definition.md#optiondefaultoption--boolean
const devServerArgs = cli.commandLineArgs(devServer, { argv: args, partial: true });
const webpackArgs = cli.commandLineArgs(core, { argv: devServerArgs._unknown || [], stopAtFirstUnknown: false });
const finalArgs = argsToCamelCase(devServerArgs._all || {});
const parsedDevServerArgs = cli.argParser(devServer, args, true);
const devServerArgs = parsedDevServerArgs.opts;
const parsedWebpackArgs = cli.argParser(core, parsedDevServerArgs.unknownArgs, true, process.title);
const webpackArgs = parsedWebpackArgs.opts;

// pass along the 'hot' argument to the dev server if it exists
if (webpackArgs && webpackArgs._all && typeof webpackArgs._all.hot !== 'undefined') {
finalArgs['hot'] = webpackArgs._all.hot;
if (webpackArgs && webpackArgs.hot !== undefined) {
devServerArgs['hot'] = webpackArgs.hot;
}

if (parsedWebpackArgs.unknownArgs.length > 0) {
parsedWebpackArgs.unknownArgs
.filter((e) => e)
.forEach((unknown) => {
logger.warn('Unknown argument:', unknown);
});
return;
}

cli.getCompiler(webpackArgs, core).then((compiler): void => {
startDevServer(compiler, finalArgs);
startDevServer(compiler, devServerArgs);
});
}
201 changes: 201 additions & 0 deletions packages/webpack-cli/__tests__/arg-parser.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,201 @@
const warnMock = jest.fn();
jest.mock('../lib/utils/logger', () => {
return {
warn: warnMock,
};
});
jest.spyOn(process, 'exit').mockImplementation(() => {});

const argParser = require('../lib/utils/arg-parser');
const { core } = require('../lib/utils/cli-flags');

const basicOptions = [
{
name: 'bool-flag',
alias: 'b',
usage: '--bool-flag',
type: Boolean,
description: 'boolean flag',
},
{
name: 'string-flag',
usage: '--string-flag <value>',
type: String,
description: 'string flag',
},
{
name: 'string-flag-with-default',
usage: '--string-flag-with-default <value>',
type: String,
description: 'string flag',
defaultValue: 'default-value',
},
{
name: 'custom-type-flag',
usage: '--custom-type-flag <value>',
type: (val) => {
return val.split(',');
},
description: 'custom type flag',
},
];

const helpAndVersionOptions = basicOptions.slice(0);
helpAndVersionOptions.push(
{
name: 'help',
usage: '--help',
type: Boolean,
description: 'help',
},
{
name: 'version',
alias: 'v',
usage: '--version',
type: Boolean,
description: 'version',
},
);

describe('arg-parser', () => {
beforeEach(() => {
warnMock.mockClear();
});

it('parses no flags', () => {
const res = argParser(basicOptions, [], true);
expect(res.unknownArgs.length).toEqual(0);
expect(res.opts).toEqual({
stringFlagWithDefault: 'default-value',
});
expect(warnMock.mock.calls.length).toEqual(0);
});

it('parses basic flags', () => {
const res = argParser(basicOptions, ['--bool-flag', '--string-flag', 'val'], true);
expect(res.unknownArgs.length).toEqual(0);
expect(res.opts).toEqual({
boolFlag: true,
stringFlag: 'val',
stringFlagWithDefault: 'default-value',
});
expect(warnMock.mock.calls.length).toEqual(0);
});

it('parses negated boolean flags', () => {
const res = argParser(basicOptions, ['--no-bool-flag'], true);
expect(res.unknownArgs.length).toEqual(0);
expect(res.opts).toEqual({
boolFlag: false,
stringFlagWithDefault: 'default-value',
});
expect(warnMock.mock.calls.length).toEqual(0);
});

it('parses boolean flag alias', () => {
const res = argParser(basicOptions, ['-b'], true);
expect(res.unknownArgs.length).toEqual(0);
expect(res.opts).toEqual({
boolFlag: true,
stringFlagWithDefault: 'default-value',
});
expect(warnMock.mock.calls.length).toEqual(0);
});

it('warns on usage of both flag and same negated flag, setting it to false', () => {
const res = argParser(basicOptions, ['--bool-flag', '--no-bool-flag'], true);
expect(res.unknownArgs.length).toEqual(0);
expect(res.opts).toEqual({
boolFlag: false,
stringFlagWithDefault: 'default-value',
});
expect(warnMock.mock.calls.length).toEqual(1);
expect(warnMock.mock.calls[0][0]).toContain('You provided both --bool-flag and --no-bool-flag');
});

it('warns on usage of both flag and same negated flag, setting it to true', () => {
const res = argParser(basicOptions, ['--no-bool-flag', '--bool-flag'], true);
expect(res.unknownArgs.length).toEqual(0);
expect(res.opts).toEqual({
boolFlag: true,
stringFlagWithDefault: 'default-value',
});
expect(warnMock.mock.calls.length).toEqual(1);
expect(warnMock.mock.calls[0][0]).toContain('You provided both --bool-flag and --no-bool-flag');
});

it('warns on usage of both flag alias and same negated flag, setting it to true', () => {
const res = argParser(basicOptions, ['--no-bool-flag', '-b'], true);
expect(res.unknownArgs.length).toEqual(0);
expect(res.opts).toEqual({
boolFlag: true,
stringFlagWithDefault: 'default-value',
});
expect(warnMock.mock.calls.length).toEqual(1);
expect(warnMock.mock.calls[0][0]).toContain('You provided both -b and --no-bool-flag');
});

it('parses string flag using equals sign', () => {
const res = argParser(basicOptions, ['--string-flag=val'], true);
expect(res.unknownArgs.length).toEqual(0);
expect(res.opts).toEqual({
stringFlag: 'val',
stringFlagWithDefault: 'default-value',
});
expect(warnMock.mock.calls.length).toEqual(0);
});

it('handles additional node args from argv', () => {
const res = argParser(basicOptions, ['node', 'index.js', '--bool-flag', '--string-flag', 'val'], false);
expect(res.unknownArgs.length).toEqual(0);
expect(res.opts).toEqual({
boolFlag: true,
stringFlag: 'val',
stringFlagWithDefault: 'default-value',
});
expect(warnMock.mock.calls.length).toEqual(0);
});

it('handles unknown args', () => {
const res = argParser(basicOptions, ['--unknown-arg', '-b', 'no-leading-dashes'], true);
expect(res.unknownArgs).toEqual(['--unknown-arg', 'no-leading-dashes']);
expect(res.opts).toEqual({
boolFlag: true,
stringFlagWithDefault: 'default-value',
});
expect(warnMock.mock.calls.length).toEqual(0);
});

it('handles custom type args', () => {
const res = argParser(basicOptions, ['--custom-type-flag', 'val1,val2,val3'], true);
expect(res.unknownArgs.length).toEqual(0);
expect(res.opts).toEqual({
customTypeFlag: ['val1', 'val2', 'val3'],
stringFlagWithDefault: 'default-value',
});
expect(warnMock.mock.calls.length).toEqual(0);
});

it('calls help callback on --help', () => {
const helpCb = jest.fn();
argParser(helpAndVersionOptions, ['--help'], true, '', helpCb);
expect(helpCb.mock.calls.length).toEqual(1);
expect(helpCb.mock.calls[0][0]).toEqual(['--help']);
});

it('calls version callback on --version', () => {
const versionCb = jest.fn();
argParser(helpAndVersionOptions, ['--version'], true, '', () => {}, versionCb);
expect(versionCb.mock.calls.length).toEqual(1);
});

it('parses webpack args', () => {
const res = argParser(core, ['--entry', 'test.js', '--hot', '-o', './dist/', '--no-watch'], true);
expect(res.unknownArgs.length).toEqual(0);
expect(res.opts.entry).toEqual('test.js');
expect(res.opts.hot).toBeTruthy();
expect(res.opts.output).toEqual('./dist/');
expect(res.opts.watch).toBeFalsy();
expect(warnMock.mock.calls.length).toEqual(0);
});
});
4 changes: 2 additions & 2 deletions packages/webpack-cli/bin/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
'use strict';
require('v8-compile-cache');
const importLocal = require('import-local');
const parseArgs = require('../lib/utils/parse-args');
const parseNodeArgs = require('../lib/utils/parse-node-args');
const runner = require('../lib/runner');

// Prefer the local installation of webpack-cli
Expand All @@ -13,6 +13,6 @@ if (importLocal(__filename)) {
process.title = 'webpack';

const [, , ...rawArgs] = process.argv;
const { cliArgs, nodeArgs } = parseArgs(rawArgs);
const { cliArgs, nodeArgs } = parseNodeArgs(rawArgs);

runner(nodeArgs, cliArgs);
69 changes: 30 additions & 39 deletions packages/webpack-cli/lib/bootstrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ const WebpackCLI = require('./webpack-cli');
const { core, commands } = require('./utils/cli-flags');
const logger = require('./utils/logger');
const cliExecuter = require('./utils/cli-executer');

const argParser = require('./utils/arg-parser');
require('./utils/process-log');

process.title = 'webpack-cli';

const isFlagPresent = (args, flag) => args.find((arg) => [flag, `--${flag}`].includes(arg));
// 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) => {
Expand All @@ -20,39 +20,21 @@ const isCommandUsed = (commands) =>
return process.argv.includes(cmd.name) || process.argv.includes(cmd.alias);
});

const resolveNegatedArgs = (args) => {
args._unknown.forEach((arg, idx) => {
if (arg.includes('--') || arg.includes('--no')) {
const argPair = arg.split('=');
const optName = arg.includes('--no') ? argPair[0].slice(5) : argPair[0].slice(2);

let argValue = arg.includes('--no') ? 'false' : argPair[1];
if (argValue === 'false') {
argValue = false;
} else if (argValue === 'true') {
argValue = true;
}
const cliFlag = core.find((opt) => opt.name === optName);
if (cliFlag) {
args[cliFlag.group][optName] = argValue;
args._all[optName] = argValue;
args._unknown[idx] = null;
}
}
});
};

async function runCLI(cli, commandIsUsed) {
let args;
const helpFlagExists = isFlagPresent(process.argv, 'help');
const versionFlagExists = isFlagPresent(process.argv, 'version') || isFlagPresent(process.argv, '-v');
const runVersion = () => {
cli.runVersion(commandIsUsed);
};
const parsedArgs = argParser(core, process.argv, false, process.title, cli.runHelp, runVersion);

if (helpFlagExists) {
if (parsedArgs.unknownArgs.includes('help')) {
cli.runHelp(process.argv);
return;
} else if (versionFlagExists) {
cli.runVersion(commandIsUsed);
return;
process.exit(0);
}

if (parsedArgs.unknownArgs.includes('version')) {
runVersion();
process.exit(0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can using help (https://github.com/tj/commander.js/#helpcb and https://github.com/tj/commander.js/#addhelpcommand) for help, same for version? Yes we can do it in future

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, just don't want to do it here because it will probably involve some significant rewriting of https://github.com/webpack/webpack-cli/blob/next/packages/webpack-cli/lib/groups/HelpGroup.js

}

if (commandIsUsed) {
Expand All @@ -61,18 +43,28 @@ async function runCLI(cli, commandIsUsed) {
return await cli.runCommand(commandIsUsed, ...args);
} else {
try {
args = cli.commandLineArgs(core, { stopAtFirstUnknown: false, partial: true });
if (args._unknown) {
resolveNegatedArgs(args);
args._unknown
// 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 === 1 && !parsedArgs.unknownArgs[0].startsWith('-')) {
entry = parsedArgs.unknownArgs[0];
} else if (parsedArgs.unknownArgs.length > 0) {
parsedArgs.unknownArgs
.filter((e) => e)
.forEach((unknown) => {
logger.warn('Unknown argument:', unknown);
});
cliExecuter();
return;
}
const result = await cli.run(args, core);
const parsedArgsOpts = parsedArgs.opts;
if (entry) {
parsedArgsOpts.entry = entry;
}
const result = await cli.run(parsedArgsOpts, core);
if (!result) {
return;
}
Expand All @@ -99,9 +91,8 @@ async function runCLI(cli, commandIsUsed) {
const newArgKeys = Object.keys(argsMap).filter((arg) => !keysToDelete.includes(argsMap[arg].pos));
// eslint-disable-next-line require-atomic-updates
process.argv = newArgKeys;
args = cli.commandLineArgs(core, { stopAtFirstUnknown: false, partial: true });

await cli.run(args, core);
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 {
Expand Down
Loading