Skip to content

Commit 0398dce

Browse files
committed
Handle existing animation modules
1 parent 3e612b7 commit 0398dce

File tree

7 files changed

+170
-33
lines changed

7 files changed

+170
-33
lines changed
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import {Tree} from '@angular-devkit/schematics';
10+
import * as ts from 'typescript';
11+
12+
/**
13+
* Whether the Angular module in the given path imports the specifed module class name.
14+
*/
15+
export function hasNgModuleImport(tree: Tree, modulePath: string, className: string): boolean {
16+
const moduleFileContent = tree.read(modulePath);
17+
18+
if (!moduleFileContent) {
19+
throw new Error(`Could not read Angular module file: ${modulePath}`);
20+
}
21+
22+
const parsedFile = ts.createSourceFile(modulePath, moduleFileContent.toString(),
23+
ts.ScriptTarget.Latest, true);
24+
let ngModuleMetadata: ts.ObjectLiteralExpression | null = null;
25+
26+
const findModuleDecorator = (node: ts.Node) => {
27+
if (ts.isDecorator(node) && ts.isCallExpression(node.expression) &&
28+
isNgModuleCallExpression(node.expression)) {
29+
ngModuleMetadata = node.expression.arguments[0] as ts.ObjectLiteralExpression;
30+
return;
31+
}
32+
33+
ts.forEachChild(node, findModuleDecorator);
34+
};
35+
36+
ts.forEachChild(parsedFile, findModuleDecorator);
37+
38+
if (!ngModuleMetadata) {
39+
throw new Error(`Could not find NgModule declaration inside: "${modulePath}"`);
40+
}
41+
42+
for (let property of ngModuleMetadata!.properties) {
43+
if (!ts.isPropertyAssignment(property) || property.name.getText() !== 'imports' ||
44+
!ts.isArrayLiteralExpression(property.initializer)) {
45+
continue;
46+
}
47+
48+
if (property.initializer.elements.some(element => element.getText() === className)) {
49+
return true;
50+
}
51+
}
52+
53+
return false;
54+
}
55+
56+
/**
57+
* Resolves the last identifier that is part of the given expression. This helps resolving
58+
* identifiers of nested property access expressions (e.g. myNamespace.core.NgModule).
59+
*/
60+
function resolveIdentifierOfExpression(expression: ts.Expression): ts.Identifier | null {
61+
if (ts.isIdentifier(expression)) {
62+
return expression;
63+
} else if (ts.isPropertyAccessExpression(expression)) {
64+
return resolveIdentifierOfExpression(expression.expression);
65+
}
66+
return null;
67+
}
68+
69+
/** Whether the specified call expression is referring to a NgModule definition. */
70+
function isNgModuleCallExpression(callExpression: ts.CallExpression): boolean {
71+
if (!callExpression.arguments.length ||
72+
!ts.isObjectLiteralExpression(callExpression.arguments[0])) {
73+
return false;
74+
}
75+
76+
const decoratorIdentifier = resolveIdentifierOfExpression(callExpression.expression);
77+
return decoratorIdentifier ? decoratorIdentifier.text === 'NgModule' : false;
78+
}

src/cdk/schematics/utils/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
*/
88

99
export * from './ast';
10+
export * from './ast/ng-module-imports';
1011
export * from './build-component';
1112
export * from './get-project';
1213
export * from './parse5-element';

src/lib/schematics/ng-add/index.spec.ts

