Skip to content

Commit a1d3501

Browse files
mgechevmgechev
committed
fix(@angular/cli): use correct schematic defaults considering workspace
Fix #14986 This PR includes some refactoring to simplify the interaction of the `NodeWorkflow` and the `BaseWorkflow` with the registry. We were registering redundant `addPostTransform`s. Some of them in the constructor of the `BaseWorkflow`, which did not allow us to intercept `addUndefinedDefaults`. Additionally, we were setting the `validateOptionsWithSchema` transform multiple times unnecessarily. An issue left to fix is support for the `--project` option in schematic commands. Currently, `getProjectName` does not know about this option, since `createWorkflow` does not know how to parse the command line arguments. The parsing logic is implemented partially by the concrete implementation of the `SchematicCommand` template method.
1 parent 560ba2a commit a1d3501

6 files changed

Lines changed: 40 additions & 27 deletions

File tree

‎packages/angular/cli/models/schematic-command.ts‎

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,12 @@ import {
1717
} from '@angular-devkit/core';
1818
import { NodeJsSyncHost } from '@angular-devkit/core/node';
1919
import { DryRunEvent, UnsuccessfulWorkflowExecution, workflow } from '@angular-devkit/schematics';
20+
import { standardFormats } from '@angular-devkit/schematics/src/formats';
2021
import {
2122
FileSystemCollection,
2223
FileSystemEngine,
2324
FileSystemSchematic,
25+
FileSystemSchematicDescription,
2426
NodeWorkflow,
2527
validateOptionsWithSchema,
2628
} from '@angular-devkit/schematics/tools';
@@ -247,6 +249,7 @@ export abstract class SchematicCommand<
247249
dryRun,
248250
packageManager: getPackageManager(this.workspace.root),
249251
root: normalize(this.workspace.root),
252+
registry: new schema.CoreSchemaRegistry(standardFormats)
250253
});
251254
workflow.engineHost.registerContextTransform(context => {
252255
// This is run by ALL schematics, so if someone uses `externalSchematics(...)` which
@@ -262,15 +265,7 @@ export abstract class SchematicCommand<
262265
}
263266
});
264267

