Skip to content

Commit

Permalink
refactor(cdk/schematics): switch to latest API version
Browse files Browse the repository at this point in the history
The latest `next` version of the CLI removed some deprecated APIs that we were depending.
These changes update all the code so that it used the correct API (according to the CLI team).
High-level overview of the changes and why they were necessary:
1. Previously we parsed the `angular.json` ourselves using `JSON.parse` in order to support very old versions
of the CLI, however this is no longer feasible, because the CLI has set up classes around the parsed data which
are non-trivial to construct. According to the CLI, we don't have to worry about older version anymore, because
the schematics infrastructure will ensure that we're running against the correct version.
2. The interface of the new API is different from the one we were using before so I had to rewrite some code.
3. Some of these new APIs are asynchronous so I've had to move some code around to accommodate it.
4. Previously we would `JSON.parse` and `JSON.stringify` the `angular.json` file whenever we needed to mutate it
(e.g. when adding a theme), but this isn't possible anymore due to the aforementioned classes around the
config file. I've reworked our schematics to use a utility from the CLI to write to the file.
5. A lot of our tests depended on parsing the `angular.json`, changing a property and writing it back as JSON.
This isn't possible, because the abstraction around the config can't be stringified so I've worked around it by
writing out the `angular.json` file from scratch in the test. While this is more code, it should be easier to maintain
in the long term.
  • Loading branch information
