Skip to content

Commit c15de55

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

File tree

5 files changed

+138
-7
lines changed

5 files changed

+138
-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: 71 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,72 @@ 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+
const {logOutput} = await runMigration();
644+
645+
expect(logOutput).toContain(
646+
`Material gesture config is used while a custom gesture config is set up`);
647+
expect(tree.readContent('/projects/cdk-testing/src/main.ts')).toContain('hammerjs');
648+
expect(tree.readContent('/projects/cdk-testing/src/test.component.ts')).toContain(dedent`
649+
import {HAMMER_GESTURE_CONFIG} from '@angular/platform-browser';
650+
import {NgModule} from '@angular/core';
651+
import {CustomGestureConfig} from "../gesture-config";
652+
653+
@NgModule({
654+
providers: [
655+
{
656+
provide: HAMMER_GESTURE_CONFIG,
657+
useClass: CustomGestureConfig
658+
},
659+
],
660+
})
661+
export class TestModule {}`);
662+
expect(tree.readContent('/projects/cdk-testing/src/sub.component.ts')).toContain(dedent`
663+
import {NgModule} from '@angular/core';
664+
665+
@NgModule({
666+
providers: [
667+
],
668+
})
669+
export class SubModule {}`);
670+
});
600671
});

‎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: 55 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,24 @@ 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+
// Print a message if we found a custom gesture config setup in combination with
136+
// references to the Angular Material gesture config. This is ambiguous and the
137+
// migration just removes the Material gesture config setup, but we still want
138+
// to create an information message.
139+
if (this._gestureConfigReferences.length) {
140+
this.logger.info(chalk.yellow(
141+
' ⚠ The HammerJS v9 migration for Angular components detected that the Angular ' +
142+
'Material gesture config is used while a custom gesture config is set up. The ' +
143+
'migration removed all references to the Angular Material gesture config.'));
144+
}
145+
} else {
146+
this._removeHammerSetup();
147+
}
125148
}
126149

127150
// Record the changes collected in the import manager. Changes need to be applied
@@ -308,6 +331,32 @@ export class HammerGesturesRule extends MigrationRule<null> {
308331
}
309332
}
310333

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

0 commit comments

Comments
 (0)