diff --git a/src/cdk/schematics/update-tool/utils/imports.ts b/src/cdk/schematics/update-tool/utils/imports.ts index 58a301d8d0ff..66450acad5fd 100644 --- a/src/cdk/schematics/update-tool/utils/imports.ts +++ b/src/cdk/schematics/update-tool/utils/imports.ts @@ -19,7 +19,7 @@ export function getImportOfIdentifier(typeChecker: ts.TypeChecker, node: ts.Iden null { const symbol = typeChecker.getSymbolAtLocation(node); - if (!symbol || !symbol.declarations.length) { + if (!symbol || !symbol.declarations || !symbol.declarations.length) { return null; } diff --git a/src/material/schematics/ng-update/test-cases/v9/hammer-migration-v9.spec.ts b/src/material/schematics/ng-update/test-cases/v9/hammer-migration-v9.spec.ts index 9aee27bf0ecb..7b0c502a7266 100644 --- a/src/material/schematics/ng-update/test-cases/v9/hammer-migration-v9.spec.ts +++ b/src/material/schematics/ng-update/test-cases/v9/hammer-migration-v9.spec.ts @@ -59,9 +59,12 @@ describe('v9 HammerJS removal', () => { import 'hammerjs'; `); - await runMigration(); + const {logOutput} = await runMigration(); expect(tree.readContent('/projects/cdk-testing/src/main.ts')).not.toContain('hammerjs'); + expect(logOutput).toContain( + 'General notice: The HammerJS v9 migration for Angular components is not able to ' + + 'migrate tests. Please manually clean up tests in your project if they rely on HammerJS.'); }); it('should remove empty named import to load hammerjs', async () => { @@ -310,9 +313,13 @@ describe('v9 HammerJS removal', () => { } `); - await runMigration(); + const {logOutput} = await runMigration(); expect(tree.readContent('/projects/cdk-testing/src/main.ts')).toContain(`import 'hammerjs';`); + expect(logOutput).toContain( + 'General notice: The HammerJS v9 migration for Angular components is not able to ' + + 'migrate tests. Please manually clean up tests in your project if they rely on the ' + + 'deprecated Angular Material gesture config.'); }); it('should ignore global reference to Hammer if not resolved to known types', async () => { @@ -741,70 +748,180 @@ describe('v9 HammerJS removal', () => { .toBe('0.0.0'); }); - it('should not remove hammerjs if no usage could be detected but custom gesture config is set up', - async () => { - appendContent('/projects/cdk-testing/src/main.ts', ` - import 'hammerjs'; - `); - - writeFile('/projects/cdk-testing/src/test.component.ts', dedent` - import {HAMMER_GESTURE_CONFIG} from '@angular/platform-browser'; - import {NgModule} from '@angular/core'; - import {CustomGestureConfig} from "../gesture-config"; - - @NgModule({ - providers: [ - { - provide: HAMMER_GESTURE_CONFIG, - useClass: CustomGestureConfig - }, - ], - }) - export class TestModule {} - `); - - writeFile('/projects/cdk-testing/src/sub.component.ts', dedent` - import {HAMMER_GESTURE_CONFIG} from '@angular/platform-browser'; - import {NgModule} from '@angular/core'; - import {GestureConfig} from '@angular/material/core'; - - @NgModule({ - providers: [ - { - provide: HAMMER_GESTURE_CONFIG, - useClass: GestureConfig - }, - ], - }) - export class SubModule {} - `); - - const {logOutput} = await runMigration(); - - expect(logOutput).toContain(`unable to perform the full migration for this target, but ` + - `removed all references to the deprecated Angular Material gesture config.`); - expect(tree.readContent('/projects/cdk-testing/src/main.ts')).toContain('hammerjs'); - expect(tree.readContent('/projects/cdk-testing/src/test.component.ts')).toContain(dedent` - import {HAMMER_GESTURE_CONFIG} from '@angular/platform-browser'; - import {NgModule} from '@angular/core'; - import {CustomGestureConfig} from "../gesture-config"; - - @NgModule({ - providers: [ - { - provide: HAMMER_GESTURE_CONFIG, - useClass: CustomGestureConfig - }, - ], - }) - export class TestModule {}`); - expect(tree.readContent('/projects/cdk-testing/src/sub.component.ts')).toContain(dedent` - import {NgModule} from '@angular/core'; - - @NgModule({ - providers: [ - ], - }) - export class SubModule {}`); - }); + describe('with custom gesture config', () => { + beforeEach(() => { + addPackageToPackageJson(tree, 'hammerjs', '0.0.0'); + appendContent('/projects/cdk-testing/src/main.ts', `import 'hammerjs';`); + }); + + it('should not setup copied gesture config if hammer is used in template', async () => { + writeFile('/projects/cdk-testing/src/test.component.ts', dedent` + import {HAMMER_GESTURE_CONFIG} from '@angular/platform-browser'; + import {NgModule} from '@angular/core'; + import {CustomGestureConfig} from "../gesture-config"; + + @NgModule({ + providers: [ + { + provide: HAMMER_GESTURE_CONFIG, + useClass: CustomGestureConfig + }, + ], + }) + export class TestModule {} + `); + + writeFile('/projects/cdk-testing/src/app/app.component.html', ` + + `); + + await runMigration(); + + expect(tree.exists('/projects/cdk-testing/src/gesture-config.ts')).toBe(false); + expect(tree.readContent('/projects/cdk-testing/src/main.ts')).toContain('hammerjs'); + expect(runner.tasks.some(t => t.name === 'node-package')).toBe(false); + expect(tree.readContent('/projects/cdk-testing/src/test.component.ts')).toContain(dedent` + import {HAMMER_GESTURE_CONFIG} from '@angular/platform-browser'; + import {NgModule} from '@angular/core'; + import {CustomGestureConfig} from "../gesture-config"; + + @NgModule({ + providers: [ + { + provide: HAMMER_GESTURE_CONFIG, + useClass: CustomGestureConfig + }, + ], + }) + export class TestModule {}`); + }); + + it('should warn if hammer is used in template and references to Material gesture config ' + + 'were detected', async () => { + writeFile('/projects/cdk-testing/src/test.component.ts', dedent` + import {HAMMER_GESTURE_CONFIG} from '@angular/platform-browser'; + import {NgModule} from '@angular/core'; + import {CustomGestureConfig} from "../gesture-config"; + + @NgModule({ + providers: [ + { + provide: HAMMER_GESTURE_CONFIG, + useClass: CustomGestureConfig + }, + ], + }) + export class TestModule {} + `); + + const subModuleFileContent = dedent` + import {NgModule} from '@angular/core'; + import {HAMMER_GESTURE_CONFIG} from '@angular/platform-browser'; + import {GestureConfig} from '@angular/material/core'; + + @NgModule({ + providers: [ + { + provide: HAMMER_GESTURE_CONFIG, + useClass: GestureConfig + }, + ], + }) + export class SubModule {} + `; + + writeFile('/projects/cdk-testing/src/sub.module.ts', subModuleFileContent); + writeFile('/projects/cdk-testing/src/app/app.component.html', ` + + `); + + const {logOutput} = await runMigration(); + + expect(logOutput).toContain( + 'This target cannot be migrated completely. Please manually remove references ' + + 'to the deprecated Angular Material gesture config.'); + expect(runner.tasks.some(t => t.name === 'node-package')).toBe(false); + expect(tree.readContent('/projects/cdk-testing/src/main.ts')).toContain('hammerjs'); + expect(tree.readContent('/projects/cdk-testing/src/test.component.ts')).toContain(dedent` + import {HAMMER_GESTURE_CONFIG} from '@angular/platform-browser'; + import {NgModule} from '@angular/core'; + import {CustomGestureConfig} from "../gesture-config"; + + @NgModule({ + providers: [ + { + provide: HAMMER_GESTURE_CONFIG, + useClass: CustomGestureConfig + }, + ], + }) + export class TestModule {}`); + expect(tree.readContent('/projects/cdk-testing/src/sub.module.ts')) + .toBe(subModuleFileContent); + }); + + it('should not remove hammerjs if no usage could be detected', async () => { + writeFile('/projects/cdk-testing/src/test.component.ts', dedent` + import {HAMMER_GESTURE_CONFIG} from '@angular/platform-browser'; + import {NgModule} from '@angular/core'; + import {CustomGestureConfig} from "../gesture-config"; + + @NgModule({ + providers: [ + { + provide: HAMMER_GESTURE_CONFIG, + useClass: CustomGestureConfig + }, + ], + }) + export class TestModule {} + `); + + writeFile('/projects/cdk-testing/src/sub.component.ts', dedent` + import {HAMMER_GESTURE_CONFIG} from '@angular/platform-browser'; + import {NgModule} from '@angular/core'; + import {GestureConfig} from '@angular/material/core'; + + @NgModule({ + providers: [ + { + provide: HAMMER_GESTURE_CONFIG, + useClass: GestureConfig + }, + ], + }) + export class SubModule {} + `); + + const {logOutput} = await runMigration(); + + expect(logOutput).toContain( + 'This target cannot be migrated completely, but all references to the ' + + 'deprecated Angular Material gesture have been removed.'); + expect(runner.tasks.some(t => t.name === 'node-package')).toBe(false); + expect(tree.readContent('/projects/cdk-testing/src/main.ts')).toContain('hammerjs'); + expect(tree.readContent('/projects/cdk-testing/src/test.component.ts')).toContain(dedent` + import {HAMMER_GESTURE_CONFIG} from '@angular/platform-browser'; + import {NgModule} from '@angular/core'; + import {CustomGestureConfig} from "../gesture-config"; + + @NgModule({ + providers: [ + { + provide: HAMMER_GESTURE_CONFIG, + useClass: CustomGestureConfig + }, + ], + }) + export class TestModule {}`); + expect(tree.readContent('/projects/cdk-testing/src/sub.component.ts')).toContain(dedent` + import {NgModule} from '@angular/core'; + + @NgModule({ + providers: [ + ], + }) + export class SubModule {}`); + }); + }); }); 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 d0bbc4829b56..434f53bca0f4 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 @@ -134,7 +134,51 @@ export class HammerGesturesRule extends MigrationRule { const hasCustomGestureConfigSetup = this._hammerConfigTokenReferences.some(r => this._checkForCustomGestureConfigSetup(r)); - if (this._usedInRuntime || this._usedInTemplate) { + /* + Possible scenarios and how the migration should change the project: + 1. We detect that a custom HammerJS gesture config set up: + - Remove references to the Material gesture config if no event from the + Angular Material gesture config is used. + - Print a warning about ambiguous setup that cannot be handled completely. + 2. We detect that HammerJS is *only* used at runtime: + - Remove references to GestureConfig of Material. + - Remove references to the "HammerModule" if present. + 3. We detect that HammerJS is used in a template: + - Copy the Material gesture config into the app. + - Rewrite all gesture config references to the newly copied one. + - Setup the new gesture config in the root app module. + 4. We detect no HammerJS usage at all: + - Remove Hammer imports + - Remove Material gesture config references + - Remove HammerModule setup if present. + - Remove Hammer script imports in "index.html" files. + */ + + if (hasCustomGestureConfigSetup) { + // If a custom gesture config is provided, we always assume that HammerJS is used. + HammerGesturesRule.globalUsesHammer = true; + if (!this._usedInTemplate && this._gestureConfigReferences.length) { + // If the Angular Material gesture events are not used and we found a custom + // gesture config, we can safely remove references to the Material gesture config + // since events provided by the Material gesture config are guaranteed to be unused. + this._removeMaterialGestureConfigSetup(); + this.printInfo( + 'The HammerJS v9 migration for Angular components detected that HammerJS is ' + + 'manually set up in combination with references to the Angular Material gesture ' + + 'config. This target cannot be migrated completely, but all references to the ' + + 'deprecated Angular Material gesture have been removed.'); + } else if (this._usedInTemplate && this._gestureConfigReferences.length) { + // Since there is a reference to the Angular Material gesture config, and we detected + // usage of a gesture event that could be provided by Angular Material, we *cannot* + // automatically remove references. This is because we do *not* know whether the + // event is actually provided by the custom config or by the Material config. + this.printInfo( + 'The HammerJS v9 migration for Angular components detected that HammerJS is ' + + 'manually set up in combination with references to the Angular Material gesture ' + + 'config. This target cannot be migrated completely. Please manually remove references ' + + 'to the deprecated Angular Material gesture config.'); + } + } else if (this._usedInRuntime || this._usedInTemplate) { // We keep track of whether Hammer is used globally. This is necessary because we // want to only remove Hammer from the "package.json" if it is not used in any project // target. Just because it isn't used in one target doesn't mean that we can safely @@ -150,24 +194,7 @@ export class HammerGesturesRule extends MigrationRule { this._setupHammerGestureConfig(); } } else { - // If HammerJS could not be detected, but we detected a custom gesture config - // setup, we only remove all references to the Angular Material gesture config. - if (hasCustomGestureConfigSetup) { - this._removeMaterialGestureConfigSetup(); - // Print a message if we found a custom gesture config setup in combination with - // references to the Angular Material gesture config. This is ambiguous and the - // migration just removes the Material gesture config setup, but we still want - // to create an information message. - if (this._gestureConfigReferences.length) { - this.printInfo( - 'The HammerJS v9 migration for Angular components detected that HammerJS is' + - 'manually set up in combination with references to the Angular Material gesture ' + - 'config. The migration is unable to perform the full migration for this target, ' + - 'but removed all references to the deprecated Angular Material gesture config.'); - } - } else { - this._removeHammerSetup(); - } + this._removeHammerSetup(); } // Record the changes collected in the import manager. Changes need to be applied @@ -183,11 +210,11 @@ export class HammerGesturesRule extends MigrationRule { // output could also be from a component having an output named similarly to a known // hammerjs event (e.g. "@Output() slide"). The usage is therefore somewhat ambiguous // and we want to print a message that developers might be able to remove Hammer manually. - if (!this._usedInRuntime && this._usedInTemplate) { - this.printInfo(chalk.yellow( + if (!hasCustomGestureConfigSetup && !this._usedInRuntime && this._usedInTemplate) { + this.printInfo( 'The HammerJS v9 migration for Angular components migrated the ' + 'project to keep HammerJS installed, but detected ambiguous usage of HammerJS. Please ' + - 'manually check if you can remove HammerJS from your application.')); + 'manually check if you can remove HammerJS from your application.'); } } @@ -763,16 +790,18 @@ export class HammerGesturesRule extends MigrationRule { * from the "package.json" if it is not used in *any* project target. */ static globalPostMigration(tree: Tree, context: SchematicContext): PostMigrationAction { + // Always notify the developer that the Hammer v9 migration does not migrate tests. + context.logger.info(chalk.yellow( + '\n⚠ General notice: The HammerJS v9 migration for Angular components is not able to ' + + 'migrate tests. Please manually clean up tests in your project if they rely on ' + + (this.globalUsesHammer ? 'the deprecated Angular Material gesture config.' : 'HammerJS.'))); + if (!this.globalUsesHammer && this._removeHammerFromPackageJson(tree)) { // Since Hammer has been removed from the workspace "package.json" file, // we schedule a node package install task to refresh the lock file. return {runPackageManager: true}; } - context.logger.info(chalk.yellow( - '⚠ The HammerJS v9 migration for Angular components is not able to migrate tests. ' + - 'Please manually clean up tests in your project if they rely on HammerJS.')); - // Clean global state once the workspace has been migrated. This is technically // not necessary in "ng update", but in tests we re-use the same rule class. this.globalUsesHammer = false; diff --git a/src/material/schematics/ng-update/upgrade-rules/hammer-gestures-v9/identifier-imports.ts b/src/material/schematics/ng-update/upgrade-rules/hammer-gestures-v9/identifier-imports.ts index 0a502a067c93..e49b3d6a902b 100644 --- a/src/material/schematics/ng-update/upgrade-rules/hammer-gestures-v9/identifier-imports.ts +++ b/src/material/schematics/ng-update/upgrade-rules/hammer-gestures-v9/identifier-imports.ts @@ -50,7 +50,7 @@ export function getImportOfIdentifier(node: ts.Identifier, typeChecker: ts.TypeC function getSpecificImportOfIdentifier(node: ts.Identifier, typeChecker: ts.TypeChecker): Import| null { const symbol = typeChecker.getSymbolAtLocation(node); - if (!symbol || !symbol.declarations.length) { + if (!symbol || !symbol.declarations || !symbol.declarations.length) { return null; } const declaration = symbol.declarations[0]; @@ -74,7 +74,7 @@ function getSpecificImportOfIdentifier(node: ts.Identifier, typeChecker: ts.Type function getImportOfNamespacedIdentifier(node: ts.Identifier, typeChecker: ts.TypeChecker): string| null { const symbol = typeChecker.getSymbolAtLocation(node); - if (!symbol || !symbol.declarations.length) { + if (!symbol || !symbol.declarations || !symbol.declarations.length) { return null; } const declaration = symbol.declarations[0]; diff --git a/tools/tslint-rules/noUndecoratedClassWithNgFieldsRule.ts b/tools/tslint-rules/noUndecoratedClassWithNgFieldsRule.ts index e4405868841b..5b6764359ec2 100644 --- a/tools/tslint-rules/noUndecoratedClassWithNgFieldsRule.ts +++ b/tools/tslint-rules/noUndecoratedClassWithNgFieldsRule.ts @@ -51,7 +51,7 @@ class Walker extends Lint.RuleWalker { /** Gets the module import of the given identifier if imported. */ private _getModuleImportOfIdentifier(node: ts.Identifier): string|null { const symbol = this._typeChecker.getSymbolAtLocation(node); - if (!symbol || !symbol.declarations.length) { + if (!symbol || !symbol.declarations || !symbol.declarations.length) { return null; } const decl = symbol.declarations[0];