Skip to content

Commit b41cadd

Browse files
committed
fix(commands): close the review gaps in the command API
- A bare class in `providers` provides itself, as in Angular; the container expands it to `{ provide: Cls, useClass: Cls }` and the definition rejects anything that is neither a class nor a `provide` object. - A COMMAND_PRECONDITIONS entry is a function or a list of functions, checked with the command's name; the list form is what lets a command keep the scope's preconditions next to its own. - An argument's `validate` runs inside the invocation's injection context. - `runCommand` and `canExecuteCommand` take `{ injector }` for a definition run as given, defaulting to the caller's injection context, then the root; a registered name rejects it. - Option priming happens inside the guarded block, so a throwing `validateOptions` leaves no command entry behind. - `getInjector()` stays as a deprecated alias of `getRootInjector()`, and the compat tests carry their original lines again. - `provideProject` lives under lib/contracts and is exported from nativescript/contracts with `CommandReference` and `CommandDispatchOptions`. - The guide follows the API on every point above.
1 parent d7de648 commit b41cadd

20 files changed

Lines changed: 564 additions & 129 deletions

‎defining-commands.md‎

Lines changed: 105 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ Validation happens where you can see it
5757

5858
A definition is checked at the moment `defineCommand` is called, not when the
5959
command eventually runs. A misspelled field, a missing `run`, an option
60-
declared with something other than the four helpers, an `arguments` value
60+
declared with something other than the five helpers, an `arguments` value
6161
outside `"none" | "any"` — each throws immediately, naming the command and the
6262
accepted form. The class form's meta is checked the same way at the
6363
`Command({ ... })` call, which also rejects handlers passed there; only a
@@ -66,10 +66,14 @@ missing `run` method waits until the definition is first read:
6666
```
6767
Invalid command definition for 'widget|add': unknown field(s) 'handler'; a
6868
definition accepts name, description, options, arguments, allowUnknownOptions,
69-
canExecute, disableAnalytics, enableHooks, setup, run, postRun. Accepted form:
70-
defineCommand({ name: "widget|add", run(ctx) { ... } }) — with the optional
71-
fields description, options, arguments, allowUnknownOptions, setup, canExecute,
72-
postRun, disableAnalytics and enableHooks.
69+
canExecute, disableAnalytics, enableHooks, providers, setup, run, shortcuts,
70+
postRun. Accepted form: defineCommand({ name: "widget|add", run(ctx) { ... } })
71+
— with the optional fields description, options, arguments,
72+
allowUnknownOptions, providers, setup, canExecute, shortcuts, postRun,
73+
disableAnalytics and enableHooks. Or the class form, class WidgetAdd extends
74+
Command({ name: "widget|add" }) { run() { ... } }, which declares the same
75+
fields except the handlers and implements run, and optionally canExecute,
76+
postRun and shortcuts, as methods.
7377
```
7478

7579
Names and the command hierarchy
@@ -98,7 +102,7 @@ Options
98102
-------
99103

100104
`options` is a schema keyed by the long option name — `output` is passed as
101-
`--output`. Declare each entry with one of the four helpers, which fix the
105+
`--output`. Declare each entry with one of the five helpers, which fix the
102106
value type:
103107

104108
| Helper | Declared with `default` | Declared without |
@@ -107,6 +111,10 @@ value type:
107111
| `stringOption` | `string` | `string \| undefined` |
108112
| `numberOption` | `number` | `number \| undefined` |
109113
| `arrayOption` | `string[]` | `string[] \| undefined` |
114+
| `objectOption` | `any` | `any` |
115+
116+
`objectOption` is for flags the parser nests, such as `--env.production`, and
117+
its value is untyped.
110118

111119
The two columns are the whole story of the option types: a flag the user did
112120
not pass is absent at runtime, so only a `default` makes the value on
@@ -211,8 +219,8 @@ one that users pass is a warning today and a failure later, never a silent
211219

