Skip to content

Commit 188e72d

Browse files
fix: cloud run command does not respect useLegacyWorkflow flag
Currently when you run `tns cloud run ...` the command executes local prepare without taking into account the value of `useLegacyWorkflow` value in nsconfig.json file. The problem is that cloud commands have specific `--` options, which are set as `dashedOptions` on command level. During command execution the following actions happen: 1. CLI starts its execution and via `$injector` constructs new instance of `$options`. 2. In the constructor of `$options` we call `this.setArgv()`, which calls `yargs` package and constructs values based on the user input of `--` options. 3. `$injector` resolves `$commandsService` and constructs new object from it. 4. In the constructor of `CommandsService`, we call `$options.setupOptions` and pass the current projectData. `setupOptions` takes into account the projectData and again calls `this.setArgv()`. After that it overwrites some of the values set by yargs based on the projectData. 5. `CommandsService` starts execution of the command by resolving new instance of it and calling its own `tryExecuteCommandAction` method. 6. `tryExecuteCommandAction` method internally calls `this.$options.validateOptions` by passing the command specific options(`dashedOptions`). The `validateOptions` method modifies the internal structure based on which yargs constructs the passed options and calls `this.setArgv()` again. At this point we overwrite the internal values of `$options` based on the user's input and taking into account the command's `dashedOptions`. However, this way we've overwritten the values set by `setupOptions` in step 4 which are based on the current project dir. To fix the behavior, make `setupOptions` private in `$options` and call it internally in the `validateOptions` method. So the new workflow is: 1. CLI starts its execution and via `$injector` constructs new instance of `$options`. 2. In the constructor of `$options` we call `this.setArgv()`, which calls `yargs` package and constructs values based on the user input of `--` options. 3. `$injector` resolves `$commandsService` and constructs new object from it. 4. In the constructor of `CommandsService`, we call `this.setupOptions` and pass the current projectData. `setupOptions` takes into account the projectData and again calls `this.setArgv()`. After that it overwrites some of the values set by yargs based on the projectData. 5. `CommandsService` starts execution of the command by resolving new instance of it and calling its own `tryExecuteCommandAction` method. 6. `tryExecuteCommandAction` method internally calls `this.$options.validateOptions` by passing the command specific options(`dashedOptions`). The `validateOptions` method calls `this.setupOptions` and pass the current projectData and command specific options. After that it calls `this.setArgv()` and the internal structure that contains the passed options is overwritten based on the command specific options. After that the method overwrites some of the values based on the passed projectData.
1 parent e173c2d commit 188e72d

File tree

4 files changed

+22
-22
lines changed

4 files changed

+22
-22
lines changed

lib/common/services/commands-service.ts

+11-11
Original file line numberDiff line numberDiff line change
@@ -29,15 +29,6 @@ export class CommandsService implements ICommandsService {
2929
private $extensibilityService: IExtensibilityService,
3030
private $optionsTracker: IOptionsTracker,
3131
private $projectDataService: IProjectDataService) {
32-
let projectData = null;
33-
try {
34-
projectData = this.$projectDataService.getProjectData();
35-
} catch (err) {
36-
this.$logger.trace(`Error while trying to get project data. More info: ${err}`);
37-
}
38-
39-
this.$options.setupOptions(projectData);
40-
this.$options.printMessagesForDeprecatedOptions(this.$logger);
4132
}
4233