crisbeto authored and wagnermaciel committed Oct 12, 2020
1 parent 727285b commit 5bac828
Show file tree
Hide file tree
Showing 24 changed files with 311 additions and 280 deletions.
56 changes: 34 additions & 22 deletions src/cdk/schematics/ng-generate/drag-drop/index.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
import {SchematicTestRunner} from '@angular-devkit/schematics/testing';
import {getProjectFromWorkspace} from '@angular/cdk/schematics';
import {getWorkspace} from '@schematics/angular/utility/config';
import {COLLECTION_PATH} from '../../index.spec';
import {createTestApp, getFileContent} from '../../testing';
import {Schema} from './schema';
Expand Down Expand Up @@ -56,16 +54,23 @@ describe('CDK drag-drop schematic', () => {

it('should respect the deprecated "styleext" option value', async () => {
let tree = await createTestApp(runner);
const workspace = getWorkspace(tree);
const project = getProjectFromWorkspace(workspace);

// We need to specify the default component options by overwriting
// the existing workspace configuration because passing the "styleext"
// option is no longer supported. Though we want to verify that we
// properly handle old CLI projects which still use the "styleext" option.
project.schematics!['@schematics/angular:component'] = {styleext: 'scss'};

tree.overwrite('angular.json', JSON.stringify(workspace));
tree.overwrite('angular.json', JSON.stringify({
version: 1,
defaultProject: 'material',
projects: {
material: {
projectType: 'application',
schematics: {
// We need to specify the default component options by overwriting
// the existing workspace configuration because passing the "styleext"
// option is no longer supported. Though we want to verify that we
// properly handle old CLI projects which still use the "styleext" option.
'@schematics/angular:component': {styleext: 'scss'}
},
root: 'projects/material'
}
}
}));
tree = await runner.runSchematicAsync('drag-drop', baseOptions, tree).toPromise();

expect(tree.files).toContain('/projects/material/src/app/foo/foo.component.scss');
Expand Down Expand Up @@ -153,16 +158,23 @@ describe('CDK drag-drop schematic', () => {

it('should respect the deprecated global "spec" option value', async () => {
let tree = await createTestApp(runner);
const workspace = getWorkspace(tree);
const project = getProjectFromWorkspace(workspace);

// We need to specify the default component options by overwriting
// the existing workspace configuration because passing the "spec"
// option is no longer supported. Though we want to verify that we
// properly handle old CLI projects which still use the "spec" option.
project.schematics!['@schematics/angular:component'] = {spec: false};

tree.overwrite('angular.json', JSON.stringify(workspace));
tree.overwrite('angular.json', JSON.stringify({
version: 1,
defaultProject: 'material',
projects: {
material: {
projectType: 'application',
schematics: {
// We need to specify the default component options by overwriting
// the existing workspace configuration because passing the "spec"
// option is no longer supported. Though we want to verify that we
// properly handle old CLI projects which still use the "spec" option.
'@schematics/angular:component': {spec: false}
},
root: 'projects/material'
}
}
}));
tree = await runner.runSchematicAsync('drag-drop', baseOptions, tree).toPromise();

expect(tree.files).not.toContain('/projects/material/src/app/foo/foo.component.spec.ts');
Expand Down
8 changes: 3 additions & 5 deletions src/cdk/schematics/ng-generate/drag-drop/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,8 @@ export default function(options: Schema): Rule {

/** Adds the required modules to the main module of the CLI project. */
function addDragDropModulesToModule(options: Schema) {
return (host: Tree) => {
const modulePath = findModuleFromOptions(host, options)!;

addModuleImportToModule(host, modulePath, 'DragDropModule', '@angular/cdk/drag-drop');
return host;
return async (host: Tree) => {
const modulePath = await findModuleFromOptions(host, options);
addModuleImportToModule(host, modulePath!, 'DragDropModule', '@angular/cdk/drag-drop');
};
}
10 changes: 5 additions & 5 deletions src/cdk/schematics/ng-update/devkit-migration-rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import {Rule, SchematicContext, Tree} from '@angular-devkit/schematics';
import {NodePackageInstallTask} from '@angular-devkit/schematics/tasks';
import {WorkspaceProject} from '@schematics/angular/utility/workspace-models';
import {ProjectDefinition} from '@angular-devkit/core/src/workspace';

import {UpdateProject} from '../update-tool';
import {WorkspacePath} from '../update-tool/file-system';
Expand Down Expand Up @@ -62,7 +62,7 @@ export function createMigrationSchematicRule(
upgradeData: UpgradeData, onMigrationCompleteFn?: PostMigrationFn): Rule {
return async (tree: Tree, context: SchematicContext) => {
const logger = context.logger;
const workspace = getWorkspaceConfigGracefully(tree);
const workspace = await getWorkspaceConfigGracefully(tree);

if (workspace === null) {
logger.error('Could not find workspace configuration file.');
Expand All @@ -74,12 +74,12 @@ export function createMigrationSchematicRule(
// we don't want to check these again, as this would result in duplicated failure messages.
const analyzedFiles = new Set<WorkspacePath>();
const fileSystem = new DevkitFileSystem(tree);
const projectNames = Object.keys(workspace.projects);
const projectNames = workspace.projects.keys();
const migrations: NullableDevkitMigration[] = [...cdkMigrations, ...extraMigrations];
let hasFailures = false;

for (const projectName of projectNames) {
const project = workspace.projects[projectName];
const project = workspace.projects.get(projectName)!;
const buildTsconfigPath = getTargetTsconfigPath(project, 'build');
const testTsconfigPath = getTargetTsconfigPath(project, 'test');

Expand Down Expand Up @@ -126,7 +126,7 @@ export function createMigrationSchematicRule(
}

/** Runs the migrations for the specified workspace project. */
function runMigrations(project: WorkspaceProject, projectName: string,
function runMigrations(project: ProjectDefinition, projectName: string,
tsconfigPath: WorkspacePath, additionalStylesheetPaths: string[],
isTestTarget: boolean) {
const program = UpdateProject.createProgramFromTsconfig(tsconfigPath, fileSystem);
Expand Down
4 changes: 2 additions & 2 deletions src/cdk/schematics/ng-update/devkit-migration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import {SchematicContext, Tree} from '@angular-devkit/schematics';
import {WorkspaceProject} from '@schematics/angular/utility/workspace-models';
import {ProjectDefinition} from '@angular-devkit/core/src/workspace';
import {Constructor, Migration, PostMigrationAction} from '../update-tool/migration';

export type DevkitContext = {
Expand All @@ -16,7 +16,7 @@ export type DevkitContext = {
/** Name of the project the migrations run against. */
projectName: string;
/** Workspace project the migrations run against. */
project: WorkspaceProject,
project: ProjectDefinition,
/** Whether the migrations run for a test target. */
isTestTarget: boolean,
};
Expand Down
4 changes: 1 addition & 3 deletions src/cdk/schematics/testing/test-case-setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,7 @@ export async function createTestCaseSetup(migrationName: string, collectionPath:

let logOutput = '';
runner.logger.subscribe(entry => logOutput += `${entry.message}\n`);

const {appTree, writeFile} =
await createFileSystemTestApp(runner);
const {appTree, writeFile} = await createFileSystemTestApp(runner);

_patchTypeScriptDefaultLib(appTree);

Expand Down
17 changes: 9 additions & 8 deletions src/cdk/schematics/utils/ast.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@
* found in the LICENSE file at https://angular.io/license
*/

import {WorkspaceProject} from '@angular-devkit/core/src/experimental/workspace';
import {SchematicsException, Tree} from '@angular-devkit/schematics';
import {Schema as ComponentOptions} from '@schematics/angular/component/schema';
import {InsertChange} from '@schematics/angular/utility/change';
import {getWorkspace} from '@schematics/angular/utility/config';
import {getWorkspace} from '@schematics/angular/utility/workspace';
import {findModuleFromOptions as internalFindModule} from '@schematics/angular/utility/find-module';
import {ProjectDefinition} from '@angular-devkit/core/src/workspace';
import * as ts from 'typescript';
import {getProjectMainFile} from './project-main-file';
import {addImportToModule, getAppModulePath} from './vendored-ast-utils';
Expand All @@ -27,7 +27,7 @@ export function parseSourceFile(host: Tree, path: string): ts.SourceFile {

/** Import and add module to root app module. */
export function addModuleImportToRootModule(host: Tree, moduleName: string, src: string,
project: WorkspaceProject) {
project: ProjectDefinition) {
const modulePath = getAppModulePath(host, getProjectMainFile(project));
addModuleImportToModule(host, modulePath, moduleName, src);
}
Expand All @@ -51,7 +51,7 @@ export function addModuleImportToModule(host: Tree, modulePath: string, moduleNa
const changes = addImportToModule(moduleSource, modulePath, moduleName, src);
const recorder = host.beginUpdate(modulePath);

changes.forEach((change) => {
changes.forEach(change => {
if (change instanceof InsertChange) {
recorder.insertLeft(change.pos, change.toAdd);
}
Expand All @@ -61,14 +61,15 @@ export function addModuleImportToModule(host: Tree, modulePath: string, moduleNa
}

/** Wraps the internal find module from options with undefined path handling */
export function findModuleFromOptions(host: Tree, options: ComponentOptions): string | undefined {
const workspace = getWorkspace(host);
export async function findModuleFromOptions(host: Tree, options: ComponentOptions):
Promise<string | undefined> {
const workspace = await getWorkspace(host);

if (!options.project) {
options.project = Object.keys(workspace.projects)[0];
options.project = Array.from(workspace.projects.keys())[0];
}

const project = workspace.projects[options.project];
const project = workspace.projects.get(options.project)!;

if (options.path === undefined) {
options.path = `/${project.root}/src/app`;
Expand Down
19 changes: 10 additions & 9 deletions src/cdk/schematics/utils/build-component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@ import {
import {FileSystemSchematicContext} from '@angular-devkit/schematics/tools';
import {Schema as ComponentOptions, Style} from '@schematics/angular/component/schema';
import {InsertChange} from '@schematics/angular/utility/change';
import {getWorkspace} from '@schematics/angular/utility/config';
import {getWorkspace} from '@schematics/angular/utility/workspace';
import {buildRelativePath, findModuleFromOptions} from '@schematics/angular/utility/find-module';
import {parseName} from '@schematics/angular/utility/parse-name';
import {validateHtmlSelector, validateName} from '@schematics/angular/utility/validation';
import {ProjectType, WorkspaceProject} from '@schematics/angular/utility/workspace-models';
import {ProjectType} from '@schematics/angular/utility/workspace-models';
import {readFileSync, statSync} from 'fs';
import {dirname, join, resolve} from 'path';
import * as ts from 'typescript';
Expand All @@ -39,17 +39,18 @@ import {
} from '../utils/vendored-ast-utils';
import {getProjectFromWorkspace} from './get-project';
import {getDefaultComponentOptions} from './schematic-options';
import {ProjectDefinition} from '@angular-devkit/core/src/workspace';

/**
* Build a default project path for generating.
* @param project The project to build the path for.
*/
function buildDefaultPath(project: WorkspaceProject): string {
function buildDefaultPath(project: ProjectDefinition): string {
const root = project.sourceRoot
? `/${project.sourceRoot}/`
: `/${project.root}/src/`;

const projectDirName = project.projectType === ProjectType.Application ? 'app' : 'lib';
const projectDirName = project.extensions.projectType === ProjectType.Application ? 'app' : 'lib';

return `${root}${projectDirName}`;
}
Expand Down Expand Up @@ -143,7 +144,7 @@ function addDeclarationToNgModule(options: ComponentOptions): Rule {
}


function buildSelector(options: ComponentOptions, projectPrefix: string) {
function buildSelector(options: ComponentOptions, projectPrefix?: string) {
let selector = strings.dasherize(options.name);
if (options.prefix) {
selector = `${options.prefix}-${selector}`;
Expand Down Expand Up @@ -176,8 +177,8 @@ function indentTextContent(text: string, numSpaces: number): string {
export function buildComponent(options: ComponentOptions,
additionalFiles: {[key: string]: string} = {}): Rule {

return (host: Tree, context: FileSystemSchematicContext) => {
const workspace = getWorkspace(host);
return async (host: Tree, context: FileSystemSchematicContext) => {
const workspace = await getWorkspace(host);
const project = getProjectFromWorkspace(workspace, options.project);
const defaultComponentOptions = getDefaultComponentOptions(project);

Expand All @@ -200,7 +201,7 @@ export function buildComponent(options: ComponentOptions,

if (options.path === undefined) {
// TODO(jelbourn): figure out if the need for this `as any` is a bug due to two different
// incompatible `WorkspaceProject` classes in @angular-devkit
// incompatible `ProjectDefinition` classes in @angular-devkit
options.path = buildDefaultPath(project as any);
}

Expand Down Expand Up @@ -257,7 +258,7 @@ export function buildComponent(options: ComponentOptions,
move(null as any, parsedPath.path),
]);

return chain([
return () => chain([
branchAndMerge(chain([
addDeclarationToNgModule(options),
mergeWith(templateSource),
Expand Down
9 changes: 4 additions & 5 deletions src/cdk/schematics/utils/get-project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,17 @@
* found in the LICENSE file at https://angular.io/license
*/

import {WorkspaceSchema, WorkspaceProject} from '@angular-devkit/core/src/experimental/workspace';
import {ProjectDefinition, WorkspaceDefinition} from '@angular-devkit/core/src/workspace';
import {SchematicsException} from '@angular-devkit/schematics';

/**
* Finds the specified project configuration in the workspace. Throws an error if the project
* couldn't be found.
*/
export function getProjectFromWorkspace(
workspace: WorkspaceSchema,
projectName?: string): WorkspaceProject {

const project = workspace.projects[projectName || workspace.defaultProject!];
workspace: WorkspaceDefinition,
projectName = workspace.extensions.defaultProject as string): ProjectDefinition {
const project = workspace.projects.get(projectName);

if (!project) {
throw new SchematicsException(`Could not find project in workspace: ${projectName}`);
Expand Down
17 changes: 8 additions & 9 deletions src/cdk/schematics/utils/project-index-file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,15 @@
*/

import {Path} from '@angular-devkit/core';
import {WorkspaceProject} from '@angular-devkit/core/src/experimental/workspace';
import {BrowserBuilderTarget} from '@schematics/angular/utility/workspace-models';
import {ProjectDefinition} from '@angular-devkit/core/src/workspace';
import {defaultTargetBuilders, getTargetsByBuilderName} from './project-targets';

/** Gets the path of the index file in the given project. */
export function getProjectIndexFiles(project: WorkspaceProject): Path[] {
// Use a set to remove duplicate index files referenced in multiple build targets
// of a project.
return [...new Set(
(getTargetsByBuilderName(project, defaultTargetBuilders.build) as BrowserBuilderTarget[])
.filter(t => t.options.index)
.map(t => t.options.index! as Path))];
export function getProjectIndexFiles(project: ProjectDefinition): Path[] {
const paths = getTargetsByBuilderName(project, defaultTargetBuilders.build)
.filter(t => t.options?.index)
.map(t => t.options!.index as Path);

// Use a set to remove duplicate index files referenced in multiple build targets of a project.
return Array.from(new Set(paths));
}
6 changes: 3 additions & 3 deletions src/cdk/schematics/utils/project-main-file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,18 @@
*/

import {Path} from '@angular-devkit/core';
import {WorkspaceProject} from '@angular-devkit/core/src/experimental/workspace';
import {ProjectDefinition} from '@angular-devkit/core/src/workspace';
import {SchematicsException} from '@angular-devkit/schematics';
import {getProjectTargetOptions} from './project-targets';

/** Looks for the main TypeScript file in the given project and returns its path. */
export function getProjectMainFile(project: WorkspaceProject): Path {
export function getProjectMainFile(project: ProjectDefinition): Path {
const buildOptions = getProjectTargetOptions(project, 'build');

if (!buildOptions.main) {
throw new SchematicsException(`Could not find the project main file inside of the ` +
`workspace config (${project.sourceRoot})`);
}

return buildOptions.main;
return buildOptions.main as Path;
}
12 changes: 6 additions & 6 deletions src/cdk/schematics/utils/project-style-file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
* found in the LICENSE file at https://angular.io/license
*/

import {normalize} from '@angular-devkit/core';
import {WorkspaceProject} from '@angular-devkit/core/src/experimental/workspace';
import {isJsonArray, normalize} from '@angular-devkit/core';
import {ProjectDefinition} from '@angular-devkit/core/src/workspace';
import {getProjectTargetOptions} from './project-targets';

/** Regular expression that matches all possible Angular CLI default style files. */
Expand All @@ -20,11 +20,11 @@ const validStyleFileRegex = /\.(c|le|sc)ss/;
* Gets a style file with the given extension in a project and returns its path. If no
* extension is specified, any style file with a valid extension will be returned.
*/
export function getProjectStyleFile(project: WorkspaceProject, extension?: string): string | null {
export function getProjectStyleFile(project: ProjectDefinition, extension?: string): string | null {
const buildOptions = getProjectTargetOptions(project, 'build');

if (buildOptions.styles && buildOptions.styles.length) {
const styles = buildOptions.styles.map(s => typeof s === 'string' ? s : s.input);
if (buildOptions.styles && isJsonArray(buildOptions.styles) && buildOptions.styles.length) {
const styles =
buildOptions.styles.map(s => typeof s === 'string' ? s : (s as {input: string}).input);

// Look for the default style file that is generated for new projects by the Angular CLI. This
// default style file is usually called `styles.ext` unless it has been changed explicitly.
Expand Down
Loading

0 comments on commit 5bac828

Please sign in to comment.