Skip to content

Commit c0c9f4a

Browse files
authored
fix(ng-update): better detection for workspace project in v9 hammer migration (#18525)
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 411d048 enabled us to pass through the workspace project. Fixes #18504.
1 parent 2ce4d7d commit c0c9f4a

File tree

5 files changed

+33
-95
lines changed

5 files changed

+33
-95
lines changed

src/cdk/schematics/update-tool/index.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,8 @@ export function runMigrationRules<T>(
5050
// Create instances of all specified migration rules.
5151
for (const ruleCtor of ruleTypes) {
5252
const rule = new ruleCtor(
53-
program, typeChecker, targetVersion, upgradeData, tree, getUpdateRecorder, basePath, logger,
54-
isTestTarget, tsconfigPath);
53+
project, program, typeChecker, targetVersion, upgradeData, tree, getUpdateRecorder,
54+
basePath, logger, isTestTarget, tsconfigPath);
5555
rule.init();
5656
if (rule.ruleEnabled) {
5757
rules.push(rule);

src/cdk/schematics/update-tool/migration-rule.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
import {logging} from '@angular-devkit/core';
1010
import {SchematicContext, Tree, UpdateRecorder} from '@angular-devkit/schematics';
11+
import {WorkspaceProject} from '@schematics/angular/utility/workspace-models';
1112
import * as ts from 'typescript';
1213
import {ResolvedResource} from './component-resource-collector';
1314
import {TargetVersion} from './target-version';
@@ -32,6 +33,8 @@ export class MigrationRule<T> {
3233
ruleEnabled = true;
3334

3435
constructor(
36+
/** Workspace project the migration rule runs against. */
37+
public project: WorkspaceProject,
3538
/** TypeScript program for the migration. */
3639
public program: ts.Program,
3740
/** TypeChecker instance for the analysis program. */

src/material/schematics/ng-update/test-cases/v9/misc/hammer-migration-v9.spec.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,24 @@ describe('v9 HammerJS removal', () => {
3838
`);
3939
}
4040

41+
it('should not throw if project tsconfig does not have explicit root file names', async () => {
42+
// Generates a second project in the workspace. This is necessary to ensure that the
43+
// migration runs logic to determine the correct workspace project.
44+
await runner.runExternalSchematicAsync(
45+
'@schematics/angular', 'application', {name: 'second-project'}, tree).toPromise();
46+
// Overwrite the default tsconfig to not specify any explicit source files. This replicates
47+
// the scenario observed in: https://github.com/angular/components/issues/18504.
48+
writeFile('/projects/cdk-testing/tsconfig.app.json', JSON.stringify({
49+
extends: '../../tsconfig.json',
50+
compilerOptions: {
51+
outDir: '../../out-tsc/app',
52+
types: []
53+
}}
54+
));
55+
addPackageToPackageJson(tree, 'hammerjs', '0.0.0');
56+
await expectAsync(runMigration()).not.toBeRejected();
57+
});
58+
4159
describe('hammerjs not used', () => {
4260
it('should remove hammerjs from "package.json" file', async () => {
4361
addPackageToPackageJson(tree, 'hammerjs', '0.0.0');

src/material/schematics/ng-update/upgrade-rules/hammer-gestures-v9/cli-workspace.ts

Lines changed: 0 additions & 59 deletions
This file was deleted.

src/material/schematics/ng-update/upgrade-rules/hammer-gestures-v9/hammer-gestures-rule.ts

Lines changed: 10 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import {
1111
normalize as devkitNormalize,
1212
Path as DevkitPath
1313
} from '@angular-devkit/core';
14-
import {SchematicContext, SchematicsException, Tree} from '@angular-devkit/schematics';
14+
import {SchematicContext, Tree} from '@angular-devkit/schematics';
1515
import {
1616
getImportOfIdentifier,
1717
getProjectIndexFiles,
@@ -27,13 +27,11 @@ import {
2727
getMetadataField
2828
} from '@angular/cdk/schematics';
2929
import {InsertChange} from '@schematics/angular/utility/change';
30-
import {getWorkspace} from '@schematics/angular/utility/config';
3130
import {WorkspaceProject} from '@schematics/angular/utility/workspace-models';
3231
import {readFileSync} from 'fs';
3332
import {dirname, join, relative} from 'path';
3433
import * as ts from 'typescript';
3534

36-
import {getProjectFromProgram} from './cli-workspace';
3735
import {findHammerScriptImportElements} from './find-hammer-script-tags';
3836
import {findMainModuleExpression} from './find-main-module';
3937
import {isHammerJsUsedInTemplate} from './hammer-template-check';
@@ -242,8 +240,7 @@ export class HammerGesturesRule extends MigrationRule<null> {
242240
* 4) Setup the "HammerModule" in the root app module (if not done already).
243241
*/
244242
private _setupHammerWithCustomEvents() {
245-
const project = this._getProjectOrThrow();
246-
const sourceRoot = devkitNormalize(project.sourceRoot || project.root);
243+
const sourceRoot = devkitNormalize(this.project.sourceRoot || this.project.root);
247244
const newConfigPath =
248245
devkitJoin(sourceRoot, this._getAvailableGestureConfigFileName(sourceRoot));
249246

@@ -261,20 +258,18 @@ export class HammerGesturesRule extends MigrationRule<null> {
261258
// Setup the gesture config provider and the "HammerModule" in the root module
262259
// if not done already. The "HammerModule" is needed in v9 since it enables the
263260
// Hammer event plugin that was previously enabled by default in v8.
264-
this._setupNewGestureConfigInRootModule(project, newConfigPath);
265-
this._setupHammerModuleInRootModule(project);
261+
this._setupNewGestureConfigInRootModule(newConfigPath);
262+
this._setupHammerModuleInRootModule();
266263
}
267264

268265
/**
269266
* Sets up the standard hammer module in the project and removes all
270267
* references to the deprecated Angular Material gesture config.
271268
*/
272269
private _setupHammerWithStandardEvents() {
273-
const project = this._getProjectOrThrow();
274-
275270
// Setup the HammerModule. The HammerModule enables support for
276271
// the standard HammerJS events.
277-
this._setupHammerModuleInRootModule(project);
272+
this._setupHammerModuleInRootModule();
278273
this._removeMaterialGestureConfigSetup();
279274
}
280275

@@ -285,13 +280,11 @@ export class HammerGesturesRule extends MigrationRule<null> {
285280
* 3) Remove "hammerjs" from all index HTML files of the current project.
286281
*/
287282
private _removeHammerSetup() {
288-
const project = this._getProjectOrThrow();
289-
290283
this._installImports.forEach(i => this._importManager.deleteImportByDeclaration(i));
291284

292285
this._removeMaterialGestureConfigSetup();
293286
this._removeHammerModuleReferences();
294-
this._removeHammerFromIndexFile(project);
287+
this._removeHammerFromIndexFile(this.project);
295288
}
296289

297290
/**
@@ -645,8 +638,8 @@ export class HammerGesturesRule extends MigrationRule<null> {
645638
}
646639

647640
/** Sets up the Hammer gesture config in the root module if needed. */
648-
private _setupNewGestureConfigInRootModule(project: WorkspaceProject, gestureConfigPath: string) {
649-
const mainFilePath = join(this.basePath, getProjectMainFile(project));
641+
private _setupNewGestureConfigInRootModule(gestureConfigPath: string) {
642+
const mainFilePath = join(this.basePath, getProjectMainFile(this.project));
650643
const rootModuleSymbol = this._getRootModuleSymbol(mainFilePath);
651644

652645
if (rootModuleSymbol === null) {
@@ -722,8 +715,8 @@ export class HammerGesturesRule extends MigrationRule<null> {
722715
}
723716

724717
/** Sets up the "HammerModule" in the root module of the project. */
725-
private _setupHammerModuleInRootModule(project: WorkspaceProject) {
726-
const mainFilePath = join(this.basePath, getProjectMainFile(project));
718+
private _setupHammerModuleInRootModule() {
719+
const mainFilePath = join(this.basePath, getProjectMainFile(this.project));
727720
const rootModuleSymbol = this._getRootModuleSymbol(mainFilePath);
728721

729722
if (rootModuleSymbol === null) {
@@ -820,23 +813,6 @@ export class HammerGesturesRule extends MigrationRule<null> {
820813
});
821814
}
822815

823-
/**
824-
* Gets the project from the current program or throws if no project
825-
* could be found.
826-
*/
827-
private _getProjectOrThrow(): WorkspaceProject {
828-
const workspace = getWorkspace(this.tree);
829-
const project = getProjectFromProgram(workspace, this.program);
830-
831-
if (!project) {
832-
throw new SchematicsException(
833-
'Could not find project to perform HammerJS v9 migration. ' +
834-
'Please ensure your workspace configuration defines a project.');
835-
}
836-
837-
return project;
838-
}
839-
840816
/** Global state of whether Hammer is used in any analyzed project target. */
841817
static globalUsesHammer = false;
842818

0 commit comments

Comments
 (0)