From 214311f495868bd1b0322ffc0b76e8a6cb5baf61 Mon Sep 17 00:00:00 2001 From: fatme Date: Mon, 15 Jul 2019 13:01:33 +0300 Subject: [PATCH] fix: respect correctly `hmr: false` from command's dashed options NativeScript CLI doesn't respect correctly `hmr: false` when the option is provided through dashed options from command. This led to the exception when the application is built using CLI and deployed on device using Xcode as the `hmr` is included in app's bundle. Fixes: https://github.com/NativeScript/nativescript-cli/issues/4846 https://github.com/NativeScript/nativescript-cli/issues/4814 --- lib/common/definitions/yargs.d.ts | 100 ------------------------ lib/common/services/commands-service.ts | 12 +-- lib/options.ts | 28 +++---- npm-shrinkwrap.json | 15 ++++ package.json | 1 + test/options.ts | 38 +++++++++ 6 files changed, 70 insertions(+), 124 deletions(-) delete mode 100644 lib/common/definitions/yargs.d.ts diff --git a/lib/common/definitions/yargs.d.ts b/lib/common/definitions/yargs.d.ts deleted file mode 100644 index aafee6b841..0000000000 --- a/lib/common/definitions/yargs.d.ts +++ /dev/null @@ -1,100 +0,0 @@ -// Type definitions for yargs -// Project: https://github.com/chevex/yargs -// Definitions by: Martin Poelstra -// Definitions: https://github.com/borisyankov/DefinitelyTyped - -declare module "yargs" { - - module yargs { - interface Argv { - argv: any; - (...args: any[]): any; - parse(...args: any[]): any; - - alias(shortName: string, longName: string): Argv; - alias(aliases: { [shortName: string]: string }): Argv; - alias(aliases: { [shortName: string]: string[] }): Argv; - - default(key: string, value: any): Argv; - default(defaults: { [key: string]: any}): Argv; - - demand(key: string, msg: string): Argv; - demand(key: string, required?: boolean): Argv; - demand(keys: string[], msg: string): Argv; - demand(keys: string[], required?: boolean): Argv; - demand(positionals: number, required?: boolean): Argv; - demand(positionals: number, msg: string): Argv; - - require(key: string, msg: string): Argv; - require(key: string, required: boolean): Argv; - require(keys: number[], msg: string): Argv; - require(keys: number[], required: boolean): Argv; - require(positionals: number, required: boolean): Argv; - require(positionals: number, msg: string): Argv; - - required(key: string, msg: string): Argv; - required(key: string, required: boolean): Argv; - required(keys: number[], msg: string): Argv; - required(keys: number[], required: boolean): Argv; - required(positionals: number, required: boolean): Argv; - required(positionals: number, msg: string): Argv; - - requiresArg(key: string): Argv; - requiresArg(keys: string[]): Argv; - - describe(key: string, description: string): Argv; - describe(descriptions: { [key: string]: string }): Argv; - - option(key: string, options: IDashedOption): Argv; - option(options: { [key: string]: IDashedOption }): Argv; - options(key: string, options: IDashedOption): Argv; - options(options: { [key: string]: IDashedOption }): Argv; - - usage(message: string, options?: { [key: string]: IDashedOption }): Argv; - usage(options?: { [key: string]: IDashedOption }): Argv; - - example(command: string, description: string): Argv; - - check(func: (argv: { [key: string]: any }, aliases: { [alias: string]: string }) => boolean): Argv; - check(func: (argv: { [key: string]: any }, aliases: { [alias: string]: string }) => string): Argv; - - boolean(key: string): Argv; - boolean(keys: string[]): Argv; - - string(key: string): Argv; - string(keys: string[]): Argv; - - config(key: string): Argv; - config(keys: string[]): Argv; - - wrap(columns: number): Argv; - - strict(): Argv; - - help(): string; - help(option: string, description?: string): Argv; - - version(version: string, option?: string, description?: string): Argv; - - showHelpOnFail(enable: boolean, message?: string): Argv; - - showHelp(func?: (message: string) => any): Argv; - - /* Undocumented */ - - normalize(key: string): Argv; - normalize(keys: string[]): Argv; - - implies(key: string, value: string): Argv; - implies(implies: { [key: string]: string }): Argv; - - count(key: string): Argv; - count(keys: string[]): Argv; - - fail(func: (msg: string) => any): void; - } - } - - var yargs: yargs.Argv; - export = yargs; -} \ No newline at end of file diff --git a/lib/common/services/commands-service.ts b/lib/common/services/commands-service.ts index b4bb6566d0..5b2bd90d5d 100644 --- a/lib/common/services/commands-service.ts +++ b/lib/common/services/commands-service.ts @@ -27,8 +27,7 @@ export class CommandsService implements ICommandsService { private $staticConfig: Config.IStaticConfig, private $helpService: IHelpService, private $extensibilityService: IExtensibilityService, - private $optionsTracker: IOptionsTracker, - private $projectDataService: IProjectDataService) { + private $optionsTracker: IOptionsTracker) { } public allCommands(opts: { includeDevCommands: boolean }): string[] { @@ -106,15 +105,8 @@ export class CommandsService implements ICommandsService { private async tryExecuteCommandAction(commandName: string, commandArguments: string[]): Promise { const command = this.$injector.resolveCommand(commandName); if (!command || !command.isHierarchicalCommand) { - let projectData = null; - try { - projectData = this.$projectDataService.getProjectData(); - } catch (err) { - this.$logger.trace(`Error while trying to get project data. More info: ${err}`); - } - const dashedOptions = command ? command.dashedOptions : null; - this.$options.validateOptions(dashedOptions, projectData); + this.$options.validateOptions(dashedOptions); } return this.canExecuteCommand(commandName, commandArguments); diff --git a/lib/options.ts b/lib/options.ts index f370b093ef..d352b62bf8 100644 --- a/lib/options.ts +++ b/lib/options.ts @@ -6,7 +6,7 @@ export class Options { private static NONDASHED_OPTION_REGEX = /(.+?)[-]([a-zA-Z])(.*)/; private optionsWhiteList = ["ui", "recursive", "reporter", "require", "timeout", "_", "$0"]; // These options shouldn't be validated - public argv: IYargArgv; + private yargsArgv: yargs.Argv; private globalOptions: IDictionary = { log: { type: OptionType.String, hasSensitiveValue: false }, verbose: { type: OptionType.Boolean, alias: "v", hasSensitiveValue: false }, @@ -19,24 +19,23 @@ export class Options { _: { type: OptionType.String, hasSensitiveValue: false } }; + public argv: yargs.Arguments; public options: IDictionary; - public setupOptions(projectData: IProjectData, commandSpecificDashedOptions?: IDictionary): void { + public setupOptions(commandSpecificDashedOptions?: IDictionary): void { if (commandSpecificDashedOptions) { _.extend(this.options, commandSpecificDashedOptions); this.setArgv(); } - if (this.argv.release && this.argv.hmr) { + this.argv.bundle = "webpack"; + + // Check if the user has explicitly provide --hmr and --release options from command line + if (this.yargsArgv.argv.release && this.yargsArgv.argv.hmr) { this.$errors.failWithoutHelp("The options --release and --hmr cannot be used simultaneously."); } - this.argv.bundle = "webpack"; - - const parsed = require("yargs-parser")(process.argv.slice(2), { 'boolean-negation': false }); - // --no-hmr -> hmr: false or --hmr false -> hmr: 'false' - const noHmr = parsed && (parsed.hmr === false || parsed.hmr === 'false'); - if (!noHmr) { + if (this.argv.hmr) { this.argv.hmr = !this.argv.release; } @@ -111,7 +110,7 @@ export class Options { pluginName: { type: OptionType.String, hasSensitiveValue: false }, includeTypeScriptDemo: { type: OptionType.String, hasSensitiveValue: false }, includeAngularDemo: { type: OptionType.String, hasSensitiveValue: false }, - hmr: { type: OptionType.Boolean, hasSensitiveValue: false }, + hmr: { type: OptionType.Boolean, hasSensitiveValue: false, default: true }, collection: { type: OptionType.String, alias: "c", hasSensitiveValue: false }, json: { type: OptionType.Boolean, hasSensitiveValue: false }, avd: { type: OptionType.String, hasSensitiveValue: true }, @@ -159,8 +158,8 @@ export class Options { return this.argv[optionName]; } - public validateOptions(commandSpecificDashedOptions?: IDictionary, projectData?: IProjectData): void { - this.setupOptions(projectData, commandSpecificDashedOptions); + public validateOptions(commandSpecificDashedOptions?: IDictionary): void { + this.setupOptions(commandSpecificDashedOptions); const parsed = Object.create(null); // DO NOT REMOVE { } as when they are missing and some of the option values is false, the each stops as it thinks we have set "return false". _.each(_.keys(this.argv), optionName => { @@ -251,12 +250,13 @@ export class Options { opts[this.getDashedOptionName(key)] = value; }); - this.argv = yargs(process.argv.slice(2)).options(opts).argv; + this.yargsArgv = yargs(process.argv.slice(2)); + this.argv = this.yargsArgv.options(opts).argv; // For backwards compatibility // Previously profileDir had a default option and calling `this.$options.profileDir` always returned valid result. // Now the profileDir should be used from $settingsService, but ensure the `this.$options.profileDir` returns the same value. - this.$settingsService.setSettings({ profileDir: this.argv.profileDir }); + this.$settingsService.setSettings({ profileDir: this.argv.profileDir }); this.argv.profileDir = this.argv["profile-dir"] = this.$settingsService.getProfileDir(); // if justlaunch is set, it takes precedence over the --watch flag and the default true value diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index 5c7e81c9fc..48fc5298fd 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -405,6 +405,21 @@ "@types/node": "*" } }, + "@types/yargs": { + "version": "13.0.0", + "resolved": "https://registry.npmjs.org/@types/yargs/-/yargs-13.0.0.tgz", + "integrity": "sha512-hY0o+kcz9M6kH32NUeb6VURghqMuCVkiUx+8Btsqhj4Hhov/hVGUx9DmBJeIkzlp1uAQK4wngQBCjqWdUUkFyA==", + "dev": true, + "requires": { + "@types/yargs-parser": "*" + } + }, + "@types/yargs-parser": { + "version": "13.0.0", + "resolved": "https://registry.npmjs.org/@types/yargs-parser/-/yargs-parser-13.0.0.tgz", + "integrity": "sha512-wBlsw+8n21e6eTd4yVv8YD/E3xq0O6nNnJIquutAsFGE7EyMKz7W6RNT6BRu1SmdgmlCZ9tb0X+j+D6HGr8pZw==", + "dev": true + }, "abbrev": { "version": "1.1.1", "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz", diff --git a/package.json b/package.json index aeac11d9b9..6c53726f4d 100644 --- a/package.json +++ b/package.json @@ -115,6 +115,7 @@ "@types/universal-analytics": "0.4.1", "@types/ws": "4.0.1", "@types/xml2js": "0.4.3", + "@types/yargs": "13.0.0", "chai": "4.0.2", "chai-as-promised": "7.0.0", "grunt": "1.0.1", diff --git a/test/options.ts b/test/options.ts index e45bdbcb9d..2ad685f0f0 100644 --- a/test/options.ts +++ b/test/options.ts @@ -286,6 +286,44 @@ describe("options", () => { assert.deepEqual(actualError, testCase.expectedError); }); }); + + describe("hmr option", () => { + const testCases = [{ + name: "should set hmr to true by default", + expectedHmrValue: true + }, { + name: "set set hmr to false when --no-hmr is provided", + args: ["--no-hmr"], + expectedHmrValue: false + }, { + name: "should set hmr to false when provided through dashed options from command", + commandSpecificDashedOptions: { + hmr: { type: OptionType.Boolean, default: false, hasSensitiveValue: false }, + }, + expectedHmrValue: false + }, { + name: "should set hmr to false by default when --release option is provided", + args: ["--release"], + expectedHmrValue: false + }, { + name: "should set hmr to false by default when --debug-brk option is provided", + args: ["--debugBrk"], + expectedHmrValue: false + }]; + + _.each(testCases, testCase => { + it(testCase.name, () => { + (testCase.args || []).forEach(arg => process.argv.push(arg)); + + const options: any = createOptions(testInjector); + options.setupOptions(testCase.commandSpecificDashedOptions); + + assert.equal(testCase.expectedHmrValue, options.argv.hmr); + + (testCase.args || []).forEach(arg => process.argv.pop()); + }); + }); + }); }); });