Lines changed: 45 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import {WorkspaceProject} from '@angular-devkit/core/src/workspace';
33
import {Tree} from '@angular-devkit/schematics';
44
import {SchematicTestRunner} from '@angular-devkit/schematics/testing';
55
import {
6+
addModuleImportToRootModule,
67
createTestApp,
78
getProjectFromWorkspace,
89
getProjectStyleFile,
@@ -37,9 +38,9 @@ describe('ng-add schematic', () => {
3738
it('should update package.json', () => {
3839
// By default, the Angular workspace schematic sets up "@angular/animations". In order
3940
// to verify that we would set up the dependency properly if someone doesn't have the
40-
// animations installed already, we remove the animations dependency explicitly.
41+
// animations installed already, we remove the animations dependency explicitly.
4142
removePackageJsonDependency(appTree, '@angular/animations');
42-
43+
4344
const tree = runner.runSchematic('ng-add', {}, appTree);
4445
const packageJson = JSON.parse(getFileContent(tree, '/package.json'));
4546
const dependencies = packageJson.dependencies;
@@ -155,27 +156,55 @@ describe('ng-add schematic', () => {
155156
});
156157
});
157158

158-
describe('animations disabled', () => {
159+
describe('animations enabled', () => {
160+
it('should add the BrowserAnimationsModule to the project module', () => {
161+
const tree = runner.runSchematic('ng-add-setup-project', {}, appTree);
162+
const fileContent = getFileContent(tree, '/projects/material/src/app/app.module.ts');
159163

160-
it('should not add @angular/animations to package.json', () => {
161-
// By default, the Angular workspace schematic sets up "@angular/animations". In order
162-
// to verify that we don't add the animations if the Animations are not installed, we need
163-
// to remove the dependency for this unit test tree.
164-
removePackageJsonDependency(appTree, '@angular/animations');
164+
expect(fileContent).toContain('BrowserAnimationsModule',
165+
'Expected the project app module to import the "BrowserAnimationsModule".');
166+
});
165167

166-
const tree = runner.runSchematic('ng-add', {animations: false}, appTree);
167-
const packageJson = JSON.parse(getFileContent(tree, '/package.json'));
168+
it('should not add BrowserAnimationsModule if NoopAnimationsModule is set up', () => {
169+
const workspace = getWorkspace(appTree);
170+
const project = getProjectFromWorkspace(workspace);
171+
172+
// Simulate the case where a developer uses `ng-add` on an Angular CLI project which already
173+
// explicitly uses the `NoopAnimationsModule`. It would be wrong to forcibly enable browser
174+
// animations without knowing what other components would be affected. In this case, we
175+
// just print a warning message.
176+
addModuleImportToRootModule(appTree, 'NoopAnimationsModule',
177+
'@angular/platform-browser/animations', project);
178+
179+
spyOn(console, 'warn');
180+
runner.runSchematic('ng-add-setup-project', {}, appTree);
168181

169-
expect(packageJson.dependencies['@angular/animations'])
170-
.toBeUndefined(`Expected '@angular/animations' to be not added to the package.json`);
182+
expect(console.warn).toHaveBeenCalledWith(
183+
jasmine.stringMatching(/Could not set up "BrowserAnimationsModule"/));
171184
});
185+
});
172186

173-
it('should not add the BrowserAnimationsModule to the project module', () => {
174-
const tree = runner.runSchematic('ng-add', {gestures: false}, appTree);
187+
describe('animations disabled', () => {
188+
it('should add the NoopAnimationsModule to the project module', () => {
189+
const tree = runner.runSchematic('ng-add-setup-project', {animations: false}, appTree);
175190
const fileContent = getFileContent(tree, '/projects/material/src/app/app.module.ts');
176191

177-
expect(fileContent).not.toContain('BrowserAnimationsModule',
178-
'Expected the project app module to not import the "BrowserAnimationsModule".');
192+
expect(fileContent).toContain('NoopAnimationsModule',
193+
'Expected the project app module to import the "NoopAnimationsModule".');
194+
});
195+
196+
it('should not add NoopAnimationsModule if BrowserAnimationsModule is set up', () => {
197+
const workspace = getWorkspace(appTree);
198+
const project = getProjectFromWorkspace(workspace);
199+
200+
// Simulate the case where a developer uses `ng-add` on an Angular CLI project which already
201+
// explicitly uses the `BrowserAnimationsModule`. It would be wrong to forcibly change
202+
// to noop animations.
203+
const fileContent = addModuleImportToRootModule(appTree, 'BrowserAnimationsModule',
204+
'@angular/platform-browser/animations', project);
205+
206+
expect(fileContent).not.toContain('NoopAnimationsModule',
207+
'Expected the project app module to not import the "NoopAnimationsModule".');
179208
});
180209
});
181210
});

src/lib/schematics/ng-add/index.ts

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,18 +21,15 @@ import {hammerjsVersion, materialVersion, requiredAngularVersionRange} from './v
2121
*/
2222
export default function(options: Schema): Rule {
2323
return (host: Tree, context: SchematicContext) => {
24+
// Version tag of the `@angular/core` dependency that has been loaded from the `package.json`
25+
// of the CLI project. This tag should be preferred because all Angular dependencies should
26+
// have the same version tag if possible.
27+
const ngCoreVersionTag = getPackageVersionFromPackageJson(host, '@angular/core');
28+
2429
addPackageToPackageJson(host, '@angular/cdk', `^${materialVersion}`);
2530
addPackageToPackageJson(host, '@angular/material', `^${materialVersion}`);
26-
27-
if (options.animations) {
28-
// Version tag of the `@angular/core` dependency that has been loaded from the `package.json`
29-
// of the CLI project. This tag should be preferred because all Angular dependencies should
30-
// have the same version tag if possible.
31-
const ngCoreVersionTag = getPackageVersionFromPackageJson(host, '@angular/core');
32-
33-
addPackageToPackageJson(host, '@angular/animations',
34-
ngCoreVersionTag || requiredAngularVersionRange);
35-
}
31+
addPackageToPackageJson(host, '@angular/animations',
32+
ngCoreVersionTag || requiredAngularVersionRange);
3633

3734
if (options.gestures) {
3835
addPackageToPackageJson(host, 'hammerjs', hammerjsVersion);

src/lib/schematics/ng-add/schema.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@
2626
"animations": {
2727
"type": "boolean",
2828
"default": true,
29-
"description": "Whether animations should be set up or not.",
30-
"x-prompt": "Set up animations for Angular Material?"
29+
"description": "Whether Angular browser animations should be set up or not.",
30+
"x-prompt": "Set up browser animations for Angular Material?"
3131
}
3232
},
3333
"required": []

src/lib/schematics/ng-add/schema.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ export interface Schema {
1414
/** Whether gesture support should be set up or not. */
1515
gestures: boolean;
1616

17-
/** Whether animations should be set up or not. */
17+
/** Whether Angular browser animations should be set up or not. */
1818
animations: boolean;
1919

2020
/** Name of pre-built theme to install. */

src/lib/schematics/ng-add/setup-project.ts

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,25 @@ import {chain, noop, Rule, SchematicsException, Tree} from '@angular-devkit/sche
1010
import {
1111
addModuleImportToRootModule,
1212
getProjectFromWorkspace,
13+
getProjectMainFile,
1314
getProjectStyleFile,
15+
hasNgModuleImport,
1416
} from '@angular/cdk/schematics';
17+
import {red, bold} from 'chalk';
1518
import {getWorkspace} from '@schematics/angular/utility/config';
19+
import {getAppModulePath} from '@schematics/angular/utility/ng-ast-utils';
1620
import * as parse5 from 'parse5';
1721
import {addFontsToIndex} from './fonts/material-fonts';
1822
import {addHammerJsToMain} from './gestures/hammerjs-import';
1923
import {Schema} from './schema';
2024
import {addThemeToAppStyles} from './theming/theming';
2125

26+
/** Name of the Angular module that enables Angular browser animations. */
27+
const browserAnimationsModuleName = 'BrowserAnimationsModule';
28+
29+
/** Name of the module that switches Angular animations to a noop implementation. */
30+
const noopAnimationsModuleName = 'NoopAnimationsModule';
31+
2232
/**
2333
* Scaffolds the basics of a Angular Material application, this includes:
2434
* - Add Packages to package.json
@@ -32,22 +42,44 @@ export default function(options: Schema): Rule {
3242
}
3343

3444
return chain([
35-
options && options.animations ? addAnimationsModule(options) : noop(),
3645
options && options.gestures ? addHammerJsToMain(options) : noop(),
46+
addAnimationsModule(options),
3747
addThemeToAppStyles(options),
3848
addFontsToIndex(options),
3949
addMaterialAppStyles(options),
4050
]);
4151
}
4252

43-
/** Adds the BrowserAnimationModule to the app module of the specified project. */
53+
/**
54+
* Adds an animation module to the root module of the specified project. In case the "animations"
55+
* option is set to false, we still add the `NoopAnimationsModule` because otherwise various
56+
* components of Angular Material will throw an exception.
57+
*/
4458
function addAnimationsModule(options: Schema) {
4559
return (host: Tree) => {
4660
const workspace = getWorkspace(host);
4761
const project = getProjectFromWorkspace(workspace, options.project);
62+
const appModulePath = getAppModulePath(host, getProjectMainFile(project));
63+
64+
if (options.animations) {
65+
// In case the project explicitly uses the noop animations module, do print an error message
66+
// that makes the user aware of the fact that we won't automatically set up animations.
67+
// We won't setup the animations because this means that we would magically replace the
68+
// animations module of the CLI project.
69+
if (hasNgModuleImport(host, appModulePath, noopAnimationsModuleName)) {
70+
return console.warn(red(`Could not set up "${bold(browserAnimationsModuleName)}" ` +
71+
`because "${bold(noopAnimationsModuleName)}" is already imported. Please manually ` +
72+
`set up browser animations.`));
73+
}
4874

49-
addModuleImportToRootModule(host, 'BrowserAnimationsModule',
50-
'@angular/platform-browser/animations', project);
75+
addModuleImportToRootModule(host, browserAnimationsModuleName,
76+
'@angular/platform-browser/animations', project);
77+
} else if (!hasNgModuleImport(host, appModulePath, browserAnimationsModuleName)) {
78+
// Do not add the noop animations module if the project explicitly uses the browser
79+
// animations module already.
80+
addModuleImportToRootModule(host, noopAnimationsModuleName,
81+
'@angular/platform-browser/animations', project);
82+
}
5183

5284
return host;
5385
};

0 commit comments

Comments
 (0)