212220
A command that forwards its command line to a separately installed CLI cannot
213221
know which flags are legitimate, so validating them here would reject the other
214-
CLI's own options. `allowUnknownOptions: true` turns the check off for that
215-
command:
222+
CLI's own options. `allowUnknownOptions: true` lets unknown flags through for
223+
that command:
216224

217225
```ts
218226
defineCommand({
@@ -225,10 +233,11 @@ defineCommand({
225233
});
226234
```
227235

228-
It maps onto `skipOptionsValidation` on the compiled command, which means the
229-
CLI never re-primes its parser for this command at all. A command-specific
230-
option therefore never reaches `ctx.options` under this flag — only options the
231-
CLI already knows globally carry values. Reach for it only when forwarding.
236+
An unknown flag is neither warned about nor rejected. Everything else about
237+
validation still applies: the parser is re-primed with the command's declared
238+
options, so `ctx.options.disableNpmInstall` above carries its value, and a
239+
known option passed with the wrong shape is still reported. Reach for it only
240+
when forwarding.
232241

233242
Positional arguments
234243
--------------------
@@ -278,12 +287,13 @@ A spec accepts:
278287
- `errorMessage` — replaces `Missing required argument '<name>'.` when the
279288
argument is required and absent.
280289
- `validate(value, ctx)` — run per value, `ctx` being the same context `run`
281-
receives. Return `true` to accept; return `false` for a default message, or
282-
return the message itself as a string. It may be `async`.
290+
receives, inside the invocation's injection context. Return `true` to
291+
accept; return `false` for a default message, or return the message itself
292+
as a string. It may be `async`.
283293

284-
Enforcement happens before `canExecute`, in this order: missing required
285-
arguments (every missing one is named at once), then too many arguments, then
286-
each `validate`.
294+
Enforcement happens after `setup` and before `canExecute`, in this order:
295+
missing required arguments (every missing one is named at once), then too many
296+
arguments, then each `validate`.
287297

288298
### Matching is strictly positional
289299

@@ -324,9 +334,9 @@ definition that leaves `arguments` at `"none"` still rejects stray positional
324334
arguments even when it supplies a `canExecute`, and a `canExecute` that only
325335
inspects options cannot accidentally widen what the command accepts.
326336

327-
`canExecute` receives a context of the same shape as `run`'s — the same
328-
`args`, the same declared options and the same `fail` — built freshly for the
329-
call, and returns a boolean (or a promise of one). Returning `false` aborts the
337+
`canExecute` receives the same context object `run` does: one context is built
338+
when the invocation opens and every stage of it shares that object. It
339+
returns a boolean (or a promise of one). Returning `false` aborts the
330340
command and prints a bare help suggestion; `ctx.fail(message)` aborts it with
331341
your own message, which is usually the friendlier choice.
332342

@@ -397,12 +407,12 @@ unchanged. Throw when you already have an `Error` to propagate; call
397407
Injection, and the first `await`
398408
--------------------------------
399409

400-
`setup`, `canExecute`, `run` and `postRun` each start inside a
401-
dependency-injection context, so `inject()` works directly:
410+
`setup`, `canExecute`, `run`, `postRun` and `shortcuts`, an argument's
411+
`validate` and each precondition start inside a dependency-injection context,
412+
so `inject()` works directly:
402413

