From a9769d5b0efb50fde94e5efecfc984a787ac356e Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Fri, 12 Jan 2024 15:45:33 +0100 Subject: [PATCH] fix(material/schematics): import async animations and remove deprecated function usages * Reworks the `ng add` schematic so it doesn't depend on deprecated functions. * Imports the async animations instead of the eagerly-loaded ones. --- src/material/schematics/ng-add/index.spec.ts | 207 ++++++------------ .../schematics/ng-add/setup-project.ts | 141 +----------- 2 files changed, 76 insertions(+), 272 deletions(-) diff --git a/src/material/schematics/ng-add/index.spec.ts b/src/material/schematics/ng-add/index.spec.ts index f4e0f09f89c0..bf600191930f 100644 --- a/src/material/schematics/ng-add/index.spec.ts +++ b/src/material/schematics/ng-add/index.spec.ts @@ -2,7 +2,6 @@ import {normalize, workspaces, logging} from '@angular-devkit/core'; import {Tree} from '@angular-devkit/schematics'; import {SchematicTestRunner} from '@angular-devkit/schematics/testing'; import { - addModuleImportToRootModule, getProjectFromWorkspace, getProjectIndexFiles, getProjectStyleFile, @@ -186,151 +185,71 @@ describe('ng-add schematic', () => { ); }); - describe('animations enabled', () => { - it('should add the BrowserAnimationsModule to the project module', async () => { - const tree = await runner.runSchematic('ng-add-setup-project', baseOptions, appTree); - const fileContent = getFileContent(tree, '/projects/material/src/app/app.module.ts'); - - expect(fileContent) - .withContext('Expected the project app module to import the "BrowserAnimationsModule".') - .toContain('BrowserAnimationsModule'); - }); - - it('should not add BrowserAnimationsModule if NoopAnimationsModule is set up', async () => { - const workspace = await getWorkspace(appTree); - const project = getProjectFromWorkspace(workspace, baseOptions.project); - - // 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, - ); - - await runner.runSchematic('ng-add-setup-project', baseOptions, appTree); - - expect(errorOutput.length).toBe(1); - expect(errorOutput[0]).toMatch(/Could not set up "BrowserAnimationsModule"/); - }); - - it('should add the provideAnimations to a bootstrapApplication call', async () => { - appTree.delete('/projects/material/src/app/app.module.ts'); - appTree.create( - '/projects/material/src/app/app.config.ts', - ` - export const appConfig = { - providers: [{ provide: 'foo', useValue: 1 }] - }; - `, - ); - appTree.overwrite( - '/projects/material/src/main.ts', - ` - import { bootstrapApplication } from '@angular/platform-browser'; - import { AppComponent } from './app/app.component'; - import { appConfig } from './app/app.config'; - - bootstrapApplication(AppComponent, appConfig); - `, - ); - - const tree = await runner.runSchematic('ng-add-setup-project', baseOptions, appTree); - const fileContent = getFileContent(tree, '/projects/material/src/app/app.config.ts'); + it('should add provideAnimationsAsync to the project module', async () => { + const tree = await runner.runSchematic('ng-add-setup-project', baseOptions, appTree); + const fileContent = getFileContent(tree, '/projects/material/src/app/app.module.ts'); - expect(fileContent).toContain( - `import { provideAnimations } from '@angular/platform-browser/animations';`, - ); - expect(fileContent).toContain(`[{ provide: 'foo', useValue: 1 }, provideAnimations()]`); - }); + expect(fileContent).toContain('provideAnimationsAsync()'); + expect(fileContent).toContain( + `import { provideAnimationsAsync } from '@angular/platform-browser/animations/async';`, + ); + }); - it('should not add provideAnimations if provideNoopAnimations is set up in a bootstrapApplication call', async () => { - appTree.delete('/projects/material/src/app/app.module.ts'); - appTree.create( - '/projects/material/src/app/app.config.ts', - ` - import { provideNoopAnimations } from '@angular/platform-browser/animations'; + it('should add the provideAnimationsAsync to a bootstrapApplication call', async () => { + appTree.delete('/projects/material/src/app/app.module.ts'); + appTree.create( + '/projects/material/src/app/app.config.ts', + ` + export const appConfig = { + providers: [{ provide: 'foo', useValue: 1 }] + }; + `, + ); + appTree.overwrite( + '/projects/material/src/main.ts', + ` + import { bootstrapApplication } from '@angular/platform-browser'; + import { AppComponent } from './app/app.component'; + import { appConfig } from './app/app.config'; - export const appConfig = { - providers: [{ provide: 'foo', useValue: 1 }, provideNoopAnimations()] - }; + bootstrapApplication(AppComponent, appConfig); `, - ); - appTree.overwrite( - '/projects/material/src/main.ts', - ` - import { bootstrapApplication } from '@angular/platform-browser'; - import { AppComponent } from './app/app.component'; - import { appConfig } from './app/app.config'; - - bootstrapApplication(AppComponent, appConfig); - `, - ); + ); - await runner.runSchematic('ng-add-setup-project', baseOptions, appTree); + const tree = await runner.runSchematic('ng-add-setup-project', baseOptions, appTree); + const fileContent = getFileContent(tree, '/projects/material/src/app/app.config.ts'); - expect(errorOutput.length).toBe(1); - expect(errorOutput[0]).toMatch( - /Could not add "provideAnimations" because "provideNoopAnimations" is already provided/, - ); - }); + expect(fileContent).toContain( + `import { provideAnimationsAsync } from '@angular/platform-browser/animations/async';`, + ); + expect(fileContent).toContain(`[{ provide: 'foo', useValue: 1 }, provideAnimationsAsync()]`); }); - describe('animations disabled', () => { - it('should add the NoopAnimationsModule to the project module', async () => { - const tree = await runner.runSchematic( - 'ng-add-setup-project', - {...baseOptions, animations: 'disabled'}, - appTree, - ); - const fileContent = getFileContent(tree, '/projects/material/src/app/app.module.ts'); - - expect(fileContent) - .withContext('Expected the project app module to import the "NoopAnimationsModule".') - .toContain('NoopAnimationsModule'); - }); - - it('should not add NoopAnimationsModule if BrowserAnimationsModule is set up', async () => { - const workspace = await getWorkspace(appTree); - const project = getProjectFromWorkspace(workspace, baseOptions.project); - - // 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. - addModuleImportToRootModule( - appTree, - 'BrowserAnimationsModule', - '@angular/platform-browser/animations', - project, - ); - - const fileContent = getFileContent(appTree, '/projects/material/src/app/app.module.ts'); + it("should add the provideAnimationAsync('noop') to the project module if animations are disabled", async () => { + const tree = await runner.runSchematic( + 'ng-add-setup-project', + {...baseOptions, animations: 'disabled'}, + appTree, + ); + const fileContent = getFileContent(tree, '/projects/material/src/app/app.module.ts'); - expect(fileContent) - .not.withContext( - 'Expected the project app module to not import the "NoopAnimationsModule".', - ) - .toContain('NoopAnimationsModule'); - }); + expect(fileContent).toContain(`provideAnimationsAsync('noop')`); + expect(fileContent).toContain( + `import { provideAnimationsAsync } from '@angular/platform-browser/animations/async';`, + ); }); - describe('animations excluded', () => { - it('should not add any animations code if animations are excluded', async () => { - const tree = await runner.runSchematic( - 'ng-add-setup-project', - {...baseOptions, animations: 'excluded'}, - appTree, - ); - const fileContent = getFileContent(tree, '/projects/material/src/app/app.module.ts'); + it('should not add any animations code if animations are excluded', async () => { + const tree = await runner.runSchematic( + 'ng-add-setup-project', + {...baseOptions, animations: 'excluded'}, + appTree, + ); + const fileContent = getFileContent(tree, '/projects/material/src/app/app.module.ts'); - expect(fileContent).not.toContain('NoopAnimationsModule'); - expect(fileContent).not.toContain('BrowserAnimationsModule'); - expect(fileContent).not.toContain('@angular/platform-browser/animations'); - expect(fileContent).not.toContain('@angular/animations'); - }); + expect(fileContent).not.toContain('provideAnimationsAsync'); + expect(fileContent).not.toContain('@angular/platform-browser/animations'); + expect(fileContent).not.toContain('@angular/animations'); }); describe('custom project builders', () => { @@ -376,7 +295,7 @@ describe('ng-add schematic', () => { overwriteTargetBuilder(appTree, 'build', 'thirdparty-builder'); await expectAsync( runner.runSchematic('ng-add-setup-project', baseOptions, appTree), - ).toBeRejectedWithError(/not using the default builders.*build/); + ).toBeRejected(); }); it('should warn if the "test" target has been changed', async () => { @@ -658,13 +577,14 @@ describe('ng-add schematic', () => { ); }); - it('should add the BrowserAnimationsModule to the project module', async () => { + it('should add the provideAnimationsAsync to the project module', async () => { const tree = await runner.runSchematic('ng-add-setup-project', baseOptions, appTree); const fileContent = getFileContent(tree, '/projects/material/src/app/app.module.ts'); - expect(fileContent) - .withContext('Expected the project app module to import the "BrowserAnimationsModule".') - .toContain('BrowserAnimationsModule'); + expect(fileContent).toContain('provideAnimationsAsync()'); + expect(fileContent).toContain( + `import { provideAnimationsAsync } from '@angular/platform-browser/animations/async';`, + ); }); }); @@ -727,13 +647,14 @@ describe('ng-add schematic', () => { ); }); - it('should add the BrowserAnimationsModule to the project module', async () => { + it('should add the provideAnimationsAsync to the project module', async () => { const tree = await runner.runSchematic('ng-add-setup-project', baseOptions, appTree); const fileContent = getFileContent(tree, '/projects/material/src/app/app.module.ts'); - expect(fileContent) - .withContext('Expected the project app module to import the "BrowserAnimationsModule".') - .toContain('BrowserAnimationsModule'); + expect(fileContent).toContain('provideAnimationsAsync()'); + expect(fileContent).toContain( + `import { provideAnimationsAsync } from '@angular/platform-browser/animations/async';`, + ); }); }); }); diff --git a/src/material/schematics/ng-add/setup-project.ts b/src/material/schematics/ng-add/setup-project.ts index ea59b97b7fc7..e3ec2e8a0906 100644 --- a/src/material/schematics/ng-add/setup-project.ts +++ b/src/material/schematics/ng-add/setup-project.ts @@ -6,22 +6,10 @@ * found in the LICENSE file at https://angular.io/license */ -import {chain, Rule, SchematicContext, Tree} from '@angular-devkit/schematics'; -import { - addModuleImportToRootModule, - getAppModulePath, - getProjectFromWorkspace, - getProjectMainFile, - getProjectStyleFile, - hasNgModuleImport, - isStandaloneApp, -} from '@angular/cdk/schematics'; -import { - importsProvidersFrom, - addFunctionalProvidersToStandaloneBootstrap, - callsProvidersFunction, -} from '@schematics/angular/private/components'; -import {getWorkspace, ProjectDefinition} from '@schematics/angular/utility/workspace'; +import {chain, noop, Rule, SchematicContext, Tree} from '@angular-devkit/schematics'; +import {getProjectFromWorkspace, getProjectStyleFile} from '@angular/cdk/schematics'; +import {getWorkspace} from '@schematics/angular/utility/workspace'; +import {addRootProvider} from '@schematics/angular/utility'; import {ProjectType} from '@schematics/angular/utility/workspace-models'; import {addFontsToIndex} from './fonts/material-fonts'; import {Schema} from './schema'; @@ -40,7 +28,14 @@ export default function (options: Schema): Rule { if (project.extensions['projectType'] === ProjectType.Application) { return chain([ - addAnimationsModule(options), + options.animations === 'excluded' + ? noop() + : addRootProvider(options.project, ({code, external}) => { + return code`${external( + 'provideAnimationsAsync', + '@angular/platform-browser/animations/async', + )}(${options.animations === 'disabled' ? `'noop'` : ''})`; + }), addThemeToAppStyles(options), addFontsToIndex(options), addMaterialAppStyles(options), @@ -57,118 +52,6 @@ export default function (options: Schema): Rule { }; } -/** - * 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 async (host: Tree, context: SchematicContext) => { - const workspace = await getWorkspace(host); - const project = getProjectFromWorkspace(workspace, options.project); - const mainFilePath = getProjectMainFile(project); - - if (isStandaloneApp(host, mainFilePath)) { - addAnimationsToStandaloneApp(host, mainFilePath, context, options); - } else { - addAnimationsToNonStandaloneApp(host, project, mainFilePath, context, options); - } - }; -} - -/** Adds the animations module to an app that is bootstrap using the standalone component APIs. */ -function addAnimationsToStandaloneApp( - host: Tree, - mainFile: string, - context: SchematicContext, - options: Schema, -) { - const animationsFunction = 'provideAnimations'; - const noopAnimationsFunction = 'provideNoopAnimations'; - - if (options.animations === 'enabled') { - // In case the project explicitly uses provideNoopAnimations, 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 provideAnimations while provideNoopAnimations - // is already configured, we would cause unexpected behavior and runtime exceptions. - if (callsProvidersFunction(host, mainFile, noopAnimationsFunction)) { - context.logger.error( - `Could not add "${animationsFunction}" ` + - `because "${noopAnimationsFunction}" is already provided.`, - ); - context.logger.info(`Please manually set up browser animations.`); - } else { - addFunctionalProvidersToStandaloneBootstrap( - host, - mainFile, - animationsFunction, - '@angular/platform-browser/animations', - ); - } - } else if ( - options.animations === 'disabled' && - !importsProvidersFrom(host, mainFile, animationsFunction) - ) { - // Do not add the provideNoopAnimations if the project already explicitly uses - // the provideAnimations. - addFunctionalProvidersToStandaloneBootstrap( - host, - mainFile, - noopAnimationsFunction, - '@angular/platform-browser/animations', - ); - } -} - -/** - * Adds the animations module to an app that is bootstrap - * using the non-standalone component APIs. - */ -function addAnimationsToNonStandaloneApp( - host: Tree, - project: ProjectDefinition, - mainFile: string, - context: SchematicContext, - options: Schema, -) { - const browserAnimationsModuleName = 'BrowserAnimationsModule'; - const noopAnimationsModuleName = 'NoopAnimationsModule'; - const appModulePath = getAppModulePath(host, mainFile); - - if (options.animations === 'enabled') { - // 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)) { - context.logger.error( - `Could not set up "${browserAnimationsModuleName}" ` + - `because "${noopAnimationsModuleName}" is already imported.`, - ); - context.logger.info(`Please manually set up browser animations.`); - } else { - addModuleImportToRootModule( - host, - browserAnimationsModuleName, - '@angular/platform-browser/animations', - project, - ); - } - } else if ( - options.animations === 'disabled' && - !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, - ); - } -} - /** * Adds custom Material styles to the project style file. The custom CSS sets up the Roboto font * and reset the default browser body margin.