Skip to content

fix(ng-add): allow using noop animations #13429

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 78 additions & 0 deletions src/cdk/schematics/utils/ast/ng-module-imports.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

import {Tree} from '@angular-devkit/schematics';
import * as ts from 'typescript';

/**
* Whether the Angular module in the given path imports the specifed module class name.
*/
export function hasNgModuleImport(tree: Tree, modulePath: string, className: string): boolean {
const moduleFileContent = tree.read(modulePath);

if (!moduleFileContent) {
throw new Error(`Could not read Angular module file: ${modulePath}`);
}

const parsedFile = ts.createSourceFile(modulePath, moduleFileContent.toString(),
ts.ScriptTarget.Latest, true);
let ngModuleMetadata: ts.ObjectLiteralExpression | null = null;

const findModuleDecorator = (node: ts.Node) => {
if (ts.isDecorator(node) && ts.isCallExpression(node.expression) &&
isNgModuleCallExpression(node.expression)) {
ngModuleMetadata = node.expression.arguments[0] as ts.ObjectLiteralExpression;
return;
}

ts.forEachChild(node, findModuleDecorator);
};

ts.forEachChild(parsedFile, findModuleDecorator);

if (!ngModuleMetadata) {
throw new Error(`Could not find NgModule declaration inside: "${modulePath}"`);
}

for (let property of ngModuleMetadata!.properties) {
if (!ts.isPropertyAssignment(property) || property.name.getText() !== 'imports' ||
!ts.isArrayLiteralExpression(property.initializer)) {
continue;
}

if (property.initializer.elements.some(element => element.getText() === className)) {
return true;
}
}

return false;
}

/**
* Resolves the last identifier that is part of the given expression. This helps resolving
* identifiers of nested property access expressions (e.g. myNamespace.core.NgModule).
*/
function resolveIdentifierOfExpression(expression: ts.Expression): ts.Identifier | null {
if (ts.isIdentifier(expression)) {
return expression;
} else if (ts.isPropertyAccessExpression(expression)) {
return resolveIdentifierOfExpression(expression.expression);
}
return null;
}

/** Whether the specified call expression is referring to a NgModule definition. */
function isNgModuleCallExpression(callExpression: ts.CallExpression): boolean {
if (!callExpression.arguments.length ||
!ts.isObjectLiteralExpression(callExpression.arguments[0])) {
return false;
}

const decoratorIdentifier = resolveIdentifierOfExpression(callExpression.expression);
return decoratorIdentifier ? decoratorIdentifier.text === 'NgModule' : false;
}
1 change: 1 addition & 0 deletions src/cdk/schematics/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/

export * from './ast';
export * from './ast/ng-module-imports';
export * from './build-component';
export * from './get-project';
export * from './parse5-element';
Expand Down
65 changes: 65 additions & 0 deletions src/lib/schematics/ng-add/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {WorkspaceProject} from '@angular-devkit/core/src/workspace';
import {Tree} from '@angular-devkit/schematics';
import {SchematicTestRunner} from '@angular-devkit/schematics/testing';
import {
addModuleImportToRootModule,
createTestApp,
getProjectFromWorkspace,
getProjectStyleFile,
Expand All @@ -27,7 +28,19 @@ describe('ng-add schematic', () => {
`Expected "${filePath}" to be added to the project styles in the workspace.`);
}

/** Removes the specified dependency from the /package.json in the given tree. */
function removePackageJsonDependency(tree: Tree, dependencyName: string) {
const packageContent = JSON.parse(getFileContent(tree, '/package.json'));
delete packageContent.dependencies[dependencyName];
tree.overwrite('/package.json', JSON.stringify(packageContent, null, 2));
}

it('should update package.json', () => {
// By default, the Angular workspace schematic sets up "@angular/animations". In order
// to verify that we would set up the dependency properly if someone doesn't have the
// animations installed already, we remove the animations dependency explicitly.
removePackageJsonDependency(appTree, '@angular/animations');

const tree = runner.runSchematic('ng-add', {}, appTree);
const packageJson = JSON.parse(getFileContent(tree, '/package.json'));
const dependencies = packageJson.dependencies;
Expand Down Expand Up @@ -142,4 +155,56 @@ describe('ng-add schematic', () => {
'Expected the project main file to not contain a HammerJS import.');
});
});

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

expect(fileContent).toContain('BrowserAnimationsModule',
'Expected the project app module to import the "BrowserAnimationsModule".');
});

it('should not add BrowserAnimationsModule if NoopAnimationsModule is set up', () => {
const workspace = getWorkspace(appTree);
const project = getProjectFromWorkspace(workspace);

// Simulate the case where a developer uses `ng-add` on an Angular CLI project which already
// explicitly uses the `NoopAnimationsModule`. It would be wrong to forcibly enable browser
// animations without knowing what other components would be affected. In this case, we
// just print a warning message.
addModuleImportToRootModule(appTree, 'NoopAnimationsModule',
'@angular/platform-browser/animations', project);

spyOn(console, 'warn');
runner.runSchematic('ng-add-setup-project', {}, appTree);

expect(console.warn).toHaveBeenCalledWith(
jasmine.stringMatching(/Could not set up "BrowserAnimationsModule"/));
});
});