403414
```ts
404-
import { defineCommand, inject } from "nativescript/contracts";
405-
import { DoctorService } from "nativescript/contracts";
415+
import { defineCommand, inject, DoctorService } from "nativescript/contracts";
406416

407417
export default defineCommand({
408418
name: "widget|check",
@@ -460,7 +470,10 @@ export default defineCommand({
460470

461471
A definition may carry `providers`, added to each invocation's own injector
462472
next to the context, so a factory or class among them can inject the
463-
invocation and is built once per invocation. One token in that list is
473+
invocation and is built once per invocation. An entry is an object with a
474+
`provide` token or a bare class, which stands for
475+
`{ provide: Cls, useClass: Cls }` as in Angular; anything else is rejected
476+
when the definition is defined. One token in that list is
464477
special: `COMMAND_PRECONDITIONS` is a multi token, and every
465478
`{ provide: COMMAND_PRECONDITIONS, multi: true, useValue: check }` contributes
466479
a **precondition** — a check on the environment the command runs in, as
@@ -470,7 +483,35 @@ arguments policy, inside the injection context, and a throw fails the
470483
invocation. That fixed order is the point: being outside a project is what a
471484
bad invocation reports first.
472485

473-
The precondition every project command declares comes from a helper:
486+
Each entry is a function, or a list of functions. Anything else fails the
487+
first invocation with `Command '<name>': COMMAND_PRECONDITIONS entry #n is not
488+
a function or a list of functions.`
489+
490+
Multi providers are per injector level, as in Angular. A command that declares
491+
any precondition of its own, `provideProject()` included, replaces the
492+
preconditions provided by the scope it was registered in; the two lists do not
493+
merge. To keep the scope's preconditions, contribute the parent's list as one
494+
more entry:
495+
496+
```ts
497+
import { COMMAND_PRECONDITIONS, inject } from "nativescript/contracts";
498+
499+
providers: [
500+
{
501+
provide: COMMAND_PRECONDITIONS,
502+
multi: true,
503+
useFactory: () =>
504+
inject(COMMAND_PRECONDITIONS, { skipSelf: true, optional: true }) || [],
505+
},
506+
provideProject(),
507+
],
508+
```
509+
510+
A built-in imports `inject` from `"../common/di"` instead.
511+
512+
The precondition every project command declares comes from a helper. The
513+
built-in sample below imports it relatively; a plugin imports `provideProject`
514+
from `"nativescript/contracts"`.
474515

475516
```ts
476517
import { provideProject } from "../command-base";
@@ -487,11 +528,13 @@ export default defineCommand({
487528
`provideProject()` resolves the project from `--path` or the working
488529
directory and fails the invocation with the usual "no project found" error
489530
when there is none. A command that does not declare it — `doctor`, `create`,
490-
the `device` family — pays nothing, and a command that needs the project only
491-
when it is there, like `clean`, resolves it itself behind its own check. Never
492-
call `initializeProjectData()` from a command; declare the provider. A plugin
493-
adds its own preconditions the same way, with its own helper returning a
494-
multi provider for the token.
531+
the `device` family — pays nothing. In such a command `inject(ProjectData)`
532+
returns the process-wide object uninitialised, without any error. A command
533+
that wants the project only when there is one, like `clean` or
534+
`device put-file`, resolves `ProjectData` behind its own check and calls
535+
`initializeProjectData()` there. A command that always needs the project
536+
declares the provider. A plugin adds its own preconditions the same way, with
537+
its own helper returning a multi provider for the token.
495538

496539
The injection context is synchronous, so the `inject()` calls belong **above
497540
the first `await`** — see [Injection, and the first
@@ -525,8 +568,8 @@ Sharing is either of two things, and neither of them is a bag:
525568

526569
### `setup`, when a command has one
527570

528-
`setup(ctx)` runs once per invocation, before `canExecute`, and its return
529-
value is handed to `canExecute`, `run` and `postRun` as their second argument.
571+
`setup(ctx)` runs once per invocation, after the preconditions and before the
572+
arguments policy and `canExecute`, and its return value is handed to `canExecute`, `run` and `postRun` as their second argument.
530573
"Once per invocation" means once across the three together — whichever the CLI
531574
reaches first triggers it, and the rest reuse the value.
532575

@@ -582,7 +625,13 @@ by `defineCommand`, and that definition is the only thing the CLI ever
582625
executes.
583626

584627
```ts
585-
import { Command, inject, stringOption } from "nativescript/contracts";
628+
import {
629+
Command,
630+
inject,
631+
ProjectData,
632+
provideProject,
633+
stringOption,
634+
} from "nativescript/contracts";
586635

587636
export class PlatformCleanCommand extends Command({
588637
name: "platform|clean",
@@ -607,7 +656,8 @@ export class PlatformCleanCommand extends Command({
607656
```
608657

609658
`meta` is the definition minus its handlers: `name`, `description`, `options`,
610-
`arguments`, `allowUnknownOptions`, `disableAnalytics` and `enableHooks`. The
659+
`arguments`, `allowUnknownOptions`, `disableAnalytics`, `enableHooks` and
660+
`providers`. The
611661
handlers are methods instead — `run` is required, and `canExecute`, `postRun`
612662
and `shortcuts` are optional, each with the same meaning and the same ordering
613663
as the fields of the same name. The result type is inferred from `run`, and
@@ -899,8 +949,9 @@ action: (ctx) => ctx.injector.get(CommandsService).runCommand("open|ios"),
899949
The command gets what a typed command line gives it, in the same order: its
900950
declared options are primed into the parser — so `ctx.options` holds this
901951
command's values and its declared defaults rather than the outer command
902-
line's — then the `arguments` policy, then `canExecute`, then `run`,
903-
`postRun`, and the command's hooks.
952+
line's — then its preconditions, as the invocation opens, then `setup`, then
953+
the `arguments` policy, then `canExecute`, then `run`, `postRun`, and the
954+
command's hooks.
904955

905956
Two things differ, both because the caller is a process that has to keep
906957
running afterwards:
@@ -946,6 +997,20 @@ given, whether or not it is registered, so `runCommand(prepareCommandDefinition)
946997
runs exactly what you hold and cannot go stale the way a string can. Its first
947998
name still identifies it for hooks and reporting.
948999

1000+
A definition run as given is compiled against an injector chosen at the call,
1001+
the way Angular's `createComponent` takes one: the `injector` in the options
1002+
bag when one is passed, otherwise the injection context the call is made from,
1003+
otherwise the CLI's root. So a definition dispatched from inside an
1004+
extension's command lands under the extension's scope, where `registerCommand`
1005+
would have placed it, and a caller holding a scope of its own names it:
1006+
1007+
```ts
1008+
await commandsService.runCommand(definition, args, { injector });
1009+
```
1010+
1011+
A registered command keeps the scope it was registered under, and passing
1012+
`injector` together with a name throws.
1013+
9491014
`CommandsService.canExecuteCommand(command, args)` asks a registered command
9501015
whether it *could* run, without running it:
9511016

@@ -1023,7 +1088,7 @@ mapping is:
10231088
| `arguments`, `canExecute` | `canExecute`: policy enforced, then the refinement |
10241089
| `setup` | — run inside `canExecute`/`execute`, memoised |
10251090
| `postRun` | `postCommandAction`, with `run`'s return value |
1026-
| `allowUnknownOptions` | `skipOptionsValidation` |
1091+
| `allowUnknownOptions` | `allowUnknownOptions` |
10271092
| — | `allowedParameters`, always `[]` |
10281093
| `disableAnalytics`, `enableHooks` | passed through unchanged |
10291094

@@ -1034,7 +1099,7 @@ empty, which is why declared `arguments` are matched positionally rather than
10341099
by the `ICommandParameter` scan.
10351100

10361101
Existing command classes need no migration. Reach for a definition when a
1037-
command is mostly "parse these flags and do this"; a class still makes sense
1038-
when a command needs constructor-injected collaborators shared across several
1102+
command is mostly "parse these flags and do this"; a legacy `ICommand` class
1103+
still makes sense when a command needs constructor-injected collaborators shared across several
10391104
methods, or `ICommandParameter` validators whose claim-any-argument matching it
10401105
actually depends on.

‎lib/commands/command-base.ts‎

Lines changed: 4 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { IProjectData, IValidatePlatformOutput } from "../definitions/project";
1+
import { IValidatePlatformOutput } from "../definitions/project";
22
import { IOptions, IPlatformValidationService } from "../declarations";
33
import { IPlatformsDataService } from "../definitions/platform";
44
import {
@@ -11,13 +11,10 @@ import {
1111
CommandOptionsSchema,
1212
objectOption,
1313
} from "../common/define-command";
14-
import { Injector, inject } from "../common/di";
15-
import type { Provider } from "../common/di/providers";
14+
import { Injector } from "../common/di";
1615
import { ProjectData } from "../contracts/project-data";
17-
import {
18-
COMMAND_PRECONDITIONS,
19-
CommandPrecondition,
20-
} from "../common/contracts/command-preconditions";
16+
17+
export { provideProject } from "../contracts/provide-project";
2118

2219
/**
2320
* The CLI-wide signing options `validatePlatformOptions` checks. A command
@@ -36,27 +33,6 @@ type PlatformSigningContext = Pick<
3633
"injector" | "options"
3734
>;
3835

39-
/**
40-
* Declares that a command runs inside a project: the project the command line
41-
* names, through `--path` or the working directory, is resolved before the
42-
* command's setup and arguments policy, and its absence fails the invocation
43-
* with the "no project found" error. `inject(ProjectData)` then reads it.
44-
*/
45-
export function provideProject(): Provider {
46-
return {
47-
provide: COMMAND_PRECONDITIONS,
48-
multi: true,
49-
useValue: requireProject,
50-
};
51-
}
52-
53-
const requireProject: CommandPrecondition = () => {
54-
const projectData = inject<IProjectData>("projectData");
55-
if (typeof projectData.initializeProjectData === "function") {
56-
projectData.initializeProjectData();
57-
}
58-
};
59-
6036
/**
6137
* The declarative form of `$platformCommandParameter`. The command declares
6238
* `provideProject()`, so resolving the project here is what makes the

‎lib/common/contracts/command-preconditions.ts‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,18 @@ export type CommandPrecondition = (
1414

1515
/**
1616
* Multi token: each `{ provide: COMMAND_PRECONDITIONS, multi: true, ... }` in
17-
* a command's `providers` contributes one precondition, and they run in the
18-
* order they were declared.
17+
* a command's `providers` contributes one precondition, or a list of them,
18+
* and they run in the order they were declared. As with Angular's multi
19+
* providers, the array is per injector level: a command that declares any
20+
* precondition of its own replaces those provided by the scope it was
21+
* registered in. To keep them, it contributes the parent's list itself:
22+
*
23+
* {
24+
* provide: COMMAND_PRECONDITIONS,
25+
* multi: true,
26+
* useFactory: () =>
27+
* inject(COMMAND_PRECONDITIONS, { skipSelf: true, optional: true }) || [],
28+
* }
1929
*/
2030
export const COMMAND_PRECONDITIONS = new InjectionToken<CommandPrecondition[]>(
2131
"commandPreconditions",

‎lib/common/contracts/commands-service.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,17 @@
11
import { Contract } from "../di/contract";
2+
import type { Injector } from "../di/injector";
23
import type { CommandReference } from "../define-command";
34

5+
export interface CommandDispatchOptions {
6+
/**
7+
* The injector a definition run as given is compiled against, the way
8+
* Angular's `createComponent` takes one. Omitted, the call's own injection
9+
* context is used, and the root when there is none. A registered name keeps
10+
* the scope it was registered under, so passing one with a name throws.
11+
*/
12+
injector?: Injector;
13+
}
14+
415
/**
516
* Dispatches commands inside the running process: the surface a command, a
617
* key shortcut or a plugin uses to run or consult another command. The command
@@ -34,6 +45,7 @@ export abstract class CommandsService {
3445
abstract runCommand(
3546
command: CommandReference,
3647
args?: string[],
48+
options?: CommandDispatchOptions,
3749
): Promise<void>;
3850

3951
/**
@@ -48,5 +60,6 @@ export abstract class CommandsService {
4860
abstract canExecuteCommand(
4961
command: CommandReference,
5062
args?: string[],
63+
options?: CommandDispatchOptions,
5164
): Promise<boolean>;
5265
}

0 commit comments

Comments
 (0)