Merge pull request #4856 from NativeScript/fatme/fix-argv

fix: respect correctly `hmr: false` from command's dashed options

Author: Fatme

lib/common/definitions/yargs.d.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<boolean | ICanExecuteCommandOutput> {
 		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);

lib/common/services/commands-service.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<IDashedOption> = {
 		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<IDashedOption>;
 
-	public setupOptions(projectData: IProjectData, commandSpecificDashedOptions?: IDictionary<IDashedOption>): void {
+	public setupOptions(commandSpecificDashedOptions?: IDictionary<IDashedOption>): 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<IDashedOption>, projectData?: IProjectData): void {
-		this.setupOptions(projectData, commandSpecificDashedOptions);
+	public validateOptions(commandSpecificDashedOptions?: IDictionary<IDashedOption>): 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(<any>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: <string>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

lib/options.ts

@@ -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",

npm-shrinkwrap.json

@@ -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());
+				});
+			});
+		});
 	});
 });