Skip to content

Commit 7a2d372

Browse files
devversionjelbourn
authored andcommitted
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. (cherry picked from commit c0c9f4a)
1 parent 7b64656 commit 7a2d372

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
@@ -48,8 +48,8 @@ export function runMigrationRules<T>(
4848
// Create instances of all specified migration rules.
4949
for (const ruleCtor of ruleTypes) {
5050
const rule = new ruleCtor(
51-
program, typeChecker, targetVersion, upgradeData, tree, getUpdateRecorder, basePath, logger,
52-
isTestTarget, tsconfigPath);
51+
project, program, typeChecker, targetVersion, upgradeData, tree, getUpdateRecorder,
52+
basePath, logger, isTestTarget, tsconfigPath);
5353
rule.init();
5454
if (rule.ruleEnabled) {
5555
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,
@@ -29,13 +29,11 @@ import {
2929
getMetadataField
3030
} from '@schematics/angular/utility/ast-utils';
3131
import {InsertChange} from '@schematics/angular/utility/change';
32-
import {getWorkspace} from '@schematics/angular/utility/config';
3332
import {WorkspaceProject} from '@schematics/angular/utility/workspace-models';
3433
import {readFileSync} from 'fs';
3534
import {dirname, join, relative} from 'path';
3635
import * as ts from 'typescript';
3736

38-
import {getProjectFromProgram} from './cli-workspace';
3937
import {findHammerScriptImportElements} from './find-hammer-script-tags';
4038
import {findMainModuleExpression} from './find-main-module';
4139
import {isHammerJsUsedInTemplate} from './hammer-template-check';
@@ -244,8 +242,7 @@ export class HammerGesturesRule extends MigrationRule<null> {
244242
* 4) Setup the "HammerModule" in the root app module (if not done already).
245243
*/
246244
private _setupHammerWithCustomEvents() {
247-
const project = this._getProjectOrThrow();
248-
const sourceRoot = devkitNormalize(project.sourceRoot || project.root);
245+
const sourceRoot = devkitNormalize(this.project.sourceRoot || this.project.root);
249246
const newConfigPath =
250247
devkitJoin(sourceRoot, this._getAvailableGestureConfigFileName(sourceRoot));
251248

@@ -263,20 +260,18 @@ export class HammerGesturesRule extends MigrationRule<null> {
263260
// Setup the gesture config provider and the "HammerModule" in the root module
264261
// if not done already. The "HammerModule" is needed in v9 since it enables the
265262
// Hammer event plugin that was previously enabled by default in v8.
266-
this._setupNewGestureConfigInRootModule(project, newConfigPath);
267-
this._setupHammerModuleInRootModule(project);
263+
this._setupNewGestureConfigInRootModule(newConfigPath);
264+
this._setupHammerModuleInRootModule();
268265
}
269266

270267
/**
271268
* Sets up the standard hammer module in the project and removes all
272269
* references to the deprecated Angular Material gesture config.
273270
*/
274271
private _setupHammerWithStandardEvents() {
275-
const project = this._getProjectOrThrow();
276-
277272
// Setup the HammerModule. The HammerModule enables support for
278273
// the standard HammerJS events.
279-
this._setupHammerModuleInRootModule(project);
274+
this._setupHammerModuleInRootModule();
280275
this._removeMaterialGestureConfigSetup();
281276
}
282277

@@ -287,13 +282,11 @@ export class HammerGesturesRule extends MigrationRule<null> {
287282
* 3) Remove "hammerjs" from all index HTML files of the current project.
288283
*/
289284
private _removeHammerSetup() {
290-
const project = this._getProjectOrThrow();
291-
292285
this._installImports.forEach(i => this._importManager.deleteImportByDeclaration(i));
293286

294287
this._removeMaterialGestureConfigSetup();
295288
this._removeHammerModuleReferences();
296-
this._removeHammerFromIndexFile(project);
289+
this._removeHammerFromIndexFile(this.project);
297290
}
298291

299292
/**
@@ -647,8 +640,8 @@ export class HammerGesturesRule extends MigrationRule<null> {
647640
}
648641

649642
/** Sets up the Hammer gesture config in the root module if needed. */
650-
private _setupNewGestureConfigInRootModule(project: WorkspaceProject, gestureConfigPath: string) {
651-
const mainFilePath = join(this.basePath, getProjectMainFile(project));
643+
private _setupNewGestureConfigInRootModule(gestureConfigPath: string) {
644+
const mainFilePath = join(this.basePath, getProjectMainFile(this.project));
652645
const rootModuleSymbol = this._getRootModuleSymbol(mainFilePath);
653646

654647
if (rootModuleSymbol === null) {
@@ -724,8 +717,8 @@ export class HammerGesturesRule extends MigrationRule<null> {
724717
}
725718

726719
/** Sets up the "HammerModule" in the root module of the project. */
727-
private _setupHammerModuleInRootModule(project: WorkspaceProject) {
728-
const mainFilePath = join(this.basePath, getProjectMainFile(project));
720+
private _setupHammerModuleInRootModule() {
721+
const mainFilePath = join(this.basePath, getProjectMainFile(this.project));
729722
const rootModuleSymbol = this._getRootModuleSymbol(mainFilePath);
730723

731724
if (rootModuleSymbol === null) {
@@ -822,23 +815,6 @@ export class HammerGesturesRule extends MigrationRule<null> {
822815
});
823816
}
824817

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

0 commit comments

Comments
 (0)