Skip to content

Commit 46908af

Browse files
devversiondevversion
committed
fixup! feat(material/ng-update): add migration for hammerjs in version 9
Address feedback
1 parent 609a374 commit 46908af

File tree

5 files changed

+126
-7
lines changed

5 files changed

+126
-7
lines changed

‎src/cdk/schematics/update-tool/target-version.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ export enum TargetVersion {
1212
V7 = 'version 7',
1313
V8 = 'version 8',
1414
V9 = 'version 9',
15+
V10 = 'version 10',
1516
}
1617

1718
/**

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

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,8 @@ describe('v9 HammerJS removal', () => {
237237
<body>
238238
<app-root></app-root>
239239
</body>
240+
<script src="some-other-script.js"></script>
241+
<script src="https://ajax.googleapis.com/ajax/libs/hammerjs/2.0.8/hammer.min.js"></script>
240242
</html>
241243
`);
242244

@@ -252,6 +254,7 @@ describe('v9 HammerJS removal', () => {
252254
<body>
253255
<app-root></app-root>
254256
</body>
257+
<script src="some-other-script.js"></script>
255258
</html>`);
256259
});
257260
});
@@ -597,4 +600,70 @@ describe('v9 HammerJS removal', () => {
597600

598601
expect(JSON.parse(tree.readContent('/package.json')).dependencies['hammerjs']).toBe('0.0.0');
599602
});
603+
604+
605+
it('should not remove hammerjs if no usage could be detected but custom gesture config is set up',
606+
async () => {
607+
appendContent('/projects/cdk-testing/src/main.ts', `
608+
import 'hammerjs';
609+
`);
610+
611+
writeFile('/projects/cdk-testing/src/test.component.ts', dedent`
612+
import {HAMMER_GESTURE_CONFIG} from '@angular/platform-browser';
613+
import {NgModule} from '@angular/core';
614+
import {CustomGestureConfig} from "../gesture-config";
615+
616+
@NgModule({
617+
providers: [
618+
{
619+
provide: HAMMER_GESTURE_CONFIG,
620+
useClass: CustomGestureConfig
621+
},
622+
],
623+
})
624+
export class TestModule {}
625+
`);
626+
627+
writeFile('/projects/cdk-testing/src/sub.component.ts', dedent`
628+
import {HAMMER_GESTURE_CONFIG} from '@angular/platform-browser';
629+
import {NgModule} from '@angular/core';
630+
import {GestureConfig} from '@angular/material/core';
631+
632+
@NgModule({
633+
providers: [
634+
{
635+
provide: HAMMER_GESTURE_CONFIG,
636+
useClass: GestureConfig
637+
},
638+
],
639+
})
640+
export class SubModule {}
641+
`);
642+
643+
await runMigration();
644+
645+
expect(tree.readContent('/projects/cdk-testing/src/main.ts')).toContain('hammerjs');
646+
expect(tree.readContent('/projects/cdk-testing/src/test.component.ts')).toContain(dedent`
647+
import {HAMMER_GESTURE_CONFIG} from '@angular/platform-browser';
648+
import {NgModule} from '@angular/core';
649+
import {CustomGestureConfig} from "../gesture-config";
650+
651+
@NgModule({
652+
providers: [
653+
{
654+
provide: HAMMER_GESTURE_CONFIG,
655+
useClass: CustomGestureConfig
656+
},
657+
],
658+
})
659+
export class TestModule {}`);
660+
expect(tree.readContent('/projects/cdk-testing/src/sub.component.ts')).toContain(dedent`
661+
import {NgModule} from '@angular/core';
662+
663+
@NgModule({
664+
providers: [
665+
],
666+
})
667+
export class SubModule {}`);
668+
});
600669
});

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ export function getMatchingProjectsByPath(
2020
const relativePath = relative(relativeProjectPath, searchPath);
2121
// If the relative path does not start with two dots and is not absolute, we
2222
// know that the search path is inside the given project path.
23-
return !!(!relativePath.startsWith('..') && !isAbsolute(relativePath));
23+
return !relativePath.startsWith('..') && !isAbsolute(relativePath);
2424
};
2525

2626
return projectNames.map(name => workspace.projects[name])

‎src/material/schematics/ng-update/upgrade-rules/hammer-gestures-v9/gesture-config.template

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,13 @@
1+
/**
2+
* Custom HammerJS configuration forked from Angular Material. With Angular v9,
3+
* Angular Material dropped HammerJS as a dependency. This configuration was added
4+
* automatically to this application because `ng update` detected that this application
5+
* directly used HammerJS.
6+
*
7+
* If this application does not depend on the custom gestures originally defined by
8+
* Angular Material, this file can be deleted.
9+
*/
10+
111
import {Injectable, Inject, Optional, Type} from '@angular/core';
212
import {HammerGestureConfig} from '@angular/platform-browser';
313
import {MAT_HAMMER_OPTIONS} from '@angular/material/core';

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

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,12 @@ interface IdentifierReference {
5959
}
6060

