feat(migrations): Add migration to add explicit Route/Routes type (#45084)

Places that use `pathMatch` need an explicit `Route` or `Routes` type on
the variable so TypeScript does not infer the type as just 'string'.

PR Close #45084
This commit is contained in:
Andrew Scott 2021-10-28 17:17:09 -07:00 committed by Andrew Kushnir
parent 5aeaedfa03
commit d56a537196
15 changed files with 662 additions and 1 deletions

View file

@ -14,6 +14,7 @@ pkg_npm(
visibility = ["//packages/core:__pkg__"],
deps = [
"//packages/core/schematics/migrations/entry-components",
"//packages/core/schematics/migrations/path-match-type",
"//packages/core/schematics/migrations/typed-forms",
],
)

View file

@ -9,6 +9,11 @@
"version": "9999.0.0",
"description": "Experimental migration that adds <any>s for Typed Forms.",
"factory": "./migrations/typed-forms/index"
},
"migration-v14-path-match-type": {
"version": "14.0.0-beta",
"description": "In Angular version 14, the `pathMatch` property of `Routes` was updated to be a strict union of the two valid options: `'full'|'prefix'`. `Routes` and `Route` variables need an explicit type so TypeScript does not infer the property as the looser `string`.",
"factory": "./migrations/path-match-type/index"
}
}
}
}

View file

