Skip to content

Commit 15a1ab7

Browse files
devversionvivian-hu-zz
authored andcommitted
fix(ng-add): allow using noop animations (#13429)
* Currently we always install `@angular/animations` and add the `BrowserAnimationsModule` to the project module. There should be a prompt/option that allows developers to use the `NoopAnimationsModule`. * Also no longer adds the `BrowserAnimationsModule` if the project already uses the `NoopAnimationsModule` (avoiding magic). Same applies for the `NoopAnimationsModule`. We won't add the noop animations module if the browser animations module is configured. Handle existing animation modules
1 parent 7b3de83 commit 15a1ab7

File tree

6 files changed

+190
-5
lines changed

6 files changed

+190
-5
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: 65 additions & 0 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,
@@ -27,7 +28,19 @@ describe('ng-add schematic', () => {
2728
`Expected "${filePath}" to be added to the project styles in the workspace.`);
2829
}
2930

31+
/** Removes the specified dependency from the /package.json in the given tree. */
32+
function removePackageJsonDependency(tree: Tree, dependencyName: string) {
33+
const packageContent = JSON.parse(getFileContent(tree, '/package.json'));
34+
delete packageContent.dependencies[dependencyName];
35+
tree.overwrite('/package.json', JSON.stringify(packageContent, null, 2));
36+
}
37+
3038
it('should update package.json', () => {
39+
// By default, the Angular workspace schematic sets up "@angular/animations". In order
40+
// to verify that we would set up the dependency properly if someone doesn't have the
41+
// animations installed already, we remove the animations dependency explicitly.
42+
removePackageJsonDependency(appTree, '@angular/animations');
43+
3144
const tree = runner.runSchematic('ng-add', {}, appTree);
3245
const packageJson = JSON.parse(getFileContent(tree, '/package.json'));
3346
const dependencies = packageJson.dependencies;
@@ -142,4 +155,56 @@ describe('ng-add schematic', () => {
142155
'Expected the project main file to not contain a HammerJS import.');
143156
});
144157
});
158+
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');
163+
164+
expect(fileContent).toContain('BrowserAnimationsModule',
165+
'Expected the project app module to import the "BrowserAnimationsModule".');
166+
});
167+
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);
181+
182+
expect(console.warn).toHaveBeenCalledWith(
183+
jasmine.stringMatching(/Could not set up "BrowserAnimationsModule"/));
184+
});
185+
});
186+
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);
190+
const fileContent = getFileContent(tree, '/projects/material/src/app/app.module.ts');
191+
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".');
208+
});
209+
});
145210
});

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,12 @@
2222
"default": true,
2323
"description": "Whether gesture support should be set up or not.",
2424
"x-prompt": "Set up HammerJS for gesture recognition?"
25+
},
26+
"animations": {
27+
"type": "boolean",
28+
"default": true,
29+
"description": "Whether Angular browser animations should be set up or not.",
30+
"x-prompt": "Set up browser animations for Angular Material?"
2531
}
2632
},
2733
"required": []

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ export interface Schema {
1414
/** Whether gesture support should be set up or not. */
1515
gestures: boolean;
1616

17+
/** Whether Angular browser animations should be set up or not. */
18+
animations: boolean;
19+
1720
/** Name of pre-built theme to install. */
1821
theme: 'indigo-pink' | 'deeppurple-amber' | 'pink-bluegrey' | 'purple-green' | 'custom';
1922
}

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

Lines changed: 37 additions & 5 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
@@ -33,21 +43,43 @@ export default function(options: Schema): Rule {
3343

3444
return chain([
3545
options && options.gestures ? addHammerJsToMain(options) : noop(),
46+
addAnimationsModule(options),
3647
addThemeToAppStyles(options),
37-
addAnimationRootConfig(options),
3848
addFontsToIndex(options),
3949
addMaterialAppStyles(options),
4050
]);
4151
}
4252

43-
/** Add browser animation module to the app module file. */
44-
function addAnimationRootConfig(options: Schema) {
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+
*/
58+
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 NoopAnimationsModule, we should print a warning
66+
// message that makes the user aware of the fact that we won't automatically set up
67+
// animations. If we would add the BrowserAnimationsModule while the NoopAnimationsModule
68+
// is already configured, we would cause unexpected behavior and runtime exceptions.
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 NoopAnimationsModule module if the project already explicitly uses
79+
// the BrowserAnimationsModule.
80+
addModuleImportToRootModule(host, noopAnimationsModuleName,
81+
'@angular/platform-browser/animations', project);
82+
}
5183

5284
return host;
5385
};

0 commit comments

Comments
 (0)