Skip to content

Commit d987e0b

Browse files
committed
refactor(commands): fail through the context and read options from it
`ctx.fail(message, { help })` replaces the commands' direct use of $errors, and signing and install options are read from the typed `ctx.options` instead of the global options object.
1 parent 693e34d commit d987e0b

42 files changed

Lines changed: 371 additions & 200 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎defining-commands.md‎

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -183,9 +183,9 @@ spelling _means_:
183183
stringOption({ alias: "p" })` steals `--path`'s shorthand. Restating an
184184
option's own shorthand (`path: stringOption({ alias: "p" })`) is fine.
185185

186-
The merge replaces the CLI-wide entry rather than patching it, so a
187-
redeclaration inherits nothing: restate the `alias` and `hasSensitiveValue` the
188-
global declaration carries if the command still wants them.
186+
A redeclaration that leaves `alias`, `default` or `hasSensitiveValue` unset
187+
keeps what the CLI-wide declaration carries for them, so `path: stringOption()`
188+
still answers to `-p` and stays out of the logs; set one only to change it.
189189

190190
### How validation behaves
191191

@@ -349,15 +349,22 @@ The run context
349349
- `ctx.injector` — this invocation's injector, a child of the one the command
350350
was registered against; see
351351
[Injection, and the first `await`](#injection-and-the-first-await).
352-
- `ctx.fail(message)` — fails the command with `message` and a usage help
353-
suggestion.
352+
- `ctx.fail(message, options?)` — fails the command with `message`, followed
353+
by a usage help suggestion unless `options.help` is `false`.
354354

355355
`run` may be synchronous or `async`; the CLI awaits the result and treats a
356356
rejection as a command failure.
357357

358358
### Failing a command
359359

360-
`ctx.fail(message)` is the idiomatic way to stop a command:
360+
`ctx.fail(message)` is the idiomatic way to stop a command. By default it
361+
follows the message with the usage help suggestion — the "Run `ns widget add
362+
--help`" line — which is what the user needs when they got the command line
363+
wrong: a missing argument, an unknown value, an invalid combination of options.
364+
365+
When the command line was fine and something else is not — the environment,
366+
the project, a file on disk — the help suggestion only gets in the way. Pass
367+
`{ help: false }` to print the message alone:
361368

362369
```ts
363370
defineCommand({
@@ -369,17 +376,22 @@ defineCommand({
369376
ctx.fail("--output is required.");
370377
}
371378

379+
if (fs.existsSync(ctx.options.output)) {
380+
ctx.fail(`${ctx.options.output} already exists.`, { help: false });
381+
}
382+
372383
/* ... */
373384
},
374385
});
375386
```
376387

377-
It is available on the `canExecute` context as well, and it returns `never`, so
378-
it can end a branch without a `return`. The message must be a non-empty string.
388+
It is available on the `setup` and `canExecute` contexts as well, and it
389+
returns `never`, so it can end a branch without a `return`. The message must be
390+
a non-empty string, and `options`, when given, a plain object. The two forms
391+
map onto the `errors` service's `failWithHelp` and `fail`.
379392

380-
Throwing is equivalent and keeps working — `ctx.fail` is sugar over the
381-
`errors` service's `failWithHelp`, which is what adds the "Run `ns widget add
382-
--help`" line. Throw when you already have an `Error` to propagate; call
393+
Throwing keeps working too: an error thrown from a handler propagates
394+
unchanged. Throw when you already have an `Error` to propagate; call
383395
`ctx.fail` when you are writing the message.
384396

385397
Injection, and the first `await`

‎lib/commands/add-platform.ts‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import {
44
IPlatformValidationService,
55
} from "../declarations";
66
import { IProjectData } from "../definitions/project";
7-
import { IErrors } from "../common/declarations";
87
import {
98
Command,
109
CommandOptionsSchema,
@@ -23,7 +22,6 @@ export class AddPlatformCommand extends Command({
2322
options: addPlatformCommandOptions,
2423
arguments: "any",
2524
}) {
26-
private $errors = inject<IErrors>("errors");
2725
private $platformCommandHelper = inject<IPlatformCommandHelper>(
2826
"platformCommandHelper",
2927
);
@@ -40,7 +38,7 @@ export class AddPlatformCommand extends Command({
4038
public async canExecute(): Promise<boolean> {
4139
const args = this.args;
4240
if (!args || args.length === 0) {
43-
this.$errors.failWithHelp(
41+
this.context.fail(
4442
"No platform specified. Please specify a platform to add.",
4543
);
4644
}
@@ -55,8 +53,9 @@ export class AddPlatformCommand extends Command({
5553
this.$projectData,
5654
)
5755
) {
58-
this.$errors.fail(
56+
this.context.fail(
5957
`Applications for platform ${arg} cannot be built on this OS`,
58+
{ help: false },
6059
);
6160
}
6261

‎lib/commands/apple-login.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { IErrors } from "../common/declarations";
21
import { defineCommand } from "../common/define-command";
32
import { inject } from "../common/di";
43
import { IApplePortalSessionService } from "../services/apple-portal/definitions";
@@ -11,7 +10,6 @@ export const appleLoginCommandDefinition = defineCommand({
1110
const $applePortalSessionService = inject<IApplePortalSessionService>(
1211
"applePortalSessionService",
1312
);
14-
const $errors = inject<IErrors>("errors");
1513
const $logger = inject<ILogger>("logger");
1614
const $prompter = inject<IPrompter>("prompter");
1715

@@ -32,8 +30,9 @@ export const appleLoginCommandDefinition = defineCommand({
3230
password,
3331
});
3432
if (!user.areCredentialsValid) {
35-
$errors.fail(
33+
context.fail(
3634
`Invalid username and password combination. Used '${username}' as the username.`,
35+
{ help: false },
3736
);
3837
}
3938

‎lib/commands/appstore-list.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { IErrors } from "../common/declarations";
21
import {
32
Command,
43
CommandOptionsSchema,
@@ -31,7 +30,6 @@ export class ListiOSAppsCommand extends Command({
3130
private $devicePlatformsConstants = inject<Mobile.IDevicePlatformsConstants>(
3231
"devicePlatformsConstants",
3332
);
34-
private $errors = inject<IErrors>("errors");
3533
private $logger = inject<ILogger>("logger");
3634
private $platformValidationService = inject<IPlatformValidationService>(
3735
"platformValidationService",
@@ -51,8 +49,9 @@ export class ListiOSAppsCommand extends Command({
5149
this.$projectData,
5250
)
5351
) {
54-
this.$errors.fail(
52+
this.context.fail(
5553
`Applications for platform ${this.$devicePlatformsConstants.iOS} can not be built on this OS`,
54+
{ help: false },
5655
);
5756
}
5857

@@ -76,8 +75,9 @@ export class ListiOSAppsCommand extends Command({
7675
},
7776
);
7877
if (!user.areCredentialsValid) {
79-
this.$errors.fail(
78+
this.context.fail(
8079
`Invalid username and password combination. Used '${username}' as the username.`,
80+
{ help: false },
8181
);
8282
}
8383

‎lib/commands/appstore-upload.ts‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import * as path from "path";
2-
import { IErrors, IHostInfo } from "../common/declarations";
2+
import { IHostInfo } from "../common/declarations";
33
import {
44
booleanOption,
55
Command,
@@ -41,7 +41,6 @@ export class PublishIOSCommand extends Command({
4141
private $devicePlatformsConstants = inject<Mobile.IDevicePlatformsConstants>(
4242
"devicePlatformsConstants",
4343
);
44-
private $errors = inject<IErrors>("errors");
4544
private $hostInfo = inject<IHostInfo>("hostInfo");
4645
private $itmsTransporterService = inject<IITMSTransporterService>(
4746
"itmsTransporterService",
@@ -61,7 +60,9 @@ export class PublishIOSCommand extends Command({
6160

6261
public canExecute(): boolean {
6362
if (!this.$hostInfo.isDarwin) {
64-
this.$errors.fail("iOS publishing is only available on macOS.");
63+
this.context.fail("iOS publishing is only available on macOS.", {
64+
help: false,
65+
});
6566
}
6667

6768
if (
@@ -70,8 +71,9 @@ export class PublishIOSCommand extends Command({
7071
this.$projectData,
7172
)
7273
) {
73-
this.$errors.fail(
74+
this.context.fail(
7475
`Applications for platform ${this.$devicePlatformsConstants.iOS} can not be built on this OS`,
76+
{ help: false },
7577
);
7678
}
7779

@@ -135,8 +137,9 @@ export class PublishIOSCommand extends Command({
135137
},
136138
);
137139
if (!user.areCredentialsValid) {
138-
this.$errors.fail(
140+
this.context.fail(
139141
`Invalid username and password combination. Used '${username}' as the username.`,
142+
{ help: false },
140143
);
141144
}
142145

‎lib/commands/build.ts‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@ import {
22
ANDROID_RELEASE_BUILD_ERROR_MESSAGE,
33
AndroidAppBundleMessages,
44
} from "../constants";
5-
import { canExecuteCommandBase, validatePlatformOptions } from "./command-base";
5+
import {
6+
canExecuteCommandBase,
7+
platformSigningOptions,
8+
validatePlatformOptions,
9+
} from "./command-base";
610
import { hasValidAndroidSigning } from "../common/helpers";
711
import {
812
IAndroidBundleValidatorHelper,
@@ -12,7 +16,6 @@ import {
1216
import { IBuildController, IBuildDataService } from "../definitions/build";
1317
import { IMigrateController } from "../definitions/migrate";
1418
import { IProjectData } from "../definitions/project";
15-
import { IErrors } from "../common/declarations";
1619
import {
1720
booleanOption,
1821
CommandName,
@@ -29,6 +32,7 @@ import { inject } from "../common/di";
2932
type BuildPlatform = "iOS" | "Android" | "visionOS";
3033

3134
const buildCommandOptions = {
35+
...platformSigningOptions,
3236
watch: booleanOption({ default: false }),
3337
hmr: booleanOption({ default: false }),
3438
force: booleanOption(),
@@ -52,7 +56,6 @@ const defineBuildCommand = <const TName extends CommandName>(
5256
async canExecute(context): Promise<boolean> {
5357
const $devicePlatformsConstants =
5458
inject<Mobile.IDevicePlatformsConstants>("devicePlatformsConstants");
55-
const $errors = inject<IErrors>("errors");
5659
const $migrateController =
5760
inject<IMigrateController>("migrateController");
5861
const $platformValidationService = inject<IPlatformValidationService>(
@@ -82,8 +85,9 @@ const defineBuildCommand = <const TName extends CommandName>(
8285
$projectData,
8386
)
8487
) {
85-
$errors.fail(
88+
context.fail(
8689
`Applications for platform ${platform} can not be built on this OS`,
90+
{ help: false },
8791
);
8892
}
8993

@@ -96,7 +100,7 @@ const defineBuildCommand = <const TName extends CommandName>(
96100
context.options.release &&
97101
!hasValidAndroidSigning(context.options)
98102
) {
99-
$errors.failWithHelp(ANDROID_RELEASE_BUILD_ERROR_MESSAGE);
103+
context.fail(ANDROID_RELEASE_BUILD_ERROR_MESSAGE);
100104
}
101105

102106
return validatePlatformOptions(context, platform);

‎lib/commands/command-base.ts‎

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,31 @@ import {
55
ICanExecuteCommandOptions,
66
INotConfiguredEnvOptions,
77
} from "../common/definitions/commands";
8-
import { ArgumentSpec, CommandContext } from "../common/define-command";
8+
import {
9+
ArgumentSpec,
10+
CommandContext,
11+
CommandOptionsSchema,
12+
objectOption,
13+
} from "../common/define-command";
914
import { Injector } from "../common/di";
1015

16+
/**
17+
* The CLI-wide signing options `validatePlatformOptions` checks. A command
18+
* that validates them spreads this into its own schema.
19+
*/
20+
export const platformSigningOptions = {
21+
provision: objectOption(),
22+
teamId: objectOption(),
23+
} satisfies CommandOptionsSchema;
24+
1125
/** The part of a command context these helpers read. */
1226
type PlatformCommandContext = Pick<CommandContext<any>, "injector">;
1327

28+
type PlatformSigningContext = Pick<
29+
CommandContext<typeof platformSigningOptions>,
30+
"injector" | "options"
31+
>;
32+
1433
/**
1534
* The declarative form of `$platformCommandParameter`. Initializing the
1635
* project data is what makes the platform check possible, so it stays part of
@@ -38,17 +57,16 @@ export const platformArgument: ArgumentSpec<any> = {
3857
};
3958

4059
export function validatePlatformOptions(
41-
context: PlatformCommandContext,
60+
context: PlatformSigningContext,
4261
platform: string,
4362
): Promise<boolean> {
44-
const $options = context.injector.get<IOptions>("options");
4563
const $projectData = context.injector.get<IProjectData>("projectData");
4664

4765
return context.injector
4866
.get<IPlatformValidationService>("platformValidationService")
4967
.validateOptions(
50-
$options.provision,
51-
$options.teamId,
68+
context.options.provision,
69+
context.options.teamId,
5270
$projectData,
5371
platform,
5472
);
@@ -82,9 +100,19 @@ function hasUsableEnvironment(
82100
);
83101
}
84102

85-
export async function canExecuteCommandBase(
103+
export function canExecuteCommandBase(
104+
context: PlatformSigningContext,
105+
platform: string,
106+
options: ICanExecuteCommandOptions & { validateOptions: true },
107+
): Promise<boolean>;
108+
export function canExecuteCommandBase(
86109
context: PlatformCommandContext,
87110
platform: string,
111+
options?: ICanExecuteCommandOptions & { validateOptions?: false },
112+
): Promise<boolean>;
113+
export async function canExecuteCommandBase(
114+
context: PlatformCommandContext | PlatformSigningContext,
115+
platform: string,
88116
options: ICanExecuteCommandOptions = {},
89117
): Promise<boolean> {
90118
const validatePlatformOutput = await validatePlatformBase(
@@ -96,7 +124,10 @@ export async function canExecuteCommandBase(
96124
let result = canExecute;
97125

98126
if (canExecute && options.validateOptions) {
99-
result = await validatePlatformOptions(context, platform);
127+
result = await validatePlatformOptions(
128+
<PlatformSigningContext>context,
129+
platform,
130+
);
100131
}
101132

102133
return result;

0 commit comments

Comments
 (0)