4334
public allCommands(opts: { includeDevCommands: boolean }): string[] {
@@ -114,8 +105,17 @@ export class CommandsService implements ICommandsService {
114105

115106
private async tryExecuteCommandAction(commandName: string, commandArguments: string[]): Promise<boolean | ICanExecuteCommandOutput> {
116107
const command = this.$injector.resolveCommand(commandName);
117-
if (!command || (command && !command.isHierarchicalCommand)) {
118-
this.$options.validateOptions(command ? command.dashedOptions : null);
108+
if (!command || !command.isHierarchicalCommand) {
109+
let projectData = null;
110+
try {
111+
projectData = this.$projectDataService.getProjectData();
112+
} catch (err) {
113+
this.$logger.trace(`Error while trying to get project data. More info: ${err}`);
114+
}
115+
116+
const dashedOptions = command ? command.dashedOptions : null;
117+
this.$options.validateOptions(dashedOptions, projectData);
118+
this.$options.printMessagesForDeprecatedOptions(this.$logger);
119119
}
120120

121121
return this.canExecuteCommand(commandName, commandArguments);

lib/declarations.d.ts

+1-2
Original file line numberDiff line numberDiff line change
@@ -503,7 +503,7 @@ interface IAndroidBundleOptions {
503503

504504
interface IOptions extends IRelease, IDeviceIdentifier, IJustLaunch, IAvd, IAvailableDevices, IProfileDir, IHasEmulatorOption, IBundleString, IPlatformTemplate, IHasEmulatorOption, IClean, IProvision, ITeamIdentifier, IAndroidReleaseOptions, IAndroidBundleOptions, INpmInstallConfigurationOptions, IPort, IEnvOptions, IPluginSeedOptions, IGenerateOptions {
505505
argv: IYargArgv;
506-
validateOptions(commandSpecificDashedOptions?: IDictionary<IDashedOption>): void;
506+
validateOptions(commandSpecificDashedOptions?: IDictionary<IDashedOption>, projectData?: IProjectData): void;
507507
options: IDictionary<IDashedOption>;
508508
shorthands: string[];
509509
/**
@@ -573,7 +573,6 @@ interface IOptions extends IRelease, IDeviceIdentifier, IJustLaunch, IAvd, IAvai
573573
performance: Object;
574574
cleanupLogFile: string;
575575
workflow: any;
576-
setupOptions(projectData: IProjectData): void;
577576
printMessagesForDeprecatedOptions(logger: ILogger): void;
578577
}
579578

lib/options.ts

+8-7
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,12 @@ export class Options {
2121

2222
public options: IDictionary<IDashedOption>;
2323

24-
public setupOptions(projectData: IProjectData): void {
24+
public setupOptions(projectData: IProjectData, commandSpecificDashedOptions?: IDictionary<IDashedOption>): void {
25+
if (commandSpecificDashedOptions) {
26+
_.extend(this.options, commandSpecificDashedOptions);
27+
this.setArgv();
28+
}
29+
2530
if (this.argv.release && this.argv.hmr) {
2631
this.$errors.failWithoutHelp("The options --release and --hmr cannot be used simultaneously.");
2732
}
@@ -173,12 +178,8 @@ export class Options {
173178
return this.argv[optionName];
174179
}
175180

176-
public validateOptions(commandSpecificDashedOptions?: IDictionary<IDashedOption>): void {
177-
if (commandSpecificDashedOptions) {
178-
_.extend(this.options, commandSpecificDashedOptions);
179-
this.setArgv();
180-
}
181-
181+
public validateOptions(commandSpecificDashedOptions?: IDictionary<IDashedOption>, projectData?: IProjectData): void {
182+
this.setupOptions(projectData, commandSpecificDashedOptions);
182183
const parsed = Object.create(null);
183184
// 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".
184185
_.each(_.keys(this.argv), optionName => {

test/options.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ describe("options", () => {
325325
it(`should pass correctly when ${testCase.name} and useLegacyWorkflow is ${useLegacyWorkflow}`, () => {
326326
(testCase.args || []).forEach(arg => process.argv.push(arg));
327327

328-
const options = createOptions(testInjector);
328+
const options: any = createOptions(testInjector);
329329
const projectData = <IProjectData>{ useLegacyWorkflow };
330330
options.setupOptions(projectData);
331331

@@ -359,7 +359,7 @@ describe("options", () => {
359359
errors.failWithoutHelp = (error: string) => actualError = error;
360360
(testCase.args || []).forEach(arg => process.argv.push(arg));
361361

362-
const options = createOptions(testInjector);
362+
const options: any = createOptions(testInjector);
363363
options.setupOptions(null);
364364

365365
(testCase.args || []).forEach(arg => process.argv.pop());

0 commit comments

Comments
 (0)