Skip to content

Commit 4794c60

Browse files
devversionandrewseguin
authored andcommitted
fix(ng-update): hammer v9 migration should not unnecessarily set up gestures (#17713)
* fix(ng-update): hammer v9 migration should not unnecessarily set up gestures We currently do not properly handle the scenario where a project uses a custom gesture config with event names matching the ones provided by the deprecated Angular Material gesture config. Prior to this commit, the migration would set up the Material gesture config next to the custom one. This is incorrect, and we should do nothing since a gesture config is already configured. Though, if we detect that there are manual references to the Angular Material gesture config, the detection is ambiguous and the migration should warn that the developer should manually remove the references to the deprecated Material gesture config. Also improves messaging for the migration, adds a description of the expected migration behavior into the migration rule and adds safety checks for `ts.Symbol#declaration` (should never be undefined according to typing, but for odd reasons it can be undefined, and we do not want to throw accidentally). * Fix comment wording * fixup! Fix comment wording Address feedback
1 parent 93dc69f commit 4794c60

File tree

5 files changed

+244
-98
lines changed

5 files changed

+244
-98
lines changed

src/cdk/schematics/update-tool/utils/imports.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ export function getImportOfIdentifier(typeChecker: ts.TypeChecker, node: ts.Iden
1919
null {
2020
const symbol = typeChecker.getSymbolAtLocation(node);
2121

22-
if (!symbol || !symbol.declarations.length) {
22+
if (!symbol || !symbol.declarations || !symbol.declarations.length) {
2323
return null;
2424
}
2525

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

Lines changed: 185 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,12 @@ describe('v9 HammerJS removal', () => {
5959
import 'hammerjs';
6060
`);
6161

62-
await runMigration();
62+
const {logOutput} = await runMigration();
6363

6464
expect(tree.readContent('/projects/cdk-testing/src/main.ts')).not.toContain('hammerjs');
65+
expect(logOutput).toContain(
66+
'General notice: The HammerJS v9 migration for Angular components is not able to ' +
67+
'migrate tests. Please manually clean up tests in your project if they rely on HammerJS.');
6568
});
6669

6770
it('should remove empty named import to load hammerjs', async () => {
@@ -310,9 +313,13 @@ describe('v9 HammerJS removal', () => {
310313
}
311314
`);
312315

313-
await runMigration();
316+
const {logOutput} = await runMigration();
314317

315318
expect(tree.readContent('/projects/cdk-testing/src/main.ts')).toContain(`import 'hammerjs';`);
319+
expect(logOutput).toContain(
320+
'General notice: The HammerJS v9 migration for Angular components is not able to ' +
321+
'migrate tests. Please manually clean up tests in your project if they rely on the ' +
322+
'deprecated Angular Material gesture config.');
316323
});
317324

318325
it('should ignore global reference to Hammer if not resolved to known types', async () => {
@@ -741,70 +748,180 @@ describe('v9 HammerJS removal', () => {
741748
.toBe('0.0.0');
742749
});
743750

744-
it('should not remove hammerjs if no usage could be detected but custom gesture config is set up',
745-
async () => {
746-
appendContent('/projects/cdk-testing/src/main.ts', `
747-
import 'hammerjs';
748-
`);
749-
750-
writeFile('/projects/cdk-testing/src/test.component.ts', dedent`
751-
import {HAMMER_GESTURE_CONFIG} from '@angular/platform-browser';
752-
import {NgModule} from '@angular/core';
753-
import {CustomGestureConfig} from "../gesture-config";
754-
755-
@NgModule({
756-
providers: [
757-
{
758-
provide: HAMMER_GESTURE_CONFIG,
759-
useClass: CustomGestureConfig
760-
},
761-
],
762-
})
763-
export class TestModule {}
764-
`);
765-
766-
writeFile('/projects/cdk-testing/src/sub.component.ts', dedent`
767-
import {HAMMER_GESTURE_CONFIG} from '@angular/platform-browser';
768-
import {NgModule} from '@angular/core';
769-
import {GestureConfig} from '@angular/material/core';
770-
771-
@NgModule({
772-
providers: [
773-
{
774-
provide: HAMMER_GESTURE_CONFIG,
775-
useClass: GestureConfig
776-
},
777-
],
778-
})
779-
export class SubModule {}
780-
`);
781-
782-
const {logOutput} = await runMigration();
783-
784-
expect(logOutput).toContain(`unable to perform the full migration for this target, but ` +
785-
`removed all references to the deprecated Angular Material gesture config.`);
786-
expect(tree.readContent('/projects/cdk-testing/src/main.ts')).toContain('hammerjs');
787-
expect(tree.readContent('/projects/cdk-testing/src/test.component.ts')).toContain(dedent`
788-
import {HAMMER_GESTURE_CONFIG} from '@angular/platform-browser';
789-
import {NgModule} from '@angular/core';
790-
import {CustomGestureConfig} from "../gesture-config";
791-
792-
@NgModule({
793-
providers: [
794-
{
795-
provide: HAMMER_GESTURE_CONFIG,
796-
useClass: CustomGestureConfig
797-
},
798-
],
799-
})
800-
export class TestModule {}`);
801-
expect(tree.readContent('/projects/cdk-testing/src/sub.component.ts')).toContain(dedent`
802-
import {NgModule} from '@angular/core';
803-
804-
@NgModule({
805-
providers: [
806-
],
807-
})
808-
export class SubModule {}`);
809-
});
751+
describe('with custom gesture config', () => {
752+
beforeEach(() => {
753+
addPackageToPackageJson(tree, 'hammerjs', '0.0.0');
754+
appendContent('/projects/cdk-testing/src/main.ts', `import 'hammerjs';`);
755+
});
756+
757+
it('should not setup copied gesture config if hammer is used in template', async () => {
758+
writeFile('/projects/cdk-testing/src/test.component.ts', dedent`
759+
import {HAMMER_GESTURE_CONFIG} from '@angular/platform-browser';
760+
import {NgModule} from '@angular/core';
761+
import {CustomGestureConfig} from "../gesture-config";
762+
763+
@NgModule({
764+
providers: [
765+
{
766+
provide: HAMMER_GESTURE_CONFIG,
767+
useClass: CustomGestureConfig
768+
},
769+
],
770+
})
771+
export class TestModule {}
772+
`);
773+
774+
writeFile('/projects/cdk-testing/src/app/app.component.html', `
775+
<span (longpress)="onPress()"></span>
776+
`);
777+
778+
await runMigration();
779+
780+
expect(tree.exists('/projects/cdk-testing/src/gesture-config.ts')).toBe(false);
781+
expect(tree.readContent('/projects/cdk-testing/src/main.ts')).toContain('hammerjs');
782+
expect(runner.tasks.some(t => t.name === 'node-package')).toBe(false);
783+
expect(tree.readContent('/projects/cdk-testing/src/test.component.ts')).toContain(dedent`
784+
import {HAMMER_GESTURE_CONFIG} from '@angular/platform-browser';
785+
import {NgModule} from '@angular/core';
786+
import {CustomGestureConfig} from "../gesture-config";
787+
788+
@NgModule({
789+
providers: [
790+
{
791+
provide: HAMMER_GESTURE_CONFIG,
792+
useClass: CustomGestureConfig
793+
},
794+
],
795+
})
796+
export class TestModule {}`);
797+
});
798+
799+
it('should warn if hammer is used in template and references to Material gesture config ' +
800+
'were detected', async () => {
801+
writeFile('/projects/cdk-testing/src/test.component.ts', dedent`
802+
import {HAMMER_GESTURE_CONFIG} from '@angular/platform-browser';
803+
import {NgModule} from '@angular/core';
804+
import {CustomGestureConfig} from "../gesture-config";
805+
806+
@NgModule({
807+
providers: [
808+
{
809+
provide: HAMMER_GESTURE_CONFIG,
810+
useClass: CustomGestureConfig
811+
},
812+
],
813+
})
814+
export class TestModule {}
815+
`);
816+
817+
const subModuleFileContent = dedent`
818+
import {NgModule} from '@angular/core';
819+
import {HAMMER_GESTURE_CONFIG} from '@angular/platform-browser';
820+
import {GestureConfig} from '@angular/material/core';
821+
822+
@NgModule({
823+
providers: [
824+
{
825+
provide: HAMMER_GESTURE_CONFIG,
826+
useClass: GestureConfig
827+
},
828+
],
829+
})
830+
export class SubModule {}
831+
`;
832+
833+
writeFile('/projects/cdk-testing/src/sub.module.ts', subModuleFileContent);
834+
writeFile('/projects/cdk-testing/src/app/app.component.html', `
835+
<span (longpress)="onPress()"></span>
836+
`);
837+
838+
const {logOutput} = await runMigration();
839+
840+
expect(logOutput).toContain(
841+
'This target cannot be migrated completely. Please manually remove references ' +
842+
'to the deprecated Angular Material gesture config.');
843+
expect(runner.tasks.some(t => t.name === 'node-package')).toBe(false);
844+
expect(tree.readContent('/projects/cdk-testing/src/main.ts')).toContain('hammerjs');
845+
expect(tree.readContent('/projects/cdk-testing/src/test.component.ts')).toContain(dedent`
846+
import {HAMMER_GESTURE_CONFIG} from '@angular/platform-browser';
847+
import {NgModule} from '@angular/core';
848+
import {CustomGestureConfig} from "../gesture-config";
849+
850+
@NgModule({
851+
providers: [
852+
{
853+
provide: HAMMER_GESTURE_CONFIG,
854+
useClass: CustomGestureConfig
855+
},
856+
],
857+
})
858+
export class TestModule {}`);
859+
expect(tree.readContent('/projects/cdk-testing/src/sub.module.ts'))
860+
.toBe(subModuleFileContent);
861+
});
862+
863+
it('should not remove hammerjs if no usage could be detected', async () => {
864+
writeFile('/projects/cdk-testing/src/test.component.ts', dedent`
865+
import {HAMMER_GESTURE_CONFIG} from '@angular/platform-browser';
866+
import {NgModule} from '@angular/core';
867+
import {CustomGestureConfig} from "../gesture-config";
868+
869+
@NgModule({
870+
providers: [
871+
{
872+
provide: HAMMER_GESTURE_CONFIG,
873+
useClass: CustomGestureConfig
874+
},
875+
],
876+
})
877+
export class TestModule {}
878+
`);
879+
880+
writeFile('/projects/cdk-testing/src/sub.component.ts', dedent`
881+
import {HAMMER_GESTURE_CONFIG} from '@angular/platform-browser';
882+
import {NgModule} from '@angular/core';
883+
import {GestureConfig} from '@angular/material/core';
884+
885+
@NgModule({
886+
providers: [
887+
{
888+
provide: HAMMER_GESTURE_CONFIG,
889+
useClass: GestureConfig
890+
},
891+
],
892+
})
893+
export class SubModule {}
894+
`);
895+
896+
const {logOutput} = await runMigration();
897+
898+
expect(logOutput).toContain(
899+
'This target cannot be migrated completely, but all references to the ' +
900+
'deprecated Angular Material gesture have been removed.');
901+
expect(runner.tasks.some(t => t.name === 'node-package')).toBe(false);
902+
expect(tree.readContent('/projects/cdk-testing/src/main.ts')).toContain('hammerjs');
903+
expect(tree.readContent('/projects/cdk-testing/src/test.component.ts')).toContain(dedent`
904+
import {HAMMER_GESTURE_CONFIG} from '@angular/platform-browser';
905+
import {NgModule} from '@angular/core';
906+
import {CustomGestureConfig} from "../gesture-config";
907+
908+
@NgModule({
909+
providers: [
910+
{
911+
provide: HAMMER_GESTURE_CONFIG,
912+
useClass: CustomGestureConfig
913+
},
914+
],
915+
})
916+
export class TestModule {}`);
917+
expect(tree.readContent('/projects/cdk-testing/src/sub.component.ts')).toContain(dedent`
918+
import {NgModule} from '@angular/core';
919+
920+
@NgModule({
921+
providers: [
922+
],
923+
})
924+
export class SubModule {}`);
925+
});
926+
});
810927
});

0 commit comments

Comments
 (0)