From a9837b1923cd6e35f559a4ebb9d0ac25cf6605fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 7 Mar 2019 23:43:43 +0100 Subject: [PATCH 1/6] chore: simplify commander/minimist usage --- packages/cli/package.json | 2 +- packages/cli/src/cliEntry.js | 72 ++++++++----------- .../cli/src/tools/assertRequiredOptions.js | 31 -------- yarn.lock | 5 +- 4 files changed, 33 insertions(+), 77 deletions(-) delete mode 100644 packages/cli/src/tools/assertRequiredOptions.js diff --git a/packages/cli/package.json b/packages/cli/package.json index 5662ced6b..fff93c214 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -20,7 +20,7 @@ }, "dependencies": { "chalk": "^1.1.1", - "commander": "^2.9.0", + "commander": "^2.19.0", "compression": "^1.7.1", "connect": "^3.6.5", "denodeify": "^1.2.1", diff --git a/packages/cli/src/cliEntry.js b/packages/cli/src/cliEntry.js index 90ee8dea5..b8f143c65 100644 --- a/packages/cli/src/cliEntry.js +++ b/packages/cli/src/cliEntry.js @@ -10,20 +10,19 @@ import chalk from 'chalk'; import childProcess from 'child_process'; import commander from 'commander'; -import minimist from 'minimist'; import path from 'path'; import type {CommandT, ContextT} from './tools/types.flow'; import getLegacyConfig from './tools/getLegacyConfig'; import {getCommands} from './commands'; import init from './commands/init/init'; -import assertRequiredOptions from './tools/assertRequiredOptions'; import logger from './tools/logger'; import pkg from '../package.json'; commander - .version(pkg.version) + .option('--version', 'Print CLI version') .option('--projectRoot [string]', 'Path to the root of the project') - .option('--reactNativePath [string]', 'Path to React Native'); + .option('--reactNativePath [string]', 'Path to React Native') + .parse(process.argv); commander.on('command:*', () => { printUnknownCommand(commander.args.join(' ')); @@ -39,35 +38,34 @@ const handleError = err => { // Custom printHelpInformation command inspired by internal Commander.js // one modified to suit our needs -function printHelpInformation() { +function printHelpInformation(examples) { let cmdName = this._name; if (this._alias) { cmdName = `${cmdName}|${this._alias}`; } const sourceInformation = this.pkg - ? [` ${chalk.bold('Source:')} ${this.pkg.name}@${this.pkg.version}`, ''] + ? [`${chalk.bold('Source:')} ${this.pkg.name}@${this.pkg.version}`, ''] : []; let output = [ + chalk.bold(`react-native ${cmdName} ${this.usage()}`), '', - chalk.bold(chalk.cyan(` react-native ${cmdName} ${this.usage()}`)), - this._description ? ` ${this._description}` : '', + this._description ? `${this._description}` : '', '', ...sourceInformation, - ` ${chalk.bold('Options:')}`, - '', - this.optionHelp().replace(/^/gm, ' '), + `${chalk.bold('Options:')}`, '', + this.optionHelp().replace(/^/gm, ' '), ]; - if (this.examples && this.examples.length > 0) { - const formattedUsage = this.examples - .map(example => ` ${example.desc}: \n ${chalk.cyan(example.cmd)}`) + if (examples && examples.length > 0) { + const formattedUsage = examples + .map(example => ` ${example.desc}: \n ${chalk.cyan(example.cmd)}`) .join('\n\n'); output = output.concat([ - chalk.bold(' Example usage:'), + chalk.bold('\nExample usage:'), '', formattedUsage, ]); @@ -93,26 +91,18 @@ const addCommand = (command: CommandT, ctx: ContextT) => { const options = command.options || []; const cmd = commander - .command(command.name, undefined, { - noHelp: !command.description, - }) + .command(command.name, undefined, {noHelp: !command.description}) .description(command.description) .action(function handleAction(...args) { const passedOptions = this.opts(); const argv: Array = Array.from(args).slice(0, -1); Promise.resolve() - .then(() => { - assertRequiredOptions(options, passedOptions); - return command.func(argv, ctx, passedOptions); - }) + .then(() => command.func(argv, ctx, passedOptions)) .catch(handleError); }); - cmd.helpInformation = printHelpInformation.bind(cmd); - cmd.examples = command.examples; - // $FlowFixMe: This is either null or not - cmd.pkg = command.pkg; + cmd.helpInformation = printHelpInformation.bind(cmd, command.examples); options.forEach(opt => cmd.option( @@ -122,11 +112,6 @@ const addCommand = (command: CommandT, ctx: ContextT) => { opt.default, ), ); - - // Redefined here to appear in the `--help` section - cmd - .option('--projectRoot [string]', 'Path to the root of the project') - .option('--reactNativePath [string]', 'Path to React Native'); }; async function run() { @@ -157,20 +142,12 @@ async function setupAndRun() { } } - /** - * Read passed `options` and take the "global" settings - * - * @todo(grabbou): Consider unifying this by removing either `commander` - * or `minimist` - */ - const options = minimist(process.argv.slice(2)); - - const root = options.projectRoot - ? path.resolve(options.projectRoot) + const root = commander.projectRoot + ? path.resolve(commander.projectRoot) : process.cwd(); - const reactNativePath = options.reactNativePath - ? path.resolve(options.reactNativePath) + const reactNativePath = commander.reactNativePath + ? path.resolve(commander.reactNativePath) : (() => { try { return path.dirname( @@ -198,9 +175,16 @@ async function setupAndRun() { commander.parse(process.argv); - if (!options._.length) { + if (commander.rawArgs.length === 2) { commander.outputHelp(); } + + // We handle --version as a special case like this because both `commander` + // and `yargs` append it to every command and we don't want to do that. + // E.g. outside command `init` has --version flag and we want to preserve it. + if (commander.args.length === 0 && commander.version === true) { + console.log(pkg.version); + } } export default { diff --git a/packages/cli/src/tools/assertRequiredOptions.js b/packages/cli/src/tools/assertRequiredOptions.js deleted file mode 100644 index c4de673c0..000000000 --- a/packages/cli/src/tools/assertRequiredOptions.js +++ /dev/null @@ -1,31 +0,0 @@ -/** - * Copyright (c) Facebook, Inc. and its affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - * - * @format - */ - -import {Option} from 'commander'; -import {camelCase} from 'lodash'; - -// Commander.js has a 2 years old open issue to support <...> syntax -// for options. Until that gets merged, we run the checks manually -// https://github.com/tj/commander.js/issues/230 -export default function assertRequiredOptions(options, passedOptions) { - options.forEach(opt => { - const option = new Option(opt.command); - - if (!option.required) { - return; - } - - const name = camelCase(option.long); - - if (!passedOptions[name]) { - // Provide commander.js like error message - throw new Error(`Option "${option.long}" is missing`); - } - }); -} diff --git a/yarn.lock b/yarn.lock index 004d46d55..77823481f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2381,17 +2381,20 @@ combined-stream@^1.0.6, combined-stream@~1.0.6: dependencies: delayed-stream "~1.0.0" -commander@^2.9.0: +commander@^2.19.0: version "2.19.0" resolved "https://registry.yarnpkg.com/commander/-/commander-2.19.0.tgz#f6198aa84e5b83c46054b94ddedbfed5ee9ff12a" + integrity sha512-6tvAOO+D6OENvRAh524Dh9jcfKTYDQAqvqezbCW82xj5X0pSrcpxtvRKHLG0yBY6SD7PSDrJaj+0AiOcKVd1Xg== commander@~2.13.0: version "2.13.0" resolved "https://registry.yarnpkg.com/commander/-/commander-2.13.0.tgz#6964bca67685df7c1f1430c584f07d7597885b9c" + integrity sha512-MVuS359B+YzaWqjCL/c+22gfryv+mCBPHAv3zyVI2GN8EY6IRP8VwtasXn8jyyhvvq84R4ImN1OKRtcbIasjYA== commander@~2.17.1: version "2.17.1" resolved "https://registry.yarnpkg.com/commander/-/commander-2.17.1.tgz#bd77ab7de6de94205ceacc72f1716d29f20a77bf" + integrity sha512-wPMUt6FnH2yzG95SA6mzjQOEKUU3aLaDEmzs1ti+1E9h+CsrZghRlqEM/EJ4KscsQVG8uNN4uVreUeT8+drlgg== compare-func@^1.3.1: version "1.3.2" From a891600304b314b15a6df775e2cbe34d9a81b70e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 8 Mar 2019 14:08:43 +0100 Subject: [PATCH 2/6] feedback --- packages/cli/src/cliEntry.js | 23 +++++++++----- .../cli/src/tools/assertRequiredOptions.js | 31 +++++++++++++++++++ 2 files changed, 47 insertions(+), 7 deletions(-) create mode 100644 packages/cli/src/tools/assertRequiredOptions.js diff --git a/packages/cli/src/cliEntry.js b/packages/cli/src/cliEntry.js index b8f143c65..4f8489534 100644 --- a/packages/cli/src/cliEntry.js +++ b/packages/cli/src/cliEntry.js @@ -15,8 +15,9 @@ import type {CommandT, ContextT} from './tools/types.flow'; import getLegacyConfig from './tools/getLegacyConfig'; import {getCommands} from './commands'; import init from './commands/init/init'; +import assertRequiredOptions from './tools/assertRequiredOptions'; import logger from './tools/logger'; -import pkg from '../package.json'; +import pkgJson from '../package.json'; commander .option('--version', 'Print CLI version') @@ -38,14 +39,14 @@ const handleError = err => { // Custom printHelpInformation command inspired by internal Commander.js // one modified to suit our needs -function printHelpInformation(examples) { +function printHelpInformation(examples, pkg) { let cmdName = this._name; if (this._alias) { cmdName = `${cmdName}|${this._alias}`; } - const sourceInformation = this.pkg - ? [`${chalk.bold('Source:')} ${this.pkg.name}@${this.pkg.version}`, ''] + const sourceInformation = pkg + ? [`${chalk.bold('Source:')} ${pkg.name}@${pkg.version}`, ''] : []; let output = [ @@ -98,11 +99,19 @@ const addCommand = (command: CommandT, ctx: ContextT) => { const argv: Array = Array.from(args).slice(0, -1); Promise.resolve() - .then(() => command.func(argv, ctx, passedOptions)) + .then(() => { + assertRequiredOptions(options, passedOptions); + return command.func(argv, ctx, passedOptions); + }) .catch(handleError); }); - cmd.helpInformation = printHelpInformation.bind(cmd, command.examples); + cmd.helpInformation = printHelpInformation.bind( + cmd, + command.examples, + // $FlowFixMe - we know pkg may be missing... + command.pkg, + ); options.forEach(opt => cmd.option( @@ -183,7 +192,7 @@ async function setupAndRun() { // and `yargs` append it to every command and we don't want to do that. // E.g. outside command `init` has --version flag and we want to preserve it. if (commander.args.length === 0 && commander.version === true) { - console.log(pkg.version); + console.log(pkgJson.version); } } diff --git a/packages/cli/src/tools/assertRequiredOptions.js b/packages/cli/src/tools/assertRequiredOptions.js new file mode 100644 index 000000000..c4de673c0 --- /dev/null +++ b/packages/cli/src/tools/assertRequiredOptions.js @@ -0,0 +1,31 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @format + */ + +import {Option} from 'commander'; +import {camelCase} from 'lodash'; + +// Commander.js has a 2 years old open issue to support <...> syntax +// for options. Until that gets merged, we run the checks manually +// https://github.com/tj/commander.js/issues/230 +export default function assertRequiredOptions(options, passedOptions) { + options.forEach(opt => { + const option = new Option(opt.command); + + if (!option.required) { + return; + } + + const name = camelCase(option.long); + + if (!passedOptions[name]) { + // Provide commander.js like error message + throw new Error(`Option "${option.long}" is missing`); + } + }); +} From 7bb247ca4764e1d653dfaa88a8b321c86bd98901 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 11 Mar 2019 10:43:00 +0100 Subject: [PATCH 3/6] remove extra parse --- packages/cli/src/cliEntry.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/cli/src/cliEntry.js b/packages/cli/src/cliEntry.js index 4f8489534..f8a630639 100644 --- a/packages/cli/src/cliEntry.js +++ b/packages/cli/src/cliEntry.js @@ -22,8 +22,7 @@ import pkgJson from '../package.json'; commander .option('--version', 'Print CLI version') .option('--projectRoot [string]', 'Path to the root of the project') - .option('--reactNativePath [string]', 'Path to React Native') - .parse(process.argv); + .option('--reactNativePath [string]', 'Path to React Native'); commander.on('command:*', () => { printUnknownCommand(commander.args.join(' ')); From 90036177383bb21e29efa5ca4b3da8e6104e2d08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 11 Mar 2019 11:23:35 +0100 Subject: [PATCH 4/6] add options back --- packages/cli/src/cliEntry.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/cli/src/cliEntry.js b/packages/cli/src/cliEntry.js index f8a630639..7a7c2f559 100644 --- a/packages/cli/src/cliEntry.js +++ b/packages/cli/src/cliEntry.js @@ -120,6 +120,15 @@ const addCommand = (command: CommandT, ctx: ContextT) => { opt.default, ), ); + + /** + * We want all commands (like "start", "link") to accept these flags, so we + * add them here instead of in every single command. This way they'll be + * displayed in commands --help menus. + */ + cmd + .option('--projectRoot [string]', 'Path to the root of the project') + .option('--reactNativePath [string]', 'Path to React Native'); }; async function run() { From 4882a930b6a9401bf40aad88958ddffdb9751b30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 11 Mar 2019 15:49:30 +0100 Subject: [PATCH 5/6] rephrase comment --- packages/cli/src/cliEntry.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/cliEntry.js b/packages/cli/src/cliEntry.js index 7a7c2f559..38cfc0eac 100644 --- a/packages/cli/src/cliEntry.js +++ b/packages/cli/src/cliEntry.js @@ -122,9 +122,9 @@ const addCommand = (command: CommandT, ctx: ContextT) => { ); /** - * We want all commands (like "start", "link") to accept these flags, so we - * add them here instead of in every single command. This way they'll be - * displayed in commands --help menus. + * We want every command (like "start", "link") to accept below options. + * To achieve that we append them to regular options of each command here. + * This way they'll be displayed in the commands --help menus. */ cmd .option('--projectRoot [string]', 'Path to the root of the project') From 89b91e665f98ed77edf4cd9940922773c759066d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 11 Mar 2019 16:08:05 +0100 Subject: [PATCH 6/6] remove excess whitespace --- packages/cli/src/cliEntry.js | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/packages/cli/src/cliEntry.js b/packages/cli/src/cliEntry.js index 38cfc0eac..7b03f702e 100644 --- a/packages/cli/src/cliEntry.js +++ b/packages/cli/src/cliEntry.js @@ -50,12 +50,9 @@ function printHelpInformation(examples, pkg) { let output = [ chalk.bold(`react-native ${cmdName} ${this.usage()}`), - '', - this._description ? `${this._description}` : '', - '', + this._description ? `\n${this._description}\n` : '', ...sourceInformation, `${chalk.bold('Options:')}`, - '', this.optionHelp().replace(/^/gm, ' '), ]; @@ -64,14 +61,10 @@ function printHelpInformation(examples, pkg) { .map(example => ` ${example.desc}: \n ${chalk.cyan(example.cmd)}`) .join('\n\n'); - output = output.concat([ - chalk.bold('\nExample usage:'), - '', - formattedUsage, - ]); + output = output.concat([chalk.bold('\nExample usage:'), formattedUsage]); } - return output.concat(['', '']).join('\n'); + return output.join('\n'); } function printUnknownCommand(cmdName) {