From 93ee05d92af53dfc9028b14fbe71a1a115387268 Mon Sep 17 00:00:00 2001 From: Joey Perrott Date: Mon, 12 Oct 2020 11:13:57 -0700 Subject: [PATCH] fix(router): create schematic for preserveQueryParams (#38762) Create a schematic for migrating preserveQueryParams to use queryParamsHandler instead. PR Close #38762 --- packages/core/schematics/BUILD.bazel | 1 + packages/core/schematics/migrations.json | 5 + .../router-preserve-query-params/BUILD.bazel | 18 ++ .../router-preserve-query-params/README.md | 35 +++ .../router-preserve-query-params/index.ts | 64 +++++ .../router-preserve-query-params/util.ts | 105 ++++++++ packages/core/schematics/test/BUILD.bazel | 1 + .../preserve_query_params_migration_spec.ts | 232 ++++++++++++++++++ 8 files changed, 461 insertions(+) create mode 100644 packages/core/schematics/migrations/router-preserve-query-params/BUILD.bazel create mode 100644 packages/core/schematics/migrations/router-preserve-query-params/README.md create mode 100644 packages/core/schematics/migrations/router-preserve-query-params/index.ts create mode 100644 packages/core/schematics/migrations/router-preserve-query-params/util.ts create mode 100644 packages/core/schematics/test/preserve_query_params_migration_spec.ts diff --git a/packages/core/schematics/BUILD.bazel b/packages/core/schematics/BUILD.bazel index 2817d90c8b..c58ce9ba1c 100644 --- a/packages/core/schematics/BUILD.bazel +++ b/packages/core/schematics/BUILD.bazel @@ -19,6 +19,7 @@ pkg_npm( "//packages/core/schematics/migrations/navigation-extras-omissions", "//packages/core/schematics/migrations/relative-link-resolution", "//packages/core/schematics/migrations/renderer-to-renderer2", + "//packages/core/schematics/migrations/router-preserve-query-params", "//packages/core/schematics/migrations/static-queries", "//packages/core/schematics/migrations/template-var-assignment", "//packages/core/schematics/migrations/undecorated-classes-with-decorated-fields", diff --git a/packages/core/schematics/migrations.json b/packages/core/schematics/migrations.json index 60c47e1e6a..5f5a9f3e70 100644 --- a/packages/core/schematics/migrations.json +++ b/packages/core/schematics/migrations.json @@ -69,6 +69,11 @@ "version": "11.0.0-beta", "description": "`async` to `waitForAsync` migration. The `async` testing function has been renamed to `waitForAsync` to avoid confusion with the native `async` keyword.", "factory": "./migrations/wait-for-async/index" + }, + "migration-v11-router-preserve-query-params": { + "version": "11.0.0-beta", + "description": "NavigationExtras.preserveQueryParams has been removed as of Angular version 11. This migration replaces any usages with the appropriate assignment of the queryParamsHandler key.", + "factory": "./migrations/router-preserve-query-params/index" } } } diff --git a/packages/core/schematics/migrations/router-preserve-query-params/BUILD.bazel b/packages/core/schematics/migrations/router-preserve-query-params/BUILD.bazel new file mode 100644 index 0000000000..58d793e1a6 --- /dev/null +++ b/packages/core/schematics/migrations/router-preserve-query-params/BUILD.bazel @@ -0,0 +1,18 @@ +load("//tools:defaults.bzl", "ts_library") + +ts_library( + name = "router-preserve-query-params", + srcs = glob(["**/*.ts"]), + tsconfig = "//packages/core/schematics:tsconfig.json", + visibility = [ + "//packages/core/schematics:__pkg__", + "//packages/core/schematics/migrations/google3:__pkg__", + "//packages/core/schematics/test:__pkg__", + ], + deps = [ + "//packages/core/schematics/utils", + "@npm//@angular-devkit/schematics", + "@npm//@types/node", + "@npm//typescript", + ], +) diff --git a/packages/core/schematics/migrations/router-preserve-query-params/README.md b/packages/core/schematics/migrations/router-preserve-query-params/README.md new file mode 100644 index 0000000000..125d39c625 --- /dev/null +++ b/packages/core/schematics/migrations/router-preserve-query-params/README.md @@ -0,0 +1,35 @@ +## Router's NavigationExtras.preserveQueryParams migration + +Previously the `NatigationExtras` property of `preserveQueryParams` defined what should be done with +query parameters on navigation. This migration updates the usages of `preserveQueryParams` to +instead use the `queryParamsHandler` property. + +#### Before +```ts +import { Component } from '@angular/core'; +import { Router } from '@angular/router'; + +@Component({}) +export class MyComponent { + constructor(private _router: Router) {} + + goHome() { + this._router.navigate('/', {preserveQueryParams: true, skipLocationChange: 'foo'}); + } +} +``` + +#### After +```ts +import { Component } from '@angular/core'; +import { Router } from '@angular/router'; + +@Component({}) +export class MyComponent { + constructor(private _router: Router) {} + + goHome() { + this._router.navigate('/', { queryParamsHandler: 'preserve', skipLocationChange: 'foo' }); + } +} +``` diff --git a/packages/core/schematics/migrations/router-preserve-query-params/index.ts b/packages/core/schematics/migrations/router-preserve-query-params/index.ts new file mode 100644 index 0000000000..973337f72c --- /dev/null +++ b/packages/core/schematics/migrations/router-preserve-query-params/index.ts @@ -0,0 +1,64 @@ +/** + * @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 * as ts from 'typescript'; + +import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths'; +import {createMigrationProgram} from '../../utils/typescript/compiler_host'; +import {findLiteralsToMigrate, migrateLiteral} from './util'; + + +/** + * Migration that switches `NavigationExtras.preserveQueryParams` to set the coresponding value via + * `NavigationExtras`'s `queryParamsHandling` attribute. + */ +export default function(): Rule { + return (tree: Tree) => { + const {buildPaths, testPaths} = getProjectTsConfigPaths(tree); + const basePath = process.cwd(); + const allPaths = [...buildPaths, ...testPaths]; + + if (!allPaths.length) { + throw new SchematicsException( + 'Could not find any tsconfig file. Cannot migrate ' + + 'NavigationExtras.preserveQueryParams usages.'); + } + + for (const tsconfigPath of allPaths) { + runPreserveQueryParamsMigration(tree, tsconfigPath, basePath); + } + }; +} + +function runPreserveQueryParamsMigration(tree: Tree, tsconfigPath: string, basePath: string) { + const {program} = createMigrationProgram(tree, tsconfigPath, basePath); + const typeChecker = program.getTypeChecker(); + const printer = ts.createPrinter(); + const sourceFiles = program.getSourceFiles().filter( + f => !f.isDeclarationFile && !program.isSourceFileFromExternalLibrary(f)); + + sourceFiles.forEach(sourceFile => { + const literalsToMigrate = findLiteralsToMigrate(sourceFile, typeChecker); + const update = tree.beginUpdate(relative(basePath, sourceFile.fileName)); + + literalsToMigrate.forEach((instances, methodName) => instances.forEach(instance => { + const migratedNode = migrateLiteral(methodName, instance); + + if (migratedNode !== instance) { + update.remove(instance.getStart(), instance.getWidth()); + update.insertRight( + instance.getStart(), + printer.printNode(ts.EmitHint.Unspecified, migratedNode, sourceFile)); + } + })); + + tree.commitUpdate(update); + }); +} diff --git a/packages/core/schematics/migrations/router-preserve-query-params/util.ts b/packages/core/schematics/migrations/router-preserve-query-params/util.ts new file mode 100644 index 0000000000..3a4efc28e6 --- /dev/null +++ b/packages/core/schematics/migrations/router-preserve-query-params/util.ts @@ -0,0 +1,105 @@ +/** + * @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 * as ts from 'typescript'; + +import {getImportSpecifier} from '../../utils/typescript/imports'; +import {isReferenceToImport} from '../../utils/typescript/symbol'; + +/** + * Configures the methods that the migration should be looking for + * and the properties from `NavigationExtras` that should be preserved. + */ +const methodConfig = new Set(['navigate', 'createUrlTree']); + +const preserveQueryParamsKey = 'preserveQueryParams'; + +export function migrateLiteral( + methodName: string, node: ts.ObjectLiteralExpression): ts.ObjectLiteralExpression { + const isMigratableMethod = methodConfig.has(methodName); + + if (!isMigratableMethod) { + throw Error(`Attempting to migrate unconfigured method called ${methodName}.`); + } + + + const propertiesToKeep: ts.ObjectLiteralElementLike[] = []; + let propertyToMigrate: ts.PropertyAssignment|ts.ShorthandPropertyAssignment|undefined = undefined; + + for (const property of node.properties) { + // Only look for regular and shorthand property assignments since resolving things + // like spread operators becomes too complicated for this migration. + if ((ts.isPropertyAssignment(property) || ts.isShorthandPropertyAssignment(property)) && + (ts.isStringLiteralLike(property.name) || ts.isNumericLiteral(property.name) || + ts.isIdentifier(property.name)) && + (property.name.text === preserveQueryParamsKey)) { + propertyToMigrate = property; + continue; + } + propertiesToKeep.push(property); + } + + // Don't modify the node if there's nothing to migrate. + if (propertyToMigrate === undefined) { + return node; + } + + if ((ts.isShorthandPropertyAssignment(propertyToMigrate) && + propertyToMigrate.objectAssignmentInitializer?.kind === ts.SyntaxKind.TrueKeyword) || + (ts.isPropertyAssignment(propertyToMigrate) && + propertyToMigrate.initializer.kind === ts.SyntaxKind.TrueKeyword)) { + return ts.updateObjectLiteral( + node, + propertiesToKeep.concat( + ts.createPropertyAssignment('queryParamsHandler', ts.createIdentifier(`'preserve'`)))); + } + + return ts.updateObjectLiteral(node, propertiesToKeep); +} + +export function findLiteralsToMigrate(sourceFile: ts.SourceFile, typeChecker: ts.TypeChecker) { + const results = new Map>( + Array.from(methodConfig.keys(), key => [key, new Set()])); + const routerImport = getImportSpecifier(sourceFile, '@angular/router', 'Router'); + const seenLiterals = new Map(); + + if (routerImport) { + sourceFile.forEachChild(function visitNode(node: ts.Node) { + // Look for calls that look like `foo.` with more than one parameter. + if (ts.isCallExpression(node) && node.arguments.length > 1 && + ts.isPropertyAccessExpression(node.expression) && ts.isIdentifier(node.expression.name) && + methodConfig.has(node.expression.name.text)) { + // Check whether the type of the object on which the + // function is called refers to the Router import. + if (isReferenceToImport(typeChecker, node.expression.expression, routerImport)) { + const methodName = node.expression.name.text; + const parameterDeclaration = + typeChecker.getTypeAtLocation(node.arguments[1]).getSymbol()?.valueDeclaration; + + // Find the source of the object literal. + if (parameterDeclaration && ts.isObjectLiteralExpression(parameterDeclaration)) { + if (!seenLiterals.has(parameterDeclaration)) { + results.get(methodName)!.add(parameterDeclaration); + seenLiterals.set(parameterDeclaration, methodName); + // If the same literal has been passed into multiple different methods, we can't + // migrate it, because the supported properties are different. When we detect such + // a case, we drop it from the results so that it gets ignored. If it's used multiple + // times for the same method, it can still be migrated. + } else if (seenLiterals.get(parameterDeclaration) !== methodName) { + results.forEach(literals => literals.delete(parameterDeclaration)); + } + } + } + } else { + node.forEachChild(visitNode); + } + }); + } + + return results; +} diff --git a/packages/core/schematics/test/BUILD.bazel b/packages/core/schematics/test/BUILD.bazel index e03a2c20ff..6825f20bda 100644 --- a/packages/core/schematics/test/BUILD.bazel +++ b/packages/core/schematics/test/BUILD.bazel @@ -17,6 +17,7 @@ ts_library( "//packages/core/schematics/migrations/navigation-extras-omissions", "//packages/core/schematics/migrations/relative-link-resolution", "//packages/core/schematics/migrations/renderer-to-renderer2", + "//packages/core/schematics/migrations/router-preserve-query-params", "//packages/core/schematics/migrations/static-queries", "//packages/core/schematics/migrations/template-var-assignment", "//packages/core/schematics/migrations/undecorated-classes-with-decorated-fields", diff --git a/packages/core/schematics/test/preserve_query_params_migration_spec.ts b/packages/core/schematics/test/preserve_query_params_migration_spec.ts new file mode 100644 index 0000000000..264fa4aa4b --- /dev/null +++ b/packages/core/schematics/test/preserve_query_params_migration_spec.ts @@ -0,0 +1,232 @@ +/** + * @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('NavigationExtras preserveQueryParams 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({ + 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 class Router { + navigate(url: string, extras?: any); + createUrlTree(commands: any[], extras?: any); + } + `); + + 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); + }); + + describe('updates the', () => { + it('`navigate` function', async () => { + writeFile('/index.ts', ` + import {Router} from '@angular/router'; + + class Navigator { + constructor(private _router: Router) {} + + goHome() { + this._router.navigate('/', {preserveQueryParams: true}); + } + } + `); + + await runMigration(); + + const content = tree.readContent('/index.ts'); + expect(content).toContain(`this._router.navigate('/', { queryParamsHandler: 'preserve' });`); + }); + + it('`createUrlTree` function', async () => { + writeFile('/index.ts', ` + import {Router} from '@angular/router'; + + class Navigator { + constructor(private _router: Router) {} + + goHome() { + this._router.createUrlTree('/', {preserveQueryParams: false}); + } + } + `); + + await runMigration(); + + const content = tree.readContent('/index.ts'); + expect(content).toContain(`this._router.createUrlTree('/', {});`); + }); + }); + + describe('updates an object which is used for the parameter', () => { + it('should migrate when the value is `true`', async () => { + writeFile('/index.ts', ` + import {Router} from '@angular/router'; + + const config = {preserveQueryParams: true, replaceUrl: true, fragment: 'foo', state: {}}; + + class Navigator { + constructor(private _router: Router) {} + + goHome() { + this._router.createUrlTree(['/'], config); + } + } + `); + + await runMigration(); + + const content = tree.readContent('/index.ts'); + expect(content).toContain( + `const config = { replaceUrl: true, fragment: 'foo', state: {}, queryParamsHandler: 'preserve' };`); + }); + + it('should remove when the value is `false`', async () => { + writeFile('/index.ts', ` + import {Router} from '@angular/router'; + + const config = {preserveQueryParams: false, replaceUrl: true, fragment: 'foo', state: {}}; + + class Navigator { + constructor(private _router: Router) {} + + goHome() { + this._router.createUrlTree(['/'], config); + } + } + `); + + await runMigration(); + + const content = tree.readContent('/index.ts'); + expect(content).toContain(`const config = { replaceUrl: true, fragment: 'foo', state: {} };`); + }); + + it('should not modify when the property is no present', async () => { + writeFile('/index.ts', ` + import {Router} from '@angular/router'; + + const config = {replaceUrl: true, fragment: 'foo', state: {}}; + + class Navigator { + constructor(private _router: Router) {} + + goHome() { + this._router.navigate('/', config); + } + } + `); + + await runMigration(); + + const content = tree.readContent('/index.ts'); + expect(content).toContain(`const config = {replaceUrl: true, fragment: 'foo', state: {}};`); + }); + }); + + describe('updates an the locally defined parameter in the method', () => { + it('should migrate when the value is `true`', async () => { + writeFile('/index.ts', ` + import {Router} from '@angular/router'; + + class Navigator { + constructor(private _router: Router) {} + + goHome() { + this._router.navigate('/', {preserveQueryParams: true, replaceUrl: true, fragment: 'foo', state: {}}); + } + } + `); + + await runMigration(); + + const content = tree.readContent('/index.ts'); + expect(content).toContain( + `this._router.navigate('/', { replaceUrl: true, fragment: 'foo', state: {}, queryParamsHandler: 'preserve' });`); + }); + + it('should remove when the value is `false`', async () => { + writeFile('/index.ts', ` + import {Router} from '@angular/router'; + + class Navigator { + constructor(private _router: Router) {} + + goHome() { + this._router.createUrlTree(['/'], {preserveQueryParams: false, replaceUrl: true, fragment: 'foo', state: {}};); + } + } + `); + + await runMigration(); + + const content = tree.readContent('/index.ts'); + expect(content).toContain( + `this._router.createUrlTree(['/'], { replaceUrl: true, fragment: 'foo', state: {} };`); + }); + + it('should not modify when the property is not present', async () => { + writeFile('/index.ts', ` + import {Router} from '@angular/router'; + + class Navigator { + constructor(private _router: Router) {} + + goHome() { + this._router.navigate('/', {replaceUrl: true, fragment: 'foo', state: {}}); + } + } + `); + + await runMigration(); + + const content = tree.readContent('/index.ts'); + expect(content).toContain( + `this._router.navigate('/', {replaceUrl: true, fragment: 'foo', state: {}});`); + }); + }); + function writeFile(filePath: string, contents: string) { + host.sync.write(normalize(filePath), virtualFs.stringToFileBuffer(contents)); + } + + function runMigration() { + return runner.runSchematicAsync('migration-v11-router-preserve-query-params', {}, tree) + .toPromise(); + } +});