Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Schematics code cleanup and refactoring to support strict compilation #18415

Merged
merged 4 commits into from Aug 3, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
32 changes: 13 additions & 19 deletions packages/schematics/angular/app-shell/index.ts
Expand Up @@ -26,7 +26,7 @@ import {
insertImport,
isImported,
} from '../utility/ast-utils';
import { Change, InsertChange } from '../utility/change';
import { applyToUpdateRecorder } from '../utility/change';
import { getAppModulePath } from '../utility/ng-ast-utils';
import { targetBuildNotFoundError } from '../utility/project-targets';
import { getWorkspace, updateWorkspace } from '../utility/workspace';
Expand Down Expand Up @@ -110,11 +110,11 @@ function getBootstrapComponentPath(
const componentSymbol = arrLiteral.elements[0].getText();

const relativePath = getSourceNodes(moduleSource)
.filter(node => node.kind === ts.SyntaxKind.ImportDeclaration)
.filter(ts.isImportDeclaration)
.filter(imp => {
return findNode(imp, ts.SyntaxKind.Identifier, componentSymbol);
})
.map((imp: ts.ImportDeclaration) => {
.map(imp => {
const pathStringLiteral = imp.moduleSpecifier as ts.StringLiteral;

return pathStringLiteral.text;
Expand Down Expand Up @@ -191,11 +191,7 @@ function addRouterModule(mainPath: string): Rule {
const moduleSource = getSourceFile(host, modulePath);
const changes = addImportToModule(moduleSource, modulePath, 'RouterModule', '@angular/router');
const recorder = host.beginUpdate(modulePath);
changes.forEach((change: Change) => {
if (change instanceof InsertChange) {
recorder.insertLeft(change.pos, change.toAdd);
}
});
applyToUpdateRecorder(recorder, changes);
host.commitUpdate(recorder);

return host;
Expand All @@ -205,8 +201,8 @@ function addRouterModule(mainPath: string): Rule {
function getMetadataProperty(metadata: ts.Node, propertyName: string): ts.PropertyAssignment {
const properties = (metadata as ts.ObjectLiteralExpression).properties;
const property = properties
.filter(prop => prop.kind === ts.SyntaxKind.PropertyAssignment)
.filter((prop: ts.PropertyAssignment) => {
.filter(ts.isPropertyAssignment)
.filter((prop) => {
const name = prop.name;
switch (name.kind) {
case ts.SyntaxKind.Identifier:
Expand Down Expand Up @@ -248,9 +244,9 @@ function addServerRoutes(options: AppShellOptions): Rule {
const routesChange = insertImport(moduleSource,
modulePath,
'Routes',
'@angular/router') as InsertChange;
if (routesChange.toAdd) {
recorder.insertLeft(routesChange.pos, routesChange.toAdd);
'@angular/router');
if (routesChange) {
applyToUpdateRecorder(recorder, [routesChange]);
}

const imports = getSourceNodes(moduleSource)
Expand All @@ -269,18 +265,16 @@ function addServerRoutes(options: AppShellOptions): Rule {
const routerModuleChange = insertImport(moduleSource,
modulePath,
'RouterModule',
'@angular/router') as InsertChange;
'@angular/router');

if (routerModuleChange.toAdd) {
recorder.insertLeft(routerModuleChange.pos, routerModuleChange.toAdd);
if (routerModuleChange) {
applyToUpdateRecorder(recorder, [routerModuleChange]);
}

const metadataChange = addSymbolToNgModuleMetadata(
moduleSource, modulePath, 'imports', 'RouterModule.forRoot(routes)');
if (metadataChange) {
metadataChange.forEach((change: InsertChange) => {
recorder.insertRight(change.pos, change.toAdd);
});
applyToUpdateRecorder(recorder, metadataChange);
}
host.commitUpdate(recorder);
}
Expand Down
9 changes: 8 additions & 1 deletion packages/schematics/angular/migrations/update-6/index.ts
Expand Up @@ -260,7 +260,7 @@ function extractProjectsConfig(
const outDir = app.outDir || defaults.outDir;
const appRoot = app.root || defaults.appRoot;

function _mapAssets(asset: string | JsonObject) {
function _mapAssets(asset: string | { glob?: string; input?: string; output?: string; allowOutsideOutDir?: boolean}) {
if (typeof asset === 'string') {
return normalize(appRoot + '/' + asset);
} else {
Expand All @@ -270,6 +270,13 @@ function extractProjectsConfig(
uses the 'allowOutsideOutDir' option which is not supported in Angular CLI 6.
`);

return null;
} else if (!asset.glob) {
logger.warn(tags.oneLine`
Asset with input '${asset.input}' was not migrated because it
does not contain a glob property.
`);

return null;
} else if (asset.output) {
return {
Expand Down
10 changes: 4 additions & 6 deletions packages/schematics/angular/service-worker/index.ts
Expand Up @@ -21,7 +21,7 @@ import {
import { NodePackageInstallTask } from '@angular-devkit/schematics/tasks';
import * as ts from '../third_party/github.com/Microsoft/TypeScript/lib/typescript';
import { addSymbolToNgModuleMetadata, getEnvironmentExportName, insertImport, isImported } from '../utility/ast-utils';
import { InsertChange } from '../utility/change';
import { applyToUpdateRecorder } from '../utility/change';
import { addPackageJsonDependency, getPackageJsonDependency } from '../utility/dependencies';
import { getAppModulePath } from '../utility/ng-ast-utils';
import { relativePathToWorkspaceRoot } from '../utility/paths';
Expand Down Expand Up @@ -63,7 +63,7 @@ function updateAppModule(mainPath: string): Rule {
const change = insertImport(moduleSource, modulePath, importModule, importPath);
if (change) {
const recorder = host.beginUpdate(modulePath);
recorder.insertLeft((change as InsertChange).pos, (change as InsertChange).toAdd);
applyToUpdateRecorder(recorder, [change]);
host.commitUpdate(recorder);
}
}
Expand All @@ -84,7 +84,7 @@ function updateAppModule(mainPath: string): Rule {
const change = insertImport(moduleSource, modulePath, importModule, importPath);
if (change) {
const recorder = host.beginUpdate(modulePath);
recorder.insertLeft((change as InsertChange).pos, (change as InsertChange).toAdd);
applyToUpdateRecorder(recorder, [change]);
host.commitUpdate(recorder);
}
}
Expand All @@ -97,9 +97,7 @@ function updateAppModule(mainPath: string): Rule {
moduleSource, modulePath, 'imports', importText);
if (metadataChanges) {
const recorder = host.beginUpdate(modulePath);
metadataChanges.forEach((change: InsertChange) => {
recorder.insertRight(change.pos, change.toAdd);
});
applyToUpdateRecorder(recorder, metadataChanges);
host.commitUpdate(recorder);
}

Expand Down
23 changes: 20 additions & 3 deletions packages/schematics/angular/utility/change.ts
Expand Up @@ -5,6 +5,8 @@
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import { UpdateRecorder } from '@angular-devkit/schematics';

export interface Host {
write(path: string, content: string): Promise<void>;
read(path: string): Promise<string>;
Expand Down Expand Up @@ -75,7 +77,7 @@ export class RemoveChange implements Change {
order: number;
description: string;

constructor(public path: string, private pos: number, private toRemove: string) {
constructor(public path: string, private pos: number, public toRemove: string) {
if (pos < 0) {
throw new Error('Negative positions are invalid');
}
Expand All @@ -101,8 +103,8 @@ export class ReplaceChange implements Change {
order: number;
description: string;

constructor(public path: string, private pos: number, private oldText: string,
private newText: string) {
constructor(public path: string, private pos: number, public oldText: string,
public newText: string) {
if (pos < 0) {
throw new Error('Negative positions are invalid');
}
Expand All @@ -125,3 +127,18 @@ export class ReplaceChange implements Change {
});
}
}

export function applyToUpdateRecorder(recorder: UpdateRecorder, changes: Change[]): void {
for (const change of changes) {
if (change instanceof InsertChange) {
recorder.insertLeft(change.pos, change.toAdd);
} else if (change instanceof RemoveChange) {
recorder.remove(change.order, change.toRemove.length);
} else if (change instanceof ReplaceChange) {
recorder.remove(change.order, change.oldText.length);
recorder.insertLeft(change.order, change.newText);
} else if (!(change instanceof NoopChange)) {
throw new Error('Unknown Change type encountered when updating a recorder.');
}
}
}
4 changes: 2 additions & 2 deletions packages/schematics/angular/utility/ng-ast-utils.ts
Expand Up @@ -62,11 +62,11 @@ export function findBootstrapModulePath(host: Tree, mainPath: string): string {
const source = ts.createSourceFile(mainPath, mainText, ts.ScriptTarget.Latest, true);
const allNodes = getSourceNodes(source);
const bootstrapModuleRelativePath = allNodes
.filter(node => node.kind === ts.SyntaxKind.ImportDeclaration)
.filter(ts.isImportDeclaration)
.filter(imp => {
return findNode(imp, ts.SyntaxKind.Identifier, bootstrapModule.getText());
})
.map((imp: ts.ImportDeclaration) => {
.map(imp => {
const modulePathStringLiteral = imp.moduleSpecifier as ts.StringLiteral;

return modulePathStringLiteral.text;
Expand Down
7 changes: 4 additions & 3 deletions packages/schematics/update/update/index.ts
Expand Up @@ -884,15 +884,15 @@ export default function(options: UpdateSchema): Rule {
)),

// Build a map of all dependencies and their packageJson.
reduce<NpmRepositoryPackageJson, Map<string, NpmRepositoryPackageJson>>(
reduce<Partial<NpmRepositoryPackageJson>, Map<string, NpmRepositoryPackageJson>>(
(acc, npmPackageJson) => {
// If the package was not found on the registry. It could be private, so we will just
// ignore. If the package was part of the list, we will error out, but will simply ignore
// if it's either not requested (so just part of package.json. silently) or if it's a
// `--all` situation. There is an edge case here where a public package peer depends on a
// private one, but it's rare enough.
if (!npmPackageJson.name) {
if (packages.has(npmPackageJson.requestedName)) {
if (npmPackageJson.requestedName && packages.has(npmPackageJson.requestedName)) {
if (options.all) {
logger.warn(`Package ${JSON.stringify(npmPackageJson.requestedName)} was not `
+ 'found on the registry. Skipping.');
Expand All @@ -903,7 +903,8 @@ export default function(options: UpdateSchema): Rule {
}
}
} else {
acc.set(npmPackageJson.name, npmPackageJson);
// If a name is present, it is assumed to be fully populated
acc.set(npmPackageJson.name, npmPackageJson as NpmRepositoryPackageJson);
}

return acc;
Expand Down