6161
export class HammerGesturesRule extends MigrationRule<null> {
62-
// Only enable this rule if the migration targets version 9 and is running
63-
// for a non-test target. We cannot migrate test targets since they have a
64-
// limited scope (in regards to source files) and therefore the HammerJS
65-
// usage detection can be incorrect.
66-
ruleEnabled = this.targetVersion === TargetVersion.V9 && !this.isTestTarget;
62+
// Only enable this rule if the migration targets v9 or v10 and is running for a non-test
63+
// target. We cannot migrate test targets since they have a limited scope
64+
// (in regards to source files) and therefore the HammerJS usage detection can be incorrect.
65+
ruleEnabled =
66+
(this.targetVersion === TargetVersion.V9 || this.targetVersion === TargetVersion.V10) &&
67+
!this.isTestTarget;
6768

6869
private _printer = ts.createPrinter();
6970
private _importManager = new ImportManager(this.getUpdateRecorder, this._printer);
@@ -112,6 +113,11 @@ export class HammerGesturesRule extends MigrationRule<null> {
112113
}
113114

114115
postAnalysis(): void {
116+
// Walk through all hammer config token references and check if there
117+
// is a potential custom gesture config setup.
118+
const hasCustomGestureConfigSetup =
119+
this._hammerConfigTokenReferences.some(r => this._checkForCustomGestureConfigSetup(r));
120+
115121
if (this._usedInRuntime || this._usedInTemplate) {
116122
// If hammer is only used at runtime, we don't need the gesture config
117123
// and can remove it (along with the hammer config token if possible)
@@ -121,7 +127,14 @@ export class HammerGesturesRule extends MigrationRule<null> {
121127
this._setupHammerGestureConfig();
122128
}
123129
} else {
124-
this._removeHammerSetup();
130+
// If HammerJS could not be detected, but we detected a custom gesture
131+
// config setup, we just remove all references to the Angular Material
132+
// gesture config. Otherwise we completely remove HammerJS from the app.
133+
if (hasCustomGestureConfigSetup) {
134+
this._removeGestureConfigSetup();
135+
} else {
136+
this._removeHammerSetup();
137+
}
125138
}
126139

127140
// Record the changes collected in the import manager. Changes need to be applied
@@ -308,6 +321,32 @@ export class HammerGesturesRule extends MigrationRule<null> {
308321
}
309322
}
310323

324+
/**
325+
* Checks if the given Hammer gesture config token reference is part of an
326+
* Angular provider definition that sets up a custom gesture config.
327+
*/
328+
private _checkForCustomGestureConfigSetup(tokenRef: IdentifierReference): boolean {
329+
// Walk up the tree to look for a parent property assignment of the
330+
// reference to the hammer gesture config token.
331+
let propertyAssignment: ts.Node = tokenRef.node;
332+
while (propertyAssignment && !ts.isPropertyAssignment(propertyAssignment)) {
333+
propertyAssignment = propertyAssignment.parent;
334+
}
335+
336+
if (!propertyAssignment || !ts.isPropertyAssignment(propertyAssignment) ||
337+
getPropertyNameText(propertyAssignment.name) !== 'provide') {
338+
return false;
339+
}
340+
341+
const objectLiteralExpr = propertyAssignment.parent;
342+
const matchingIdentifiers = findMatchingChildNodes(objectLiteralExpr, ts.isIdentifier);
343+
344+
// We naively assume that if there is a reference to the "GestureConfig" export
345+
// from Angular Material in the provider literal, that the provider sets up the
346+
// Angular Material gesture config.
347+
return !this._gestureConfigReferences.some(r => matchingIdentifiers.includes(r.node));
348+
}
349+
311350
/**
312351
* Determines an available file name for the gesture config which should
313352
* be stored in the specified file path.

0 commit comments

Comments
 (0)