From aeb443a68348bf714ed262d8582aa268b3dec5f5 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Mon, 17 Feb 2020 14:51:03 +0100 Subject: [PATCH] fix(ng-update): better detection for workspace project in v9 hammer migration Currently the v9 HammerJS migration determines the workspace project by consulting the `ts.Program` that has been constructed. This logic is not guaranteed to work because a TypeScript program doesn't necessarily need to contain any "root" file names. This means that we cannot reliably determine the workspace project from the `ts.Program` and we will throw an error that no project could be found. We can improve this logic by simply using the workspace project that is associated with the originating tsconfig file. This was previously not possible, but 411d048249368e640cae4f31941dd67b24f07446 enabled us to pass through the workspace project. Fixes #18504. --- src/cdk/schematics/update-tool/index.ts | 4 +- .../schematics/update-tool/migration-rule.ts | 3 + .../v9/misc/hammer-migration-v9.spec.ts | 18 ++++++ .../hammer-gestures-v9/cli-workspace.ts | 59 ------------------- .../hammer-gestures-rule.ts | 44 ++++---------- 5 files changed, 33 insertions(+), 95 deletions(-) delete mode 100644 src/material/schematics/ng-update/upgrade-rules/hammer-gestures-v9/cli-workspace.ts diff --git a/src/cdk/schematics/update-tool/index.ts b/src/cdk/schematics/update-tool/index.ts index 372aac364c40..cec95105a7b8 100644 --- a/src/cdk/schematics/update-tool/index.ts +++ b/src/cdk/schematics/update-tool/index.ts @@ -50,8 +50,8 @@ export function runMigrationRules( // Create instances of all specified migration rules. for (const ruleCtor of ruleTypes) { const rule = new ruleCtor( - program, typeChecker, targetVersion, upgradeData, tree, getUpdateRecorder, basePath, logger, - isTestTarget, tsconfigPath); + project, program, typeChecker, targetVersion, upgradeData, tree, getUpdateRecorder, + basePath, logger, isTestTarget, tsconfigPath); rule.init(); if (rule.ruleEnabled) { rules.push(rule); diff --git a/src/cdk/schematics/update-tool/migration-rule.ts b/src/cdk/schematics/update-tool/migration-rule.ts index eb397d8cc799..f92de6debc3c 100644 --- a/src/cdk/schematics/update-tool/migration-rule.ts +++ b/src/cdk/schematics/update-tool/migration-rule.ts @@ -8,6 +8,7 @@ import {logging} from '@angular-devkit/core'; import {SchematicContext, Tree, UpdateRecorder} from '@angular-devkit/schematics'; +import {WorkspaceProject} from '@schematics/angular/utility/workspace-models'; import * as ts from 'typescript'; import {ResolvedResource} from './component-resource-collector'; import {TargetVersion} from './target-version'; @@ -32,6 +33,8 @@ export class MigrationRule { ruleEnabled = true; constructor( + /** Workspace project the migration rule runs against. */ + public project: WorkspaceProject, /** TypeScript program for the migration. */ public program: ts.Program, /** TypeChecker instance for the analysis program. */ diff --git a/src/material/schematics/ng-update/test-cases/v9/misc/hammer-migration-v9.spec.ts b/src/material/schematics/ng-update/test-cases/v9/misc/hammer-migration-v9.spec.ts index fc1c6a6f689b..2b6f86d3e150 100644 --- a/src/material/schematics/ng-update/test-cases/v9/misc/hammer-migration-v9.spec.ts +++ b/src/material/schematics/ng-update/test-cases/v9/misc/hammer-migration-v9.spec.ts @@ -38,6 +38,24 @@ describe('v9 HammerJS removal', () => { `); } + it('should not throw if project tsconfig does not have explicit root file names', async () => { + // Generates a second project in the workspace. This is necessary to ensure that the + // migration runs logic to determine the correct workspace project. + await runner.runExternalSchematicAsync( + '@schematics/angular', 'application', {name: 'second-project'}, tree).toPromise(); + // Overwrite the default tsconfig to not specify any explicit source files. This replicates + // the scenario observed in: https://github.com/angular/components/issues/18504. + writeFile('/projects/cdk-testing/tsconfig.app.json', JSON.stringify({ + extends: '../../tsconfig.json', + compilerOptions: { + outDir: '../../out-tsc/app', + types: [] + }} + )); + addPackageToPackageJson(tree, 'hammerjs', '0.0.0'); + await expectAsync(runMigration()).not.toBeRejected(); + }); + describe('hammerjs not used', () => { it('should remove hammerjs from "package.json" file', async () => { addPackageToPackageJson(tree, 'hammerjs', '0.0.0'); diff --git a/src/material/schematics/ng-update/upgrade-rules/hammer-gestures-v9/cli-workspace.ts b/src/material/schematics/ng-update/upgrade-rules/hammer-gestures-v9/cli-workspace.ts deleted file mode 100644 index b65d2e4cb9f4..000000000000 --- a/src/material/schematics/ng-update/upgrade-rules/hammer-gestures-v9/cli-workspace.ts +++ /dev/null @@ -1,59 +0,0 @@ -/** - * @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 {WorkspaceProject, WorkspaceSchema} from '@schematics/angular/utility/workspace-models'; -import {isAbsolute, relative} from 'path'; -import * as ts from 'typescript'; - -/** Finds all projects which contain the given path. */ -export function getMatchingProjectsByPath( - workspace: WorkspaceSchema, searchPath: string): WorkspaceProject[] { - const projectNames = Object.keys(workspace.projects); - const isProjectMatching = (relativeProjectPath: string): boolean => { - // Build the relative path from the real project path to the - // possible project path based on the specified search path. - const relativePath = relative(relativeProjectPath, searchPath); - // If the relative path does not start with two dots and is not absolute, we - // know that the search path is inside the given project path. - return !relativePath.startsWith('..') && !isAbsolute(relativePath); - }; - - return projectNames.map(name => workspace.projects[name]) - .filter(p => isProjectMatching(p.root)) - .sort((a, b) => b.root.length - a.root.length); -} - -/** - * Gets the matching Angular CLI workspace project from the given program. Project - * is determined by checking root file names of the program against project paths. - * - * If there is only one project set up, the project will be returned regardless of - * whether it matches any of the specified program files. - */ -export function getProjectFromProgram( - workspace: WorkspaceSchema, program: ts.Program): WorkspaceProject|null { - const projectNames = Object.keys(workspace.projects); - - // If there is only one project, we just return it without looking - // for other matching projects. - if (projectNames.length === 1) { - return workspace.projects[projectNames[0]]; - } - - const basePath = program.getCurrentDirectory(); - // Go through the root file names of the program and return the first project - // that matches a given root file. We can't just take any arbitrary file in the - // list since sometimes there can be root files which do not belong to any project. - for (let filePath of program.getRootFileNames()) { - const matchingProjects = getMatchingProjectsByPath(workspace, relative(basePath, filePath)); - if (matchingProjects.length) { - return matchingProjects[0]; - } - } - return null; -} diff --git a/src/material/schematics/ng-update/upgrade-rules/hammer-gestures-v9/hammer-gestures-rule.ts b/src/material/schematics/ng-update/upgrade-rules/hammer-gestures-v9/hammer-gestures-rule.ts index 90d1612362b4..8ec0f511a9f3 100644 --- a/src/material/schematics/ng-update/upgrade-rules/hammer-gestures-v9/hammer-gestures-rule.ts +++ b/src/material/schematics/ng-update/upgrade-rules/hammer-gestures-v9/hammer-gestures-rule.ts @@ -11,7 +11,7 @@ import { normalize as devkitNormalize, Path as DevkitPath } from '@angular-devkit/core'; -import {SchematicContext, SchematicsException, Tree} from '@angular-devkit/schematics'; +import {SchematicContext, Tree} from '@angular-devkit/schematics'; import { getImportOfIdentifier, getProjectIndexFiles, @@ -27,13 +27,11 @@ import { getMetadataField } from '@angular/cdk/schematics'; import {InsertChange} from '@schematics/angular/utility/change'; -import {getWorkspace} from '@schematics/angular/utility/config'; import {WorkspaceProject} from '@schematics/angular/utility/workspace-models'; import {readFileSync} from 'fs'; import {dirname, join, relative} from 'path'; import * as ts from 'typescript'; -import {getProjectFromProgram} from './cli-workspace'; import {findHammerScriptImportElements} from './find-hammer-script-tags'; import {findMainModuleExpression} from './find-main-module'; import {isHammerJsUsedInTemplate} from './hammer-template-check'; @@ -242,8 +240,7 @@ export class HammerGesturesRule extends MigrationRule { * 4) Setup the "HammerModule" in the root app module (if not done already). */ private _setupHammerWithCustomEvents() { - const project = this._getProjectOrThrow(); - const sourceRoot = devkitNormalize(project.sourceRoot || project.root); + const sourceRoot = devkitNormalize(this.project.sourceRoot || this.project.root); const newConfigPath = devkitJoin(sourceRoot, this._getAvailableGestureConfigFileName(sourceRoot)); @@ -261,8 +258,8 @@ export class HammerGesturesRule extends MigrationRule { // Setup the gesture config provider and the "HammerModule" in the root module // if not done already. The "HammerModule" is needed in v9 since it enables the // Hammer event plugin that was previously enabled by default in v8. - this._setupNewGestureConfigInRootModule(project, newConfigPath); - this._setupHammerModuleInRootModule(project); + this._setupNewGestureConfigInRootModule(newConfigPath); + this._setupHammerModuleInRootModule(); } /** @@ -270,11 +267,9 @@ export class HammerGesturesRule extends MigrationRule { * references to the deprecated Angular Material gesture config. */ private _setupHammerWithStandardEvents() { - const project = this._getProjectOrThrow(); - // Setup the HammerModule. The HammerModule enables support for // the standard HammerJS events. - this._setupHammerModuleInRootModule(project); + this._setupHammerModuleInRootModule(); this._removeMaterialGestureConfigSetup(); } @@ -285,13 +280,11 @@ export class HammerGesturesRule extends MigrationRule { * 3) Remove "hammerjs" from all index HTML files of the current project. */ private _removeHammerSetup() { - const project = this._getProjectOrThrow(); - this._installImports.forEach(i => this._importManager.deleteImportByDeclaration(i)); this._removeMaterialGestureConfigSetup(); this._removeHammerModuleReferences(); - this._removeHammerFromIndexFile(project); + this._removeHammerFromIndexFile(this.project); } /** @@ -645,8 +638,8 @@ export class HammerGesturesRule extends MigrationRule { } /** Sets up the Hammer gesture config in the root module if needed. */ - private _setupNewGestureConfigInRootModule(project: WorkspaceProject, gestureConfigPath: string) { - const mainFilePath = join(this.basePath, getProjectMainFile(project)); + private _setupNewGestureConfigInRootModule(gestureConfigPath: string) { + const mainFilePath = join(this.basePath, getProjectMainFile(this.project)); const rootModuleSymbol = this._getRootModuleSymbol(mainFilePath); if (rootModuleSymbol === null) { @@ -722,8 +715,8 @@ export class HammerGesturesRule extends MigrationRule { } /** Sets up the "HammerModule" in the root module of the project. */ - private _setupHammerModuleInRootModule(project: WorkspaceProject) { - const mainFilePath = join(this.basePath, getProjectMainFile(project)); + private _setupHammerModuleInRootModule() { + const mainFilePath = join(this.basePath, getProjectMainFile(this.project)); const rootModuleSymbol = this._getRootModuleSymbol(mainFilePath); if (rootModuleSymbol === null) { @@ -820,23 +813,6 @@ export class HammerGesturesRule extends MigrationRule { }); } - /** - * Gets the project from the current program or throws if no project - * could be found. - */ - private _getProjectOrThrow(): WorkspaceProject { - const workspace = getWorkspace(this.tree); - const project = getProjectFromProgram(workspace, this.program); - - if (!project) { - throw new SchematicsException( - 'Could not find project to perform HammerJS v9 migration. ' + - 'Please ensure your workspace configuration defines a project.'); - } - - return project; - } - /** Global state of whether Hammer is used in any analyzed project target. */ static globalUsesHammer = false;