@ -7,6 +7,8 @@ ts_library(
visibility = ["//packages/core/schematics/test/google3:__pkg__"],
deps = [
"//packages/core/schematics/migrations/entry-components",
"//packages/core/schematics/migrations/path-match-type",
"//packages/core/schematics/migrations/path-match-type/google3",
"//packages/core/schematics/migrations/typed-forms",
"//packages/core/schematics/utils",
"//packages/core/schematics/utils/tslint",

View file

@ -0,0 +1,48 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* 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 {RuleFailure, Rules} from 'tslint';
import ts from 'typescript';
import {TslintUpdateRecorder} from '../path-match-type/google3/tslint_update_recorder';
import {PathMatchTypeTransform} from '../path-match-type/transform';
/**
* TSLint rule that updates return value for guards that return UrlTree.
*/
export class Rule extends Rules.TypedRule {
override applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): RuleFailure[] {
const ruleName = this.ruleName;
const sourceFiles = program.getSourceFiles().filter(
s => !s.isDeclarationFile && !program.isSourceFileFromExternalLibrary(s));
const updateRecorders = new Map<ts.SourceFile, TslintUpdateRecorder>();
const transform = new PathMatchTypeTransform(getUpdateRecorder);
// Migrate all source files in the project.
transform.migrate(sourceFiles);
// Record the changes collected in the import manager.
transform.recordChanges();
if (updateRecorders.has(sourceFile)) {
return updateRecorders.get(sourceFile)!.failures;
}
return [];
/** Gets the update recorder for the specified source file. */
function getUpdateRecorder(sourceFile: ts.SourceFile): TslintUpdateRecorder {
if (updateRecorders.has(sourceFile)) {
return updateRecorders.get(sourceFile)!;
}
const printer = ts.createPrinter();
const recorder = new TslintUpdateRecorder(ruleName, sourceFile, printer);
updateRecorders.set(sourceFile, recorder);
return recorder;
}
}
}

View file

@ -0,0 +1,19 @@
load("//tools:defaults.bzl", "ts_library")
ts_library(
name = "path-match-type",
srcs = glob(["**/*.ts"]),
tsconfig = "//packages/core/schematics:tsconfig.json",
visibility = [
"//packages/core/schematics:__pkg__",
"//packages/core/schematics/migrations/google3:__pkg__",
"//packages/core/schematics/migrations/path-match-type/google3:__pkg__",
"//packages/core/schematics/test:__pkg__",
],
deps = [
"//packages/core/schematics/utils",
"@npm//@angular-devkit/schematics",
"@npm//@types/node",
"@npm//typescript",
],
)

View file

@ -0,0 +1,32 @@
## Route.pathMatch type changing from `string` `'full'|'prefix'`
This migration updates the type of `Route` or `Routes` which use `patchMatch` to be
explicit so they conform the new strict type of the property. The explicit `Route`/`Routes`
type is necessary because otherwise TypeScript will consider the type of `pathMatch`
to be `string`.
#### Before
```ts
import { NgModule } from '@angular/core';
import { RouterModule } from '@angular/router';
const routes = [{path: '', pathMatch: 'full', redirectTo: 'home'}]
@NgModule({
imports: [RouterModule.forRoot(routes)]
})
export const RoutingModule {}
```
#### After
```ts
import { NgModule } from '@angular/core';
import { RouterModule, Routes } from '@angular/router';
const routes: Routes = [{path: '', pathMatch: 'full', redirectTo: 'home'}]
@NgModule({
imports: [RouterModule.forRoot(routes)]
})
export const RoutingModule {}
```

View file

@ -0,0 +1,13 @@
load("//tools:defaults.bzl", "ts_library")
ts_library(
name = "google3",
srcs = glob(["**/*.ts"]),
tsconfig = "//packages/core/schematics:tsconfig.json",
visibility = ["//packages/core/schematics/migrations/google3:__pkg__"],
deps = [
"//packages/core/schematics/migrations/path-match-type",
"@npm//tslint",
"@npm//typescript",
],
)

View file

@ -0,0 +1,45 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* 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 {Replacement, RuleFailure} from 'tslint';
import ts from 'typescript';
import {UpdateRecorder} from '../update_recorder';
export class TslintUpdateRecorder implements UpdateRecorder {
failures: RuleFailure[] = [];
constructor(
private ruleName: string, private sourceFile: ts.SourceFile, private printer: ts.Printer) {}
updateNode(
oldNode: ts.VariableDeclaration, newNode: ts.VariableDeclaration, sourceFile: ts.SourceFile) {
const newText =
': ' + this.printer.printNode(ts.EmitHint.Unspecified, newNode.type!, sourceFile);
this.failures.push(new RuleFailure(
this.sourceFile, oldNode.name.getEnd(), 0, 'Must use explicit `Route`/`Routes` type.',
this.ruleName, new Replacement(oldNode.name.getEnd(), 0, newText)));
}
/** Adds the specified import to the source file at the given position */
addNewImport(start: number, importText: string) {
this.failures.push(new RuleFailure(
this.sourceFile, start, 0, `Source file needs to have import: "${importText}"`,
this.ruleName, Replacement.appendText(start, importText)));
}
/** Updates existing named imports to the given new named imports. */
updateExistingImport(namedBindings: ts.NamedImports, newNamedBindings: string): void {
this.failures.push(new RuleFailure(
this.sourceFile, namedBindings.getStart(), 0,
`Import needs to be updated to import symbols: "${newNamedBindings}"`, this.ruleName,
new Replacement(namedBindings.getStart(), namedBindings.getWidth(), newNamedBindings)));
}
commitUpdate() {}
}

View file

@ -0,0 +1,85 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* 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 {Rule, SchematicsException, Tree} from '@angular-devkit/schematics';
import {relative} from 'path';
import ts from 'typescript';
import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths';
import {canMigrateFile, createMigrationProgram} from '../../utils/typescript/compiler_host';
import {PathMatchTypeTransform} from './transform';
import {UpdateRecorder} from './update_recorder';
/** Migration that adds explicit type to `Route`/`Routes` which use `patchMatch` */
export default function(): Rule {
return async (tree: Tree) => {
const {buildPaths, testPaths} = await getProjectTsConfigPaths(tree);
const basePath = process.cwd();
const allPaths = [...buildPaths, ...testPaths];
if (!allPaths.length) {
throw new SchematicsException(
'Could not find any tsconfig file. Cannot perform pathMatch migration.');
}
for (const tsconfigPath of allPaths) {
runMigration(tree, tsconfigPath, basePath);
}
};
}
function runMigration(tree: Tree, tsconfigPath: string, basePath: string): void {
const {program} = createMigrationProgram(tree, tsconfigPath, basePath);
const sourceFiles =
program.getSourceFiles().filter(sourceFile => canMigrateFile(basePath, sourceFile, program));
const updateRecorders = new Map<ts.SourceFile, UpdateRecorder>();
const transform = new PathMatchTypeTransform(getUpdateRecorder);
// Migrate all source files in the project.
transform.migrate(sourceFiles);
// Record the changes collected in the import manager.
transform.recordChanges();
// Walk through each update recorder and commit the update. We need to commit the
// updates in batches per source file as there can be only one recorder per source
// file in order to avoid shifted character offsets.
updateRecorders.forEach(recorder => recorder.commitUpdate());
/** Gets the update recorder for the specified source file. */
function getUpdateRecorder(sourceFile: ts.SourceFile): UpdateRecorder {
if (updateRecorders.has(sourceFile)) {
return updateRecorders.get(sourceFile)!;
}
const printer = ts.createPrinter();
const treeRecorder = tree.beginUpdate(relative(basePath, sourceFile.fileName));
const recorder: UpdateRecorder = {
updateNode(oldExpr: ts.VariableDeclaration, newExpr: ts.VariableDeclaration) {
treeRecorder.insertRight(
oldExpr.name.getEnd(),
': ' + printer.printNode(ts.EmitHint.Unspecified, newExpr.type!, sourceFile));
},
addNewImport(start: number, importText: string) {
// New imports should be inserted at the left while decorators should be inserted
// at the right in order to ensure that imports are inserted before the decorator
// if the start position of import and decorator is the source file start.
treeRecorder.insertLeft(start, importText);
},
updateExistingImport(namedBindings: ts.NamedImports, newNamedBindings: string) {
treeRecorder.remove(namedBindings.getStart(), namedBindings.getWidth());
treeRecorder.insertRight(namedBindings.getStart(), newNamedBindings);
},
commitUpdate() {
tree.commitUpdate(treeRecorder);
}
};
updateRecorders.set(sourceFile, recorder);
return recorder;
}
}

View file

@ -0,0 +1,36 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* 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 ts from 'typescript';
import {ImportManager} from '../../utils/import_manager';
import {UpdateRecorder} from './update_recorder';
import {findExpressionsToMigrate} from './util';
export class PathMatchTypeTransform {
private printer = ts.createPrinter();
private importManager = new ImportManager(this.getUpdateRecorder, this.printer);
constructor(private getUpdateRecorder: (sf: ts.SourceFile) => UpdateRecorder) {}
migrate(sourceFiles: ts.SourceFile[]): void {
for (const sourceFile of sourceFiles) {
const toMigrate = findExpressionsToMigrate(sourceFile, this.importManager);
const recorder = this.getUpdateRecorder(sourceFile);
for (const [oldNode, newNode] of toMigrate) {
recorder.updateNode(oldNode, newNode, sourceFile);
}
}
}
/** Records all changes that were made in the import manager. */
recordChanges() {
this.importManager.recordChanges();
}
}

View file

@ -0,0 +1,22 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* 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 ts from 'typescript';
import {ImportManagerUpdateRecorder} from '../../utils/import_manager';
/**
* Update recorder interface that is used to transform source files
* in a non-colliding way.
*/
export interface UpdateRecorder extends ImportManagerUpdateRecorder {
updateNode(
oldNode: ts.VariableDeclaration, newNode: ts.VariableDeclaration,
sourceFile: ts.SourceFile): void;
commitUpdate(): void;
}

View file

@ -0,0 +1,98 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* 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 ts from 'typescript';
import {ImportManager} from '../../utils/import_manager';
export function findExpressionsToMigrate(sourceFile: ts.SourceFile, importManager: ImportManager) {
const migratedNodesMap: Map<ts.VariableDeclaration, ts.VariableDeclaration> = new Map();
let _currentVariableDecl: ts.VariableDeclaration|null = null;
(() => {
sourceFile.forEachChild(function visitNode(node: ts.Node) {
if (ts.isVariableDeclaration(node)) {
_currentVariableDecl = node;
node.forEachChild(visitNode);
_currentVariableDecl = null;
}
if (isRouteOrRoutesVariableDeclaration(node)) {
// The variable declaration is already explicitly typed as `Route` or `Routes` so it does
// not need a migration.
return;
} else if (ts.isObjectLiteralExpression(node)) {
if (_currentVariableDecl !== null && _currentVariableDecl.type === undefined) {
visitObjectLiteral(node);
}
} else {
node.forEachChild(visitNode);
}
});
function visitObjectLiteral(obj: ts.ObjectLiteralExpression) {
const hasPathMatch = obj.properties.some(p => isPropertyWithName(p, 'pathMatch'));
const hasPath = obj.properties.some(p => isPropertyWithName(p, 'path'));
const childrenProperty = obj.properties.find(p => isPropertyWithName(p, 'children'));
// The object must have _both_ pathMatch _and_ path for us to be reasonably sure that it's
// a `Route` definition.
if (hasPath && hasPathMatch) {
updateCurrentVariableDeclaration();
} else if (
childrenProperty !== undefined && ts.isPropertyAssignment(childrenProperty) &&
ts.isArrayLiteralExpression(childrenProperty.initializer)) {
// Also need to check the children if it exists
for (const child of childrenProperty.initializer.elements) {
if (ts.isObjectLiteralExpression(child)) {
visitObjectLiteral(child);
// If the child caused a migration, we can exit early
if (_currentVariableDecl && migratedNodesMap.has(_currentVariableDecl)) {
break;
}
}
}
}
}
function isPropertyWithName(p: ts.ObjectLiteralElementLike, name: string) {
if (ts.isPropertyAssignment(p)) {
return p.name.getText() === name;
} else if (ts.isShorthandPropertyAssignment(p)) {
return p.name.getText() === name;
} else {
// Don't attempt to migrate edge case spreadAssignment
return false;
}
}
function updateCurrentVariableDeclaration() {
if (_currentVariableDecl === null || _currentVariableDecl.initializer === undefined) {
return;
}
let typeToUse: ts.TypeNode;
if (ts.isArrayLiteralExpression(_currentVariableDecl.initializer)) {
typeToUse = importManager.addImportToSourceFile(sourceFile, 'Routes', '@angular/router') as
unknown as ts.TypeNode;
} else {
typeToUse = importManager.addImportToSourceFile(sourceFile, 'Route', '@angular/router') as
unknown as ts.TypeNode;
}
const migrated = ts.factory.updateVariableDeclaration(
_currentVariableDecl, _currentVariableDecl.name, _currentVariableDecl.exclamationToken,
typeToUse, _currentVariableDecl.initializer);
migratedNodesMap.set(_currentVariableDecl, migrated);
}
})();
return migratedNodesMap;
}
function isRouteOrRoutesVariableDeclaration(node: ts.Node) {
return ts.isVariableDeclaration(node) && node.type &&
(node.type.getText() === 'Route' || node.type.getText() === 'Routes');
}

View file

@ -9,6 +9,7 @@ ts_library(
],
deps = [
"//packages/core/schematics/migrations/entry-components",
"//packages/core/schematics/migrations/path-match-type",
"//packages/core/schematics/migrations/typed-forms",
"//packages/core/schematics/utils",
"@npm//@angular-devkit/core",

View file

@ -0,0 +1,129 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* 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 {readFileSync, writeFileSync} from 'fs';
import {dirname, join} from 'path';
import * as shx from 'shelljs';
import {Configuration, Linter} from 'tslint';
describe('Google3 path match type', () => {
const rulesDirectory = dirname(require.resolve('../../migrations/google3/pathMatchTypeRule'));
let tmpDir: string;
beforeEach(() => {
tmpDir = join(process.env['TEST_TMPDIR']!, 'google3-test');
shx.mkdir('-p', tmpDir);
// We need to declare the Angular symbols we're testing for, otherwise type checking won't work.
writeFile('router.d.ts', `
export declare class UrlTree {
}
`);
writeFile('rxjs.d.ts', `
export declare class Observable<T>{}
`);
writeFile('operators.d.ts', `
export declare function map(): void;
`);
writeFile('tsconfig.json', JSON.stringify({
compilerOptions: {
module: 'es2015',
baseUrl: './',
},
}));
});
afterEach(() => shx.rm('-r', tmpDir));
function runTSLint(fix: boolean) {
const program = Linter.createProgram(join(tmpDir, 'tsconfig.json'));
const linter = new Linter({fix, rulesDirectory: [rulesDirectory]}, program);
const config = Configuration.parseConfigFile({rules: {'path-match-type': true}});
program.getRootFileNames().forEach(fileName => {
linter.lint(fileName, program.getSourceFile(fileName)!.getFullText(), config);
});
return linter;
}
function writeFile(fileName: string, content: string) {
writeFileSync(join(tmpDir, fileName), content);
}
function getFile(fileName: string) {
return readFileSync(join(tmpDir, fileName), 'utf8');
}
it('should migrate Route literal', async () => {
writeFile('/index.ts', `
const route = {path: 'abc', pathMatch: 'full'};
`);
runTSLint(true);
const content = getFile('/index.ts');
expect(content).toContain(`import { Route } from "@angular/router";`);
expect(content).toContain(`const route: Route = {path: 'abc', pathMatch: 'full'};`);
});
it('should migrate Routes literal', async () => {
writeFile('/index.ts', `
const routes = [{path: 'abc', pathMatch: 'full'}];
`);
runTSLint(true);
const content = getFile('/index.ts');
expect(content).toContain(`import { Routes } from "@angular/router";`);
expect(content).toContain(`const routes: Routes = [{path: 'abc', pathMatch: 'full'}];`);
});
it('should migrate Routes with children', async () => {
writeFile('/index.ts', `
const routes = [{path: 'home', children: [{path: 'abc', pathMatch: 'full'}]}];
`);
runTSLint(true);
const content = getFile('/index.ts');
expect(content).toContain(
`const routes: Routes = [{path: 'home', children: [{path: 'abc', pathMatch: 'full'}]}];`);
});
it('should NOT migrate Route if it already has an explicit type', async () => {
writeFile('/index.ts', `
export interface OtherType {}
const routes: OtherType = {path: 'abc', pathMatch: 'full'};
`);
runTSLint(true);
const content = getFile('/index.ts');
expect(content).toContain(`const routes: OtherType = {path: 'abc', pathMatch: 'full'};`);
});
it('should NOT migrate Routes if it already has an explicit type', async () => {
writeFile('/index.ts', `
export interface OtherType {}
const routes: OtherType = [{path: 'abc', pathMatch: 'full'}];
`);
runTSLint(true);
const content = getFile('/index.ts');
expect(content).toContain(`const routes: OtherType = [{path: 'abc', pathMatch: 'full'}];`);
});
});

View file

@ -0,0 +1,125 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* 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 {getSystemPath, normalize, virtualFs} from '@angular-devkit/core';
import {TempScopedNodeJsSyncHost} from '@angular-devkit/core/node/testing';
import {HostTree} from '@angular-devkit/schematics';
import {SchematicTestRunner, UnitTestTree} from '@angular-devkit/schematics/testing';
import * as shx from 'shelljs';
describe('PathMatch type migration', () => {
let runner: SchematicTestRunner;
let host: TempScopedNodeJsSyncHost;
let tree: UnitTestTree;
let tmpDirPath: string;
let previousWorkingDir: string;
beforeEach(() => {
runner = new SchematicTestRunner('test', require.resolve('../migrations.json'));
host = new TempScopedNodeJsSyncHost();
tree = new UnitTestTree(new HostTree(host));
writeFile('/tsconfig.json', JSON.stringify({
compilerOptions: {
lib: ['es2015'],
strictNullChecks: true,
},
}));
writeFile('/angular.json', JSON.stringify({
version: 1,
projects: {t: {architect: {build: {options: {tsConfig: './tsconfig.json'}}}}}
}));
// We need to declare the Angular symbols we're testing for, otherwise type checking won't work.
writeFile('/node_modules/@angular/router/index.d.ts', `
export declare interface Route {
}
export declare interface Routes {
}
`);
previousWorkingDir = shx.pwd();
tmpDirPath = getSystemPath(host.root);
// Switch into the temporary directory path. This allows us to run
// the schematic against our custom unit test tree.
shx.cd(tmpDirPath);
});
afterEach(() => {
shx.cd(previousWorkingDir);
shx.rm('-r', tmpDirPath);
});
it('should migrate Route literal', async () => {
writeFile('/index.ts', `
const route = {path: 'abc', pathMatch: 'full'};
`);
await runMigration();
const content = tree.readContent('/index.ts');
expect(content).toContain(`import { Route } from "@angular/router";`);
expect(content).toContain(`const route: Route = {path: 'abc', pathMatch: 'full'};`);
});
it('should migrate Routes literal', async () => {
writeFile('/index.ts', `
const routes = [{path: 'abc', pathMatch: 'full'}];
`);
await runMigration();
const content = tree.readContent('/index.ts');
expect(content).toContain(`import { Routes } from "@angular/router";`);
expect(content).toContain(`const routes: Routes = [{path: 'abc', pathMatch: 'full'}];`);
});
it('should migrate Routes with children', async () => {
writeFile('/index.ts', `
const routes = [{path: 'home', children: [{path: 'abc', pathMatch: 'full'}]}];
`);
await runMigration();
const content = tree.readContent('/index.ts');
expect(content).toContain(
`const routes: Routes = [{path: 'home', children: [{path: 'abc', pathMatch: 'full'}]}];`);
});
it('should NOT migrate Route if it already has an explicit type', async () => {
writeFile('/index.ts', `
export interface OtherType {}
const routes: OtherType = {path: 'abc', pathMatch: 'full'};
`);
await runMigration();
const content = tree.readContent('/index.ts');
expect(content).toContain(`const routes: OtherType = {path: 'abc', pathMatch: 'full'};`);
});
it('should NOT migrate Routes if it already has an explicit type', async () => {
writeFile('/index.ts', `
export interface OtherType {}
const routes: OtherType = [{path: 'abc', pathMatch: 'full'}];
`);
await runMigration();
const content = tree.readContent('/index.ts');
expect(content).toContain(`const routes: OtherType = [{path: 'abc', pathMatch: 'full'}];`);
});
function writeFile(filePath: string, contents: string) {
host.sync.write(normalize(filePath), virtualFs.stringToFileBuffer(contents));
}
function runMigration() {
return runner.runSchematicAsync('migration-v14-path-match-type', {}, tree).toPromise();
}
});