Skip to content

Commit 92528b1

Browse files
committed
refactor(@angular/cli): several small refactoring and code quality improvements
This PR brings a number of small refactors to improve code quality in the new args parser implementation.
1 parent 33ec5e7 commit 92528b1

File tree

13 files changed

+76
-96
lines changed

13 files changed

+76
-96
lines changed

packages/angular/cli/lib/cli/index.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import { schema } from '@angular-devkit/core';
109
import { createConsoleLogger } from '@angular-devkit/core/node';
1110
import { format } from 'util';
1211
import { CommandModuleError } from '../../src/command-builder/command-module';
@@ -79,7 +78,7 @@ export default async function (options: { testing?: boolean; cliArgs: string[] }
7978
try {
8079
return await runCommand(options.cliArgs, logger, workspace);
8180
} catch (err) {
82-
if (err instanceof CommandModuleError || err instanceof schema.SchemaValidationException) {
81+
if (err instanceof CommandModuleError) {
8382
logger.fatal(`Error: ${err.message}`);
8483
} else if (err instanceof Error) {
8584
try {

packages/angular/cli/src/command-builder/architect-base-command-module.ts

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ export abstract class ArchitectBaseCommandModule<T>
3131
protected readonly missingErrorTarget: string | undefined;
3232

3333
protected async runSingleTarget(target: Target, options: OtherOptions): Promise<number> {
34-
// Remove options
3534
const architectHost = await this.getArchitectHost();
3635

3736
let builderName: string;
@@ -69,10 +68,7 @@ export abstract class ArchitectBaseCommandModule<T>
6968
return this._architectHost;
7069
}
7170

72-
const { workspace } = this.context;
73-
if (!workspace) {
74-
throw new CommandModuleError('A workspace is required for this command.');
75-
}
71+
const workspace = this.getWorkspaceOrThrow();
7672

7773
return (this._architectHost = new WorkspaceNodeModulesArchitectHost(
7874
workspace,
@@ -90,23 +86,13 @@ export abstract class ArchitectBaseCommandModule<T>
9086
registry.addPostTransform(json.schema.transforms.addUndefinedDefaults);
9187
registry.useXDeprecatedProvider((msg) => this.context.logger.warn(msg));
9288

93-
const { workspace } = this.context;
94-
if (!workspace) {
95-
throw new CommandModuleError('Cannot invoke this command outside of a workspace');
96-
}
97-
9889
const architectHost = this.getArchitectHost();
9990

10091
return (this._architect = new Architect(architectHost, registry));
10192
}
10293

10394
protected async getArchitectTargetOptions(target: Target): Promise<Option[]> {
104-
const { workspace } = this.context;
105-
if (!workspace) {
106-
throw new CommandModuleError('A workspace is required for this command.');
107-
}
108-
109-
const architectHost = new WorkspaceNodeModulesArchitectHost(workspace, workspace.basePath);
95+
const architectHost = this.getArchitectHost();
11096
const builderConf = await architectHost.getBuilderNameForTarget(target);
11197

11298
let builderDesc;

packages/angular/cli/src/command-builder/architect-command-module.ts

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -58,14 +58,8 @@ export abstract class ArchitectCommandModule
5858
}
5959

6060
async run(options: Options<ArchitectCommandArgs> & OtherOptions): Promise<number | void> {
61-
const { logger, workspace } = this.context;
62-
if (!workspace) {
63-
logger.fatal('A workspace is required for this command.');
64-
65-
return 1;
66-
}
67-
6861
const target = this.getArchitectTarget();
62+
6963
const { configuration = '', project, ...architectOptions } = options;
7064

7165
if (!project) {
@@ -112,15 +106,11 @@ export abstract class ArchitectCommandModule
112106
}
113107

114108
private getArchitectTarget(): string {
115-
// 'build [project]' -> 'build'
116-
return this.command?.split(' ', 1)[0];
109+
return this.commandName;
117110
}
118111

119112
private getProjectNamesByTarget(target: string): string[] | undefined {
120-
const workspace = this.context.workspace;
121-
if (!workspace) {
122-
throw new CommandModuleError('A workspace is required for this command.');
123-
}
113+
const workspace = this.getWorkspaceOrThrow();
124114

125115
const allProjectsForTargetName: string[] = [];
126116
for (const [name, project] of workspace.projects) {

packages/angular/cli/src/command-builder/command-module.ts

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import { analytics, logging, normalize, strings } from '@angular-devkit/core';
9+
import { analytics, logging, normalize, schema, strings } from '@angular-devkit/core';
1010
import { readFileSync } from 'fs';
1111
import * as path from 'path';
1212
import {
@@ -38,7 +38,7 @@ export interface CommandContext {
3838
root: string;
3939
workspace?: AngularWorkspace;
4040
logger: logging.Logger;
41-
/** Arguments parsed in free from without parser configuration. */
41+
/** Arguments parsed in free-from without parser configuration. */
4242
args: {
4343
positional: string[];
4444
options: {
@@ -121,16 +121,25 @@ export abstract class CommandModule<T extends {} = {}> implements CommandModuleI
121121
await this.reportAnalytics(camelCasedOptions);
122122
}
123123

124-
// Run and time command.
125-
const startTime = Date.now();
126-
const result = await this.run(camelCasedOptions as Options<T> & OtherOptions);
127-
const endTime = Date.now();
128-
129-
analytics.timing(this.commandName, 'duration', endTime - startTime);
130-
await analytics.flush();
131-
132-
if (typeof result === 'number' && result > 0) {
133-
process.exitCode = result;
124+
let exitCode: number | void | undefined;
125+
try {
126+
// Run and time command.
127+
const startTime = Date.now();
128+
exitCode = await this.run(camelCasedOptions as Options<T> & OtherOptions);
129+
const endTime = Date.now();
130+
analytics.timing(this.commandName, 'duration', endTime - startTime);
131+
await analytics.flush();
132+
} catch (e) {
133+
if (e instanceof schema.SchemaValidationException) {
134+
this.context.logger.fatal(`Error: ${e.message}`);
135+
exitCode = 1;
136+
} else {
137+
throw e;
138+
}
139+
} finally {
140+
if (typeof exitCode === 'number' && exitCode > 0) {
141+
process.exitCode = exitCode;
142+
}
134143
}
135144
}
136145

@@ -223,6 +232,15 @@ export abstract class CommandModule<T extends {} = {}> implements CommandModuleI
223232

224233
return localYargs;
225234
}
235+
236+
protected getWorkspaceOrThrow(): AngularWorkspace {
237+
const { workspace } = this.context;
238+
if (!workspace) {
239+
throw new CommandModuleError('A workspace is required for this command.');
240+
}
241+
242+
return workspace;
243+
}
226244
}
227245

228246
/**

packages/angular/cli/src/command-builder/command-runner.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -90,22 +90,22 @@ export async function runCommand(
9090
}
9191
}
9292

93-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
94-
const commandModule = new CommandModule(context) as any;
93+
const commandModule = new CommandModule(context);
9594
const describe = jsonHelp ? commandModule.fullDescribe : commandModule.describe;
9695

9796
localYargs = localYargs.command({
9897
command: commandModule.command,
99-
aliases: commandModule.aliases,
98+
aliases: 'aliases' in commandModule ? commandModule.aliases : undefined,
10099
describe:
101100
// We cannot add custom fields in help, such as long command description which is used in AIO.
102-
// Therefore, we get around this by adding a complex object as a string which we later parse when geneerating the help files.
101+
// Therefore, we get around this by adding a complex object as a string which we later parse when generating the help files.
103102
describe !== undefined && typeof describe === 'object'
104103
? JSON.stringify(describe)
105104
: describe,
106-
deprecated: commandModule.deprecated,
107-
builder: (x) => commandModule.builder(x),
108-
handler: (x) => commandModule.handler(x),
105+
deprecated: 'deprecated' in commandModule ? commandModule.deprecated : undefined,
106+
builder: (argv) => commandModule.builder(argv) as yargs.Argv,
107+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
108+
handler: (args: any) => commandModule.handler(args),
109109
});
110110
}
111111

packages/angular/cli/src/command-builder/utilities/architect.ts

Lines changed: 0 additions & 16 deletions
This file was deleted.

packages/angular/cli/src/command-builder/utilities/json-help.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { FullDescribe } from '../command-module';
1111

1212
export interface JsonHelp {
1313
name: string;
14-
description?: string;
14+
shortDescription?: string;
1515
command: string;
1616
longDescription?: string;
1717
longDescriptionRelativePath?: string;
@@ -111,22 +111,22 @@ export function jsonHelpUsage(): string {
111111
}))
112112
.sort((a, b) => a.name.localeCompare(b.name));
113113

114-
const parseDescription = (rawDescription: string) => {
114+
const parseDescription = (rawDescription: string): Partial<JsonHelp> => {
115115
try {
116116
const {
117117
longDescription,
118-
describe: description,
118+
describe: shortDescription,
119119
longDescriptionRelativePath,
120120
} = JSON.parse(rawDescription) as FullDescribe;
121121

122122
return {
123-
description,
123+
shortDescription,
124124
longDescriptionRelativePath,
125125
longDescription,
126126
};
127127
} catch {
128128
return {
129-
description: rawDescription,
129+
shortDescription: rawDescription,
130130
};
131131
}
132132
};

packages/angular/cli/src/commands/analytics/cli.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,7 @@ interface AnalyticsCommandArgs {
2222

2323
export class AnalyticsCommandModule extends CommandModule<AnalyticsCommandArgs> {
2424
command = 'analytics <setting-or-project>';
25-
describe =
26-
'Configures the gathering of Angular CLI usage metrics. See https://angular.io/cli/usage-analytics-gathering.';
25+
describe = 'Configures the gathering of Angular CLI usage metrics.';
2726
longDescriptionPath = join(__dirname, 'long-description.md');
2827

2928
builder(localYargs: Argv): Argv<AnalyticsCommandArgs> {

packages/angular/cli/src/commands/analytics/long-description.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,5 @@ The value of `setting-or-project` is one of the following.
66
which uses a common CI user.
77
- `prompt`: Prompts the user to set the status interactively.
88
- `project`: Sets the default status for the project to the `project-setting` value, which can be any of the other values. The `project-setting` argument is ignored for all other values of `setting_or_project`.
9+
10+
For further details, see [Gathering an Viewing CLI Usage Analytics](cli/usage-analytics-gathering).

packages/angular/cli/src/commands/config/long-description.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ or indirectly on the command line using this command.
66

77
The configurable property names match command option names,
88
except that in the configuration file, all names must use camelCase,
9-
while on the command line options can be given in either camelCase or dash-case.
9+
while on the command line options can be given dash-case.
1010

1111
For further details, see [Workspace Configuration](guide/workspace-config).
12+
13+
For configuration of CLI usage analytics, see [Gathering an Viewing CLI Usage Analytics](cli/usage-analytics-gathering).

0 commit comments

Comments
 (0)