describe('animations disabled', () => {
it('should add the NoopAnimationsModule to the project module', () => {
const tree = runner.runSchematic('ng-add-setup-project', {animations: false}, appTree);
const fileContent = getFileContent(tree, '/projects/material/src/app/app.module.ts');

expect(fileContent).toContain('NoopAnimationsModule',
'Expected the project app module to import the "NoopAnimationsModule".');
});

it('should not add NoopAnimationsModule if BrowserAnimationsModule is set up', () => {
const workspace = getWorkspace(appTree);
const project = getProjectFromWorkspace(workspace);

// Simulate the case where a developer uses `ng-add` on an Angular CLI project which already
// explicitly uses the `BrowserAnimationsModule`. It would be wrong to forcibly change
// to noop animations.
const fileContent = addModuleImportToRootModule(appTree, 'BrowserAnimationsModule',
'@angular/platform-browser/animations', project);

expect(fileContent).not.toContain('NoopAnimationsModule',
'Expected the project app module to not import the "NoopAnimationsModule".');
});
});
});
6 changes: 6 additions & 0 deletions src/lib/schematics/ng-add/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@
"default": true,
"description": "Whether gesture support should be set up or not.",
"x-prompt": "Set up HammerJS for gesture recognition?"
},
"animations": {
"type": "boolean",
"default": true,
"description": "Whether Angular browser animations should be set up or not.",
"x-prompt": "Set up browser animations for Angular Material?"
}
},
"required": []
Expand Down
3 changes: 3 additions & 0 deletions src/lib/schematics/ng-add/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ export interface Schema {
/** Whether gesture support should be set up or not. */
gestures: boolean;

/** Whether Angular browser animations should be set up or not. */
animations: boolean;

/** Name of pre-built theme to install. */
theme: 'indigo-pink' | 'deeppurple-amber' | 'pink-bluegrey' | 'purple-green' | 'custom';
}
42 changes: 37 additions & 5 deletions src/lib/schematics/ng-add/setup-project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,25 @@ import {chain, noop, Rule, SchematicsException, Tree} from '@angular-devkit/sche
import {
addModuleImportToRootModule,
getProjectFromWorkspace,
getProjectMainFile,
getProjectStyleFile,
hasNgModuleImport,
} from '@angular/cdk/schematics';
import {red, bold} from 'chalk';
import {getWorkspace} from '@schematics/angular/utility/config';
import {getAppModulePath} from '@schematics/angular/utility/ng-ast-utils';
import * as parse5 from 'parse5';
import {addFontsToIndex} from './fonts/material-fonts';
import {addHammerJsToMain} from './gestures/hammerjs-import';
import {Schema} from './schema';
import {addThemeToAppStyles} from './theming/theming';

/** Name of the Angular module that enables Angular browser animations. */
const browserAnimationsModuleName = 'BrowserAnimationsModule';

/** Name of the module that switches Angular animations to a noop implementation. */
const noopAnimationsModuleName = 'NoopAnimationsModule';

/**
* Scaffolds the basics of a Angular Material application, this includes:
* - Add Packages to package.json
Expand All @@ -33,21 +43,43 @@ export default function(options: Schema): Rule {

return chain([
options && options.gestures ? addHammerJsToMain(options) : noop(),
addAnimationsModule(options),
addThemeToAppStyles(options),
addAnimationRootConfig(options),
addFontsToIndex(options),
addMaterialAppStyles(options),
]);
}

/** Add browser animation module to the app module file. */
function addAnimationRootConfig(options: Schema) {
/**
* Adds an animation module to the root module of the specified project. In case the "animations"
* option is set to false, we still add the `NoopAnimationsModule` because otherwise various
* components of Angular Material will throw an exception.
*/
function addAnimationsModule(options: Schema) {
return (host: Tree) => {
const workspace = getWorkspace(host);
const project = getProjectFromWorkspace(workspace, options.project);
const appModulePath = getAppModulePath(host, getProjectMainFile(project));

if (options.animations) {
// In case the project explicitly uses the NoopAnimationsModule, we should print a warning
// message that makes the user aware of the fact that we won't automatically set up
// animations. If we would add the BrowserAnimationsModule while the NoopAnimationsModule
// is already configured, we would cause unexpected behavior and runtime exceptions.
if (hasNgModuleImport(host, appModulePath, noopAnimationsModuleName)) {
return console.warn(red(`Could not set up "${bold(browserAnimationsModuleName)}" ` +
`because "${bold(noopAnimationsModuleName)}" is already imported. Please manually ` +
`set up browser animations.`));
}

addModuleImportToRootModule(host, 'BrowserAnimationsModule',
'@angular/platform-browser/animations', project);
addModuleImportToRootModule(host, browserAnimationsModuleName,
'@angular/platform-browser/animations', project);
} else if (!hasNgModuleImport(host, appModulePath, browserAnimationsModuleName)) {
// Do not add the NoopAnimationsModule module if the project already explicitly uses
// the BrowserAnimationsModule.
addModuleImportToRootModule(host, noopAnimationsModuleName,
'@angular/platform-browser/animations', project);
}

return host;
};
Expand Down