265-
workflow.engineHost.registerOptionsTransform(validateOptionsWithSchema(workflow.registry));
266-
267-
if (options.defaults) {
268-
workflow.registry.addPreTransform(schema.transforms.addUndefinedDefaults);
269-
} else {
270-
workflow.registry.addPostTransform(schema.transforms.addUndefinedDefaults);
271-
}
272-
273-
workflow.registry.addSmartDefaultProvider('projectName', () => {
268+
const getProjectName = () => {
274269
if (this._workspace) {
275270
try {
276271
return (
@@ -292,7 +287,24 @@ export abstract class SchematicCommand<
292287
}
293288

294289
return undefined;
295-
});
290+
};
291+
292+
workflow.engineHost.registerOptionsTransform(
293+
<T extends {}>(schematic: FileSystemSchematicDescription, current: T) => ({
294+
...getSchematicDefaults(schematic.collection.name, schematic.name, getProjectName()),
295+
...current,
296+
})
297+
);
298+
299+
if (options.defaults) {
300+
workflow.registry.addPreTransform(schema.transforms.addUndefinedDefaults);
301+
} else {
302+
workflow.registry.addPostTransform(schema.transforms.addUndefinedDefaults);
303+
}
304+
305+
workflow.engineHost.registerOptionsTransform(validateOptionsWithSchema(workflow.registry));
306+
307+
workflow.registry.addSmartDefaultProvider('projectName', getProjectName);
296308

297309
if (options.interactive !== false && isTTY()) {
298310
workflow.registry.usePromptProvider((definitions: Array<schema.PromptDefinition>) => {

‎packages/angular_devkit/schematics/src/workflow/base.ts‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import { EMPTY, Observable, Subject, concat, from, of, throwError } from 'rxjs';
1010
import { concatMap, defaultIfEmpty, ignoreElements, last, map, tap } from 'rxjs/operators';
1111
import { Engine, EngineHost, SchematicEngine } from '../engine';
1212
import { UnsuccessfulWorkflowExecution } from '../exception/exception';
13-
import { standardFormats } from '../formats';
1413
import { DryRunEvent, DryRunSink } from '../sink/dryrun';
1514
import { HostSink } from '../sink/host';
1615
import { Sink } from '../sink/sink';
@@ -28,7 +27,7 @@ import {
2827
export interface BaseWorkflowOptions {
2928
host: virtualFs.Host;
3029
engineHost: EngineHost<{}, {}>;
31-
registry?: schema.CoreSchemaRegistry;
30+
registry: schema.CoreSchemaRegistry;
3231

3332
force?: boolean;
3433
dryRun?: boolean;
@@ -63,13 +62,7 @@ export abstract class BaseWorkflow implements Workflow {
6362
constructor(options: BaseWorkflowOptions) {
6463
this._host = options.host;
6564
this._engineHost = options.engineHost;
66-
67-
if (options.registry) {
68-
this._registry = options.registry;
69-
} else {
70-
this._registry = new schema.CoreSchemaRegistry(standardFormats);
71-
this._registry.addPostTransform(schema.transforms.addUndefinedDefaults);
72-
}
65+
this._registry = options.registry;
7366

7467
this._engine = new SchematicEngine(this._engineHost, this);
7568

‎packages/angular_devkit/schematics/tools/description.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919

2020

2121
export interface FileSystemCollectionDescription {
22+
readonly name: string;
2223
readonly path: string;
2324
readonly version?: string;
2425
readonly schematics: { [name: string]: FileSystemSchematicDesc };
@@ -28,6 +29,8 @@ export interface FileSystemCollectionDescription {
2829
export interface FileSystemSchematicJsonDescription {
2930
readonly aliases?: string[];
3031
readonly factory: string;
32+
readonly name: string;
33+
readonly collection: FileSystemCollectionDescription;
3134
readonly description: string;
3235
readonly schema?: string;
3336
readonly extends?: string;

‎packages/angular_devkit/schematics/tools/workflow/node-workflow.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,13 @@
55
* Use of this source code is governed by an MIT-style license that can be
66
* found in the LICENSE file at https://angular.io/license
77
*/
8-
import { Path, getSystemPath, virtualFs } from '@angular-devkit/core';
8+
import { Path, getSystemPath, schema, virtualFs } from '@angular-devkit/core';
99
import {
1010
workflow,
1111
} from '@angular-devkit/schematics'; // tslint:disable-line:no-implicit-dependencies
1212
import { BuiltinTaskExecutor } from '../../tasks/node';
1313
import { FileSystemEngine } from '../description';
1414
import { NodeModulesEngineHost } from '../node-module-engine-host';
15-
import { validateOptionsWithSchema } from '../schema-option-transform';
1615

1716
/**
1817
* A workflow specifically for Node tools.
@@ -24,7 +23,8 @@ export class NodeWorkflow extends workflow.BaseWorkflow {
2423
force?: boolean;
2524
dryRun?: boolean;
2625
root?: Path,
27-
packageManager?: string;
26+
packageManager?: string,
27+
registry: schema.CoreSchemaRegistry
2828
},
2929
) {
3030
const engineHost = new NodeModulesEngineHost();
@@ -34,9 +34,9 @@ export class NodeWorkflow extends workflow.BaseWorkflow {
3434

3535
force: options.force,
3636
dryRun: options.dryRun,
37+
registry: options.registry
3738
});
3839

39-
engineHost.registerOptionsTransform(validateOptionsWithSchema(this._registry));
4040
engineHost.registerTaskExecutor(
4141
BuiltinTaskExecutor.NodePackage,
4242
{

‎packages/angular_devkit/schematics/tools/workflow/node-workflow_spec.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88
// tslint:disable:no-implicit-dependencies
9+
import { schema } from '@angular-devkit/core';
910
import { NodeJsSyncHost } from '@angular-devkit/core/node';
1011
import { NodeWorkflow } from '@angular-devkit/schematics/tools';
1112
import * as path from 'path';
@@ -14,7 +15,7 @@ import * as path from 'path';
1415
describe('NodeWorkflow', () => {
1516
// TODO: this test seems to either not work on windows or on linux.
1617
xit('works', done => {
17-
const workflow = new NodeWorkflow(new NodeJsSyncHost(), { dryRun: true });
18+
const workflow = new NodeWorkflow(new NodeJsSyncHost(), { dryRun: true, registry: new schema.CoreSchemaRegistry() });
1819
const collection = path.join(__dirname, '../../../../schematics/angular/package.json');
1920

2021
workflow.execute({

‎packages/angular_devkit/schematics_cli/bin/schematics.ts‎

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

10-
import 'symbol-observable';
1110
// symbol polyfill must go first
1211
// tslint:disable-next-line:ordered-imports import-groups
1312
import {
@@ -25,9 +24,11 @@ import {
2524
SchematicEngine,
2625
UnsuccessfulWorkflowExecution,
2726
} from '@angular-devkit/schematics';
28-
import { NodeModulesEngineHost, NodeWorkflow } from '@angular-devkit/schematics/tools';
27+
import { standardFormats } from '@angular-devkit/schematics/src/formats';
28+
import { NodeModulesEngineHost, NodeWorkflow, validateOptionsWithSchema } from '@angular-devkit/schematics/tools';
2929
import * as inquirer from 'inquirer';
3030
import * as minimist from 'minimist';
31+
import 'symbol-observable';
3132

3233

3334
/**
@@ -160,9 +161,12 @@ export async function main({
160161

161162
/** Create a Virtual FS Host scoped to where the process is being run. **/
162163
const fsHost = new virtualFs.ScopedHost(new NodeJsSyncHost(), normalize(process.cwd()));
164+
const registry = new schema.CoreSchemaRegistry(standardFormats);
163165

164166
/** Create the workflow that will be executed with this run. */
165-
const workflow = new NodeWorkflow(fsHost, { force, dryRun });
167+
const workflow = new NodeWorkflow(fsHost, { force, dryRun, registry });
168+
registry.addPostTransform(schema.transforms.addUndefinedDefaults);
169+
workflow.engineHost.registerOptionsTransform(validateOptionsWithSchema(registry));
166170

167171
// Indicate to the user when nothing has been done. This is automatically set to off when there's
168172
// a new DryRunEvent.

0 commit comments

Comments
 (0)