From eec6669733ff0ff3a2e7f3ff191ad1b4a8a8494b Mon Sep 17 00:00:00 2001 From: Matthieu Riegler Date: Wed, 15 Oct 2025 00:15:28 +0200 Subject: [PATCH] refactor(migrations): use the `applicationProviders` for the zoneless migration PR #64354 introduced the `applicationProviders` for `bootstrapModule`, this allows a simpler migration output. --- .../migration.spec.ts | 412 ++++++++++++++---- .../bootstrap-options-migration/migration.ts | 290 +++++------- 2 files changed, 456 insertions(+), 246 deletions(-) diff --git a/packages/core/schematics/migrations/bootstrap-options-migration/migration.spec.ts b/packages/core/schematics/migrations/bootstrap-options-migration/migration.spec.ts index c7634e5dde1..9c90b14de2c 100644 --- a/packages/core/schematics/migrations/bootstrap-options-migration/migration.spec.ts +++ b/packages/core/schematics/migrations/bootstrap-options-migration/migration.spec.ts @@ -798,16 +798,13 @@ describe('bootstrap options migration', () => { const actual = fs.readFile(absoluteFrom('/app/app.module.ts')); const expected = ` - import { NgModule, provideZoneChangeDetection } from '@angular/core'; + import { NgModule } from '@angular/core'; import { BrowserModule } from '@angular/platform-browser'; import { AppComponent } from './app.component'; - @NgModule({ providers: [provideZoneChangeDetection()]}) - export class ZoneChangeDetectionModule {} - @NgModule({ declarations: [AppComponent], - imports: [ZoneChangeDetectionModule, BrowserModule], + imports: [BrowserModule], bootstrap: [AppComponent] }) export class AppModule {} @@ -818,10 +815,13 @@ describe('bootstrap options migration', () => { const mainActual = fs.readFile(absoluteFrom('/main.ts')); const mainExpected = ` + import { provideZoneChangeDetection } from "@angular/core"; import { ${platformBrowserFn} } from '@angular/${packageName}'; import { AppModule } from './app/app.module'; - ${platformBrowserFn}().bootstrapModule(AppModule); + ${platformBrowserFn}().bootstrapModule(AppModule, { + applicationProviders: [provideZoneChangeDetection()], + }); `; expect(mainActual.replace(/\s+/g, '')) .withContext(diffText(mainExpected, mainActual)) @@ -1008,16 +1008,13 @@ describe('bootstrap options migration', () => { const actual = fs.readFile(absoluteFrom('/app/app.module.ts')); const expected = ` - import { NgModule, provideZoneChangeDetection } from '@angular/core'; + import { NgModule } from '@angular/core'; import { BrowserModule } from '@angular/platform-browser'; import { AppComponent } from './app.component'; - @NgModule({ providers: [provideZoneChangeDetection({ eventCoalescing: true })] }) - export class ZoneChangeDetectionModule {} - @NgModule({ declarations: [AppComponent], - imports: [ZoneChangeDetectionModule, BrowserModule], + imports: [BrowserModule], providers: [], bootstrap: [AppComponent] }) @@ -1026,6 +1023,20 @@ describe('bootstrap options migration', () => { expect(actual.replace(/\s+/g, '')) .withContext(diffText(expected, actual)) .toEqual(expected.replace(/\s+/g, '')); + + const mainActual = fs.readFile(absoluteFrom('/main.ts')); + const mainExpected = ` + import { provideZoneChangeDetection } from "@angular/core"; + import { ${platformBrowserFn} } from '@angular/${packageName}'; + import { AppModule } from './app/app.module'; + + ${platformBrowserFn}().bootstrapModule(AppModule, { + applicationProviders: [provideZoneChangeDetection({ eventCoalescing: true })], + }); + `; + expect(mainActual.replace(/\s+/g, '')) + .withContext(diffText(mainExpected, mainActual)) + .toEqual(mainExpected.replace(/\s+/g, '')); }); it('should migrate ngZoneRunCoalescing', async () => { @@ -1070,16 +1081,13 @@ describe('bootstrap options migration', () => { const actual = fs.readFile(absoluteFrom('/app/app.module.ts')); const expected = ` - import { NgModule, provideZoneChangeDetection } from '@angular/core'; + import { NgModule } from '@angular/core'; import { BrowserModule } from '@angular/platform-browser'; import { AppComponent } from './app.component'; - @NgModule({providers: [provideZoneChangeDetection({runCoalescing: true})]}) - export class ZoneChangeDetectionModule {} - @NgModule({ declarations: [AppComponent], - imports: [ZoneChangeDetectionModule, BrowserModule], + imports: [BrowserModule], providers: [], bootstrap: [AppComponent] }) @@ -1088,6 +1096,18 @@ describe('bootstrap options migration', () => { expect(actual.replace(/\s+/g, '')) .withContext(diffText(expected, actual)) .toEqual(expected.replace(/\s+/g, '')); + + const mainActual = fs.readFile(absoluteFrom('/main.ts')); + const mainExpected = ` + import { provideZoneChangeDetection } from "@angular/core"; + import { ${platformBrowserFn} } from '@angular/${packageName}'; + import { AppModule } from './app/app.module'; + + ${platformBrowserFn}().bootstrapModule(AppModule, {applicationProviders: [provideZoneChangeDetection({runCoalescing: true})], }); + `; + expect(mainActual.replace(/\s+/g, '')) + .withContext(diffText(mainExpected, mainActual)) + .toEqual(mainExpected.replace(/\s+/g, '')); }); it('should not add provideZoneChangeDetection if provideZonelessChangeDetection is present', async () => { @@ -1263,16 +1283,13 @@ describe('bootstrap options migration', () => { const actual = fs.readFile(absoluteFrom('/app/app.module.ts')); const expected = ` - import { NgModule, provideZoneChangeDetection } from '@angular/core'; + import { NgModule } from '@angular/core'; import { BrowserModule } from '@angular/platform-browser'; import { AppComponent } from './app.component'; - @NgModule({providers: [provideZoneChangeDetection() ]}) - export class ZoneChangeDetectionModule {} - @NgModule({ declarations: [AppComponent], - imports: [ZoneChangeDetectionModule, BrowserModule], + imports: [BrowserModule], bootstrap: [AppComponent] }) export class AppModule {} @@ -1280,6 +1297,18 @@ describe('bootstrap options migration', () => { expect(actual.replace(/\s+/g, '')) .withContext(diffText(expected, actual)) .toEqual(expected.replace(/\s+/g, '')); + + const mainActual = fs.readFile(absoluteFrom('/main.ts')); + const mainExpected = ` + import { provideZoneChangeDetection } from "@angular/core"; + import { ${platformBrowserFn} } from '@angular/${packageName}'; + import { AppModule } from './app/app.module'; + + ${platformBrowserFn}().bootstrapModule(AppModule, {applicationProviders: [provideZoneChangeDetection()],}); + `; + expect(mainActual.replace(/\s+/g, '')) + .withContext(diffText(mainExpected, mainActual)) + .toEqual(mainExpected.replace(/\s+/g, '')); }); it('should migrate ngZone with a custom class', async () => { @@ -1331,22 +1360,13 @@ describe('bootstrap options migration', () => { const actual = fs.readFile(absoluteFrom('/app/app.module.ts')); const expected = ` - import { MyZone } from "./my-zone"; - import { NgModule, NgZone, provideZoneChangeDetection } from '@angular/core'; + import { NgModule } from '@angular/core'; import { BrowserModule } from '@angular/platform-browser'; import { AppComponent } from './app.component'; - @NgModule({ - providers: [ - provideZoneChangeDetection(), - {provide: NgZone, useClass: MyZone} - ] - }) - export class ZoneChangeDetectionModule {} - @NgModule({ declarations: [AppComponent], - imports: [ZoneChangeDetectionModule, BrowserModule], + imports: [BrowserModule], bootstrap: [AppComponent] }) export class AppModule {} @@ -1354,6 +1374,19 @@ describe('bootstrap options migration', () => { expect(actual.replace(/\s+/g, '')) .withContext(diffText(expected, actual)) .toEqual(expected.replace(/\s+/g, '')); + + const mainActual = fs.readFile(absoluteFrom('/main.ts')); + const mainExpected = ` + import { provideZoneChangeDetection } from "@angular/core"; + import { ${platformBrowserFn} } from '@angular/${packageName}'; + import { AppModule } from './app/app.module'; + import { MyZone } from './app/my-zone'; + + ${platformBrowserFn}().bootstrapModule(AppModule, {applicationProviders: [provideZoneChangeDetection(), { provide: NgZone, useClass: MyZone }],}); + `; + expect(mainActual.replace(/\s+/g, '')) + .withContext(diffText(mainExpected, mainActual)) + .toEqual(mainExpected.replace(/\s+/g, '')); }); it('should remove ignoreChangesOutsideZone', async () => { @@ -1397,16 +1430,13 @@ describe('bootstrap options migration', () => { const actual = fs.readFile(absoluteFrom('/app/app.module.ts')); const expected = ` - import { NgModule, provideZoneChangeDetection } from '@angular/core'; + import { NgModule } from '@angular/core'; import { BrowserModule } from '@angular/platform-browser'; import { AppComponent } from './app.component'; - @NgModule({ providers: [provideZoneChangeDetection()] }) - export class ZoneChangeDetectionModule {} - @NgModule({ declarations: [AppComponent], - imports: [ZoneChangeDetectionModule, BrowserModule], + imports: [BrowserModule], bootstrap: [AppComponent] }) export class AppModule {} @@ -1414,6 +1444,18 @@ describe('bootstrap options migration', () => { expect(actual.replace(/\s+/g, '')) .withContext(diffText(expected, actual)) .toEqual(expected.replace(/\s+/g, '')); + + const mainActual = fs.readFile(absoluteFrom('/main.ts')); + const mainExpected = ` + import { provideZoneChangeDetection } from "@angular/core"; + import { ${platformBrowserFn} } from '@angular/${packageName}'; + import { AppModule } from './app/app.module'; + + ${platformBrowserFn}().bootstrapModule(AppModule, {applicationProviders: [provideZoneChangeDetection()],}); + `; + expect(mainActual.replace(/\s+/g, '')) + .withContext(diffText(mainExpected, mainActual)) + .toEqual(mainExpected.replace(/\s+/g, '')); }); it('should not add provideZoneChangeDetection if it is already present', async () => { @@ -1545,6 +1587,75 @@ describe('bootstrap options migration', () => { .toEqual(mainExpected.replace(/\s+/g, '')); }); + it('should keep compiler options if present', async () => { + const {fs} = await runTsurgeMigration(new BootstrapOptionsMigration(), [ + ...typeFiles, + { + name: absoluteFrom('/main.ts'), + isProgramRootFile: true, + contents: ` + import { ${platformBrowserFn} } from '@angular/${packageName}'; + import { AppModule } from './app/app.module'; + + ${platformBrowserFn}().bootstrapModule(AppModule, { preserveWhitespaces: true, ngZoneRunCoalescing: true }); + `, + }, + { + name: absoluteFrom('/app/app.module.ts'), + contents: ` + import { NgModule } from '@angular/core'; + import { BrowserModule } from '@angular/platform-browser'; + import { AppComponent } from './app.component'; + + @NgModule({ + declarations: [AppComponent], + imports: [BrowserModule], + bootstrap: [AppComponent] + }) + export class AppModule {} + `, + }, + { + name: absoluteFrom('/app/app.component.ts'), + contents: ` + import { Component } from '@angular/core'; + + @Component({selector: 'app-root', template: ''}) + export class AppComponent {} + `, + }, + ]); + + const actual = fs.readFile(absoluteFrom('/app/app.module.ts')); + const expected = ` + import { NgModule } from '@angular/core'; + import { BrowserModule } from '@angular/platform-browser'; + import { AppComponent } from './app.component'; + + @NgModule({ + declarations: [AppComponent], + imports: [BrowserModule], + bootstrap: [AppComponent] + }) + export class AppModule {} + `; + expect(actual.replace(/\s+/g, '')) + .withContext(diffText(expected, actual)) + .toEqual(expected.replace(/\s+/g, '')); + + const mainActual = fs.readFile(absoluteFrom('/main.ts')); + const mainExpected = ` + import { provideZoneChangeDetection } from "@angular/core"; + import { ${platformBrowserFn} } from '@angular/${packageName}'; + import { AppModule } from './app/app.module'; + + ${platformBrowserFn}().bootstrapModule(AppModule, { applicationProviders: [provideZoneChangeDetection({ runCoalescing: true })], preserveWhitespaces: true, }); + `; + expect(mainActual.replace(/\s+/g, '')) + .withContext(diffText(mainExpected, mainActual)) + .toEqual(mainExpected.replace(/\s+/g, '')); + }); + it('should migrate imports when it is a variable identifier', async () => { const {fs} = await runTsurgeMigration(new BootstrapOptionsMigration(), [ ...typeFiles, @@ -1588,18 +1699,15 @@ describe('bootstrap options migration', () => { const actual = fs.readFile(absoluteFrom('/app/app.module.ts')); const expected = ` - import { NgModule, provideZoneChangeDetection } from '@angular/core'; + import { NgModule } from '@angular/core'; import { BrowserModule } from '@angular/platform-browser'; import { AppComponent } from './app.component'; const myImports = [BrowserModule]; - @NgModule({ providers: [ provideZoneChangeDetection({ runCoalescing: true }) ] }) - export class ZoneChangeDetectionModule {} - @NgModule({ declarations: [AppComponent], - imports: [ZoneChangeDetectionModule, ...myImports], + imports: myImports, bootstrap: [AppComponent] }) export class AppModule {} @@ -1607,6 +1715,18 @@ describe('bootstrap options migration', () => { expect(actual.replace(/\s+/g, '')) .withContext(diffText(expected, actual)) .toEqual(expected.replace(/\s+/g, '')); + + const mainActual = fs.readFile(absoluteFrom('/main.ts')); + const mainExpected = ` + import { provideZoneChangeDetection } from "@angular/core"; + import { ${platformBrowserFn} } from '@angular/${packageName}'; + import { AppModule } from './app/app.module'; + + ${platformBrowserFn}().bootstrapModule(AppModule, { applicationProviders: [provideZoneChangeDetection({ runCoalescing: true })], }); + `; + expect(mainActual.replace(/\s+/g, '')) + .withContext(diffText(mainExpected, mainActual)) + .toEqual(mainExpected.replace(/\s+/g, '')); }); it('should add provideZoneChangeDetection if no bootstrap options are present', async () => { @@ -1650,16 +1770,13 @@ describe('bootstrap options migration', () => { const actual = fs.readFile(absoluteFrom('/app/app.module.ts')); const expected = ` - import { NgModule, provideZoneChangeDetection } from '@angular/core'; + import { NgModule } from '@angular/core'; import { BrowserModule } from '@angular/platform-browser'; import { AppComponent } from './app.component'; - @NgModule({ providers: [provideZoneChangeDetection()] }) - export class ZoneChangeDetectionModule {} - @NgModule({ declarations: [AppComponent], - imports: [ZoneChangeDetectionModule, BrowserModule], + imports: [BrowserModule], bootstrap: [AppComponent] }) export class AppModule {} @@ -1670,10 +1787,11 @@ describe('bootstrap options migration', () => { const mainActual = fs.readFile(absoluteFrom('/main.ts')); const mainExpected = ` + import { provideZoneChangeDetection } from "@angular/core"; import { ${platformBrowserFn} } from '@angular/${packageName}'; import { AppModule } from './app/app.module'; - ${platformBrowserFn}().bootstrapModule(AppModule); + ${platformBrowserFn}().bootstrapModule(AppModule, { applicationProviders: [provideZoneChangeDetection()], }); `; expect(mainActual.replace(/\s+/g, '')) .withContext(diffText(mainExpected, mainActual)) @@ -1687,25 +1805,23 @@ describe('bootstrap options migration', () => { name: absoluteFrom('/main.ts'), isProgramRootFile: true, contents: ` + import { provideZoneChangeDetection } from "@angular/core"; import { ${platformBrowserFn} } from '@angular/${packageName}'; import { AppModule } from './app/app.module'; - ${platformBrowserFn}().bootstrapModule(AppModule); + ${platformBrowserFn}().bootstrapModule(AppModule, { applicationProviders: [ provideZoneChangeDetection() ] }); `, }, { name: absoluteFrom('/app/app.module.ts'), contents: ` - import { NgModule, provideZoneChangeDetection } from '@angular/core'; + import { NgModule } from '@angular/core'; import { BrowserModule } from '@angular/platform-browser'; import { AppComponent } from './app.component'; - @NgModule({ providers: [ provideZoneChangeDetection() ] }) - export class ZoneChangeDetectionModule {} - @NgModule({ declarations: [AppComponent], - imports: [ZoneChangeDetectionModule, BrowserModule], + imports: [BrowserModule], bootstrap: [AppComponent] }) export class AppModule {} @@ -1724,16 +1840,13 @@ describe('bootstrap options migration', () => { const actual = fs.readFile(absoluteFrom('/app/app.module.ts')); const expected = ` - import { NgModule, provideZoneChangeDetection } from '@angular/core'; + import { NgModule } from '@angular/core'; import { BrowserModule } from '@angular/platform-browser'; import { AppComponent } from './app.component'; - @NgModule({ providers: [ provideZoneChangeDetection() ] }) - export class ZoneChangeDetectionModule {} - @NgModule({ declarations: [AppComponent], - imports: [ZoneChangeDetectionModule, BrowserModule], + imports: [BrowserModule], bootstrap: [AppComponent] }) export class AppModule {} @@ -1744,10 +1857,11 @@ describe('bootstrap options migration', () => { const mainActual = fs.readFile(absoluteFrom('/main.ts')); const mainExpected = ` + import { provideZoneChangeDetection } from "@angular/core"; import { ${platformBrowserFn} } from '@angular/${packageName}'; import { AppModule } from './app/app.module'; - ${platformBrowserFn}().bootstrapModule(AppModule); + ${platformBrowserFn}().bootstrapModule(AppModule, { applicationProviders: [ provideZoneChangeDetection() ] }); `; expect(mainActual.replace(/\s+/g, '')).withContext(diffText(mainExpected, mainActual)); }); @@ -1797,19 +1911,16 @@ describe('bootstrap options migration', () => { const actual = fs.readFile(absoluteFrom('/app/app.module.ts')); const expected = ` - import { NgModule, provideZoneChangeDetection } from '@angular/core'; + import { NgModule } from '@angular/core'; import { BrowserModule } from '@angular/platform-browser'; import { AppComponent } from './app.component'; - @NgModule({ providers: [provideZoneChangeDetection()] }) - export class ZoneChangeDetectionModule {} - /** * This is a comment. */ @NgModule({ declarations: [AppComponent], - imports: [ZoneChangeDetectionModule, BrowserModule], + imports: [BrowserModule], bootstrap: [AppComponent] }) export class AppModule {} @@ -1817,6 +1928,18 @@ describe('bootstrap options migration', () => { expect(actual.replace(/\s+/g, '')) .withContext(diffText(expected, actual)) .toEqual(expected.replace(/\s+/g, '')); + + const mainActual = fs.readFile(absoluteFrom('/main.ts')); + const mainExpected = ` + import { provideZoneChangeDetection } from "@angular/core"; + import { ${platformBrowserFn} } from '@angular/${packageName}'; + import { AppModule } from './app/app.module'; + + ${platformBrowserFn}().bootstrapModule(AppModule, { applicationProviders: [provideZoneChangeDetection()], }); + `; + expect(mainActual.replace(/\s+/g, '')) + .withContext(diffText(mainExpected, mainActual)) + .toEqual(mainExpected.replace(/\s+/g, '')); }); it('should gracefuly fail with a TODO when having non-string ngZone', async () => { @@ -1860,16 +1983,13 @@ describe('bootstrap options migration', () => { const actual = fs.readFile(absoluteFrom('/app/app.module.ts')); const expected = ` - import { NgModule, provideZoneChangeDetection } from '@angular/core'; + import { NgModule } from '@angular/core'; import { BrowserModule } from '@angular/platform-browser'; import { AppComponent } from './app.component'; - @NgModule({ providers: [provideZoneChangeDetection()] }) - export class ZoneChangeDetectionModule {} - @NgModule({ declarations: [AppComponent], - imports: [ZoneChangeDetectionModule, BrowserModule], + imports: [BrowserModule], bootstrap: [AppComponent] }) export class AppModule {} @@ -1880,11 +2000,12 @@ describe('bootstrap options migration', () => { const mainActual = fs.readFile(absoluteFrom('/main.ts')); const mainExpected = ` + import { provideZoneChangeDetection } from "@angular/core"; import { ${platformBrowserFn} } from '@angular/${packageName}'; import { AppModule } from './app/app.module'; // TODO: BootstrapOptions are deprecated & ignored. Configure NgZone in the providers array of the application module instead. - ${platformBrowserFn}().bootstrapModule(AppModule, { ngZone: new NgZone({}) }); + ${platformBrowserFn}().bootstrapModule(AppModule, { applicationProviders: [provideZoneChangeDetection()], ngZone: new NgZone({}), }); `; expect(mainActual.replace(/\s+/g, '')) .withContext(diffText(mainExpected, mainActual)) @@ -1898,6 +2019,7 @@ describe('bootstrap options migration', () => { name: absoluteFrom('/main.ts'), isProgramRootFile: true, contents: ` + import { NgZone } from "@angular/core"; import { ${platformBrowserFn} } from '@angular/${packageName}'; import { AppModule } from './app/app.module'; @@ -1934,16 +2056,13 @@ describe('bootstrap options migration', () => { const actual = fs.readFile(absoluteFrom('/app/app.module.ts')); const expected = ` - import { NgModule, provideZoneChangeDetection } from '@angular/core'; + import { NgModule } from '@angular/core'; import { BrowserModule } from '@angular/platform-browser'; import { AppComponent } from './app.component'; - @NgModule({ providers: [provideZoneChangeDetection()] }) - export class ZoneChangeDetectionModule {} - @NgModule({ declarations: [AppComponent], - imports: [ZoneChangeDetectionModule, BrowserModule], + imports: [BrowserModule], bootstrap: [AppComponent] }) export class AppModule {} @@ -1954,18 +2073,157 @@ describe('bootstrap options migration', () => { const mainActual = fs.readFile(absoluteFrom('/main.ts')); const mainExpected = ` + import { NgZone, provideZoneChangeDetection } from "@angular/core"; import { ${platformBrowserFn} } from '@angular/${packageName}'; import { AppModule } from './app/app.module'; const ngZone = new NgZone({}); // TODO: BootstrapOptions are deprecated & ignored. Configure NgZone in the providers array of the application module instead. - ${platformBrowserFn}().bootstrapModule(AppModule, { ngZone: ngZone }); + ${platformBrowserFn}().bootstrapModule(AppModule, { applicationProviders: [provideZoneChangeDetection()], ngZone, }); `; expect(mainActual.replace(/\s+/g, '')) .withContext(diffText(mainExpected, mainActual)) .toEqual(mainExpected.replace(/\s+/g, '')); }); + + it('should migrate option array with multiple items', async () => { + const {fs} = await runTsurgeMigration(new BootstrapOptionsMigration(), [ + ...typeFiles, + { + name: absoluteFrom('/main.ts'), + isProgramRootFile: true, + contents: ` + import { ${platformBrowserFn} } from '@angular/${packageName}'; + import { AppModule } from './app/app.module'; + + ${platformBrowserFn}().bootstrapModule(AppModule, [{ preserveWhitespaces: true},{ngZoneRunCoalescing: true }]); + `, + }, + { + name: absoluteFrom('/app/app.module.ts'), + contents: ` + import { NgModule } from '@angular/core'; + import { BrowserModule } from '@angular/platform-browser'; + import { AppComponent } from './app.component'; + + @NgModule({ + declarations: [AppComponent], + imports: [BrowserModule], + bootstrap: [AppComponent] + }) + export class AppModule {} + `, + }, + { + name: absoluteFrom('/app/app.component.ts'), + contents: ` + import { Component } from '@angular/core'; + + @Component({selector: 'app-root', template: ''}) + export class AppComponent {} + `, + }, + ]); + + const actual = fs.readFile(absoluteFrom('/app/app.module.ts')); + const expected = ` + import { NgModule } from '@angular/core'; + import { BrowserModule } from '@angular/platform-browser'; + import { AppComponent } from './app.component'; + + @NgModule({ + declarations: [AppComponent], + imports: [BrowserModule], + bootstrap: [AppComponent] + }) + export class AppModule {} + `; + expect(actual.replace(/\s+/g, '')) + .withContext(diffText(expected, actual)) + .toEqual(expected.replace(/\s+/g, '')); + + const mainActual = fs.readFile(absoluteFrom('/main.ts')); + const mainExpected = ` + import { provideZoneChangeDetection } from "@angular/core"; + import { ${platformBrowserFn} } from '@angular/${packageName}'; + import { AppModule } from './app/app.module'; + + ${platformBrowserFn}().bootstrapModule(AppModule, { applicationProviders: [provideZoneChangeDetection({ runCoalescing: true })], preserveWhitespaces: true, }); + `; + expect(mainActual.replace(/\s+/g, '')) + .withContext(diffText(mainExpected, mainActual)) + .toEqual(mainExpected.replace(/\s+/g, '')); + }); + + it('should migrate option array with a single item', async () => { + const {fs} = await runTsurgeMigration(new BootstrapOptionsMigration(), [ + ...typeFiles, + { + name: absoluteFrom('/main.ts'), + isProgramRootFile: true, + contents: ` + import { ${platformBrowserFn} } from '@angular/${packageName}'; + import { AppModule } from './app/app.module'; + + ${platformBrowserFn}().bootstrapModule(AppModule, [{ngZoneRunCoalescing: true }]); + `, + }, + { + name: absoluteFrom('/app/app.module.ts'), + contents: ` + import { NgModule } from '@angular/core'; + import { BrowserModule } from '@angular/platform-browser'; + import { AppComponent } from './app.component'; + + @NgModule({ + declarations: [AppComponent], + imports: [BrowserModule], + bootstrap: [AppComponent] + }) + export class AppModule {} + `, + }, + { + name: absoluteFrom('/app/app.component.ts'), + contents: ` + import { Component } from '@angular/core'; + + @Component({selector: 'app-root', template: ''}) + export class AppComponent {} + `, + }, + ]); + + const actual = fs.readFile(absoluteFrom('/app/app.module.ts')); + const expected = ` + import { NgModule } from '@angular/core'; + import { BrowserModule } from '@angular/platform-browser'; + import { AppComponent } from './app.component'; + + @NgModule({ + declarations: [AppComponent], + imports: [BrowserModule], + bootstrap: [AppComponent] + }) + export class AppModule {} + `; + expect(actual.replace(/\s+/g, '')) + .withContext(diffText(expected, actual)) + .toEqual(expected.replace(/\s+/g, '')); + + const mainActual = fs.readFile(absoluteFrom('/main.ts')); + const mainExpected = ` + import { provideZoneChangeDetection } from "@angular/core"; + import { ${platformBrowserFn} } from '@angular/${packageName}'; + import { AppModule } from './app/app.module'; + + ${platformBrowserFn}().bootstrapModule(AppModule, { applicationProviders: [provideZoneChangeDetection({ runCoalescing: true })], }); + `; + expect(mainActual.replace(/\s+/g, '')) + .withContext(diffText(mainExpected, mainActual)) + .toEqual(mainExpected.replace(/\s+/g, '')); + }); }); }); }); diff --git a/packages/core/schematics/migrations/bootstrap-options-migration/migration.ts b/packages/core/schematics/migrations/bootstrap-options-migration/migration.ts index f5f70471c44..6abee5bbaef 100644 --- a/packages/core/schematics/migrations/bootstrap-options-migration/migration.ts +++ b/packages/core/schematics/migrations/bootstrap-options-migration/migration.ts @@ -21,7 +21,7 @@ import {TypeScriptReflectionHost} from '@angular/compiler-cli/src/ngtsc/reflecti import {applyImportManagerChanges} from '../../utils/tsurge/helpers/apply_import_manager'; import {getAngularDecorators} from '@angular/compiler-cli/src/ngtsc/annotations'; import {findLiteralProperty} from '../../utils/typescript/property_name'; -import {getImportSpecifier, getRelativePath} from '../../utils/typescript/imports'; +import {getImportSpecifier} from '../../utils/typescript/imports'; import {isReferenceToImport} from '../../utils/typescript/symbol'; interface CompilationUnitData { @@ -31,6 +31,12 @@ interface CompilationUnitData { const CORE_PACKAGE = '@angular/core'; const PROVIDE_ZONE_CHANGE_DETECTION = 'provideZoneChangeDetection'; const ZONE_CD_PROVIDER = `${PROVIDE_ZONE_CHANGE_DETECTION}()`; +const SAFE_TO_REMOVE_OPTIONS = [ + 'ignoreChangesOutsideZone', + 'ngZoneRunCoalescing', + 'ngZoneEventCoalescing', +]; +const BOOTSTRAP_OPTIONS = ['ngZone', ...SAFE_TO_REMOVE_OPTIONS]; export class BootstrapOptionsMigration extends TsurgeFunnelMigration< CompilationUnitData, @@ -54,8 +60,6 @@ export class BootstrapOptionsMigration extends TsurgeFunnelMigration< const { bootstrapAppSpecifier, - platformBrowserDynamicSpecifier, - platformBrowserSpecifier, testBedSpecifier, createApplicationSpecifier, getTestBedSpecifier, @@ -83,17 +87,7 @@ export class BootstrapOptionsMigration extends TsurgeFunnelMigration< ts.isCallExpression(node) && ts.isPropertyAccessExpression(node.expression) && node.expression.name.text === 'bootstrapModule' && - ts.isCallExpression(node.expression.expression) && - (isReferenceToImport( - typeChecker, - node.expression.expression.expression, - platformBrowserSpecifier, - ) || - isReferenceToImport( - typeChecker, - node.expression.expression.expression, - platformBrowserDynamicSpecifier, - )) + node.arguments.length > 0 ); }; const isTestBedInitEnvironmentNode = (node: ts.Node): node is ts.CallExpression => { @@ -154,10 +148,6 @@ export class BootstrapOptionsMigration extends TsurgeFunnelMigration< sourceFile.forEachChild(walk); } - // The combine method might not run when there is a single target. - // So we deduplicate here - replacements = deduplicateReplacements(replacements); - applyImportManagerChanges(importManager, replacements, info.sourceFiles, info); return confirmAsSerializable({replacements}); } @@ -167,7 +157,7 @@ export class BootstrapOptionsMigration extends TsurgeFunnelMigration< unitB: CompilationUnitData, ): Promise> { const combined = [...unitA.replacements, ...unitB.replacements]; - return confirmAsSerializable({replacements: deduplicateReplacements(combined)}); + return confirmAsSerializable({replacements: combined}); } override async globalMeta(data: CompilationUnitData): Promise> { @@ -335,19 +325,16 @@ export class BootstrapOptionsMigration extends TsurgeFunnelMigration< return; } - const moduleSourceFile = moduleClass.getSourceFile(); - const moduleProjectFile = projectFile(moduleSourceFile, info); - - if (moduleSourceFile.getText().includes('ZoneChangeDetectionModule')) { - // If the file already contains the ZoneChangeDetectionModule, we can skip it. - return; - } - - // Always remove the options argument + const optionsNode = node.arguments[1]; + const file = projectFile(sourceFile, info); replacements.push( new Replacement( - projectFile(sourceFile, info), - new TextUpdate({position: moduleIdentifier.getEnd(), end: node.getEnd() - 1, toInsert: ''}), + file, + new TextUpdate({ + position: moduleIdentifier.getEnd(), + end: node.getEnd() - 1, + toInsert: '', + }), ), ); @@ -357,17 +344,35 @@ export class BootstrapOptionsMigration extends TsurgeFunnelMigration< } // Let's try to understand the bootstrap options. - const optionsNode = node.arguments[1]; - const options = - optionsNode && ts.isObjectLiteralExpression(optionsNode) - ? evaluator.evaluate(optionsNode) - : null; + let options = optionsNode ? evaluator.evaluate(optionsNode) : null; + let extraOptions = new Map(); let zoneCdProvider = ZONE_CD_PROVIDER; let zoneInstanceProvider: string | null = null; + if (Array.isArray(options)) { + const mergedOptions = options.reduce((acc, item) => { + if (item instanceof Map) { + for (const [k, v] of item) { + acc.set(k, v); + if (!SAFE_TO_REMOVE_OPTIONS.includes(k)) { + extraOptions.set(k, v); + } + } + } + return acc; + }, new Map()); + + options = mergedOptions; + } + if (options instanceof Map) { - const ngZoneOption = options.get('ngZone'); + [...options.entries()].forEach(([k, v]) => { + if (!BOOTSTRAP_OPTIONS.includes(k) && typeof v !== 'string') { + extraOptions.set(k, v); + } + }); + if (options.has('ngZoneRunCoalescing') || options.has('ngZoneEventCoalescing')) { const config: string[] = []; if (options.get('ngZoneRunCoalescing')) { @@ -379,28 +384,18 @@ export class BootstrapOptionsMigration extends TsurgeFunnelMigration< zoneCdProvider = `${PROVIDE_ZONE_CHANGE_DETECTION}(${config.length > 0 ? `{ ${config.join(', ')} }` : ''})`; } + const ngZoneOption = options.get('ngZone'); + if (ngZoneOption instanceof Reference) { - importManager.addImport({ - exportModuleSpecifier: CORE_PACKAGE, - exportSymbolName: 'NgZone', - requestedFile: moduleSourceFile, - }); const clazz = ngZoneOption.node; if (ts.isClassDeclaration(clazz) && clazz.name) { - const customZoneSourceFile = clazz.getSourceFile(); - const exportModuleSpecifier = - ngZoneOption.bestGuessOwningModule?.specifier ?? - getRelativePath(moduleSourceFile.fileName, customZoneSourceFile.fileName); - importManager.addImport({ - exportModuleSpecifier, - exportSymbolName: clazz.name.text, - requestedFile: moduleSourceFile, - }); - zoneInstanceProvider = `{provide: NgZone, useClass: ${clazz.name.text}}`; + removePropertiesFromLiteral(file, optionsNode, ['ngZone'], replacements); + } + } else if (typeof ngZoneOption === 'string') { + if (ngZoneOption === 'noop') { + return; } - } else if (typeof ngZoneOption === 'string' && ngZoneOption === 'noop') { - return; } else if (ngZoneOption && typeof ngZoneOption !== 'string') { // This is a case where we're not able to migrate automatically // The migration fails gracefully, keeps the ngZone option and adds a TODO. @@ -414,19 +409,9 @@ export class BootstrapOptionsMigration extends TsurgeFunnelMigration< }); if (ngZoneValue) { // We re-add the ngZone option - replacements.push( - new Replacement( - projectFile(sourceFile, info), - new TextUpdate({ - position: moduleIdentifier.getEnd(), - end: node.getEnd() - 1, - toInsert: `, {ngZone: ${ngZoneValue}}`, - }), - ), - ); + extraOptions.set('ngZone', ngZoneValue); } - // And add the TODO replacements.push( new Replacement( projectFile(sourceFile, info), @@ -446,21 +431,27 @@ export class BootstrapOptionsMigration extends TsurgeFunnelMigration< providers.push(zoneInstanceProvider); } - if (providers.length > 0) { - importManager.addImport({ - exportModuleSpecifier: CORE_PACKAGE, - exportSymbolName: PROVIDE_ZONE_CHANGE_DETECTION, - requestedFile: moduleSourceFile, - }); + importManager.addImport({ + exportModuleSpecifier: CORE_PACKAGE, + exportSymbolName: PROVIDE_ZONE_CHANGE_DETECTION, + requestedFile: sourceFile, + }); - addProvidersToNgModule( - moduleProjectFile, - moduleSourceFile, - ngModule, - providers.join(',\n'), - replacements, - ); - } + // if we only use the key, we use the a shorthand asignment + const extraOptionsStr = [...extraOptions.entries()] + .map(([k, v]) => (k != v ? `${k}: ${v},` : `${k},`)) + .join(', '); + + replacements.push( + new Replacement( + file, + new TextUpdate({ + position: moduleIdentifier.end, + end: moduleIdentifier.end, + toInsert: `, { applicationProviders: [${providers.join(', ')}], ${extraOptionsStr}}`, + }), + ), + ); } private analyzeTestBedInitEnvironment( @@ -501,48 +492,6 @@ export class BootstrapOptionsMigration extends TsurgeFunnelMigration< } } -function addProvidersToNgModule( - projectFile: ProjectFile, - moduleSourceFile: ts.SourceFile, - ngModule: ts.ObjectLiteralExpression, - providersText: string, - replacements: Replacement[], -) { - // ObjLiteral => callExp => Decorator => ClassExpression - const moduleClassDeclaration = ngModule.parent.parent.parent; - const insertPosition = moduleClassDeclaration.getStart(moduleSourceFile, true) - 1; - addZoneCDModule(providersText, projectFile, insertPosition, replacements); - - const importsNode = findLiteralProperty(ngModule, 'imports'); - if (importsNode && ts.isPropertyAssignment(importsNode)) { - insertZoneCDModule( - importsNode.initializer, - projectFile, - replacements, - 'ZoneChangeDetectionModule', - ); - } else { - const text = `imports: [ZoneChangeDetectionModule]`; - const toInsert = `${text},\n`; - let position = ngModule.getStart() + 1; - - if (ngModule.properties.length > 0) { - const firstProperty = ngModule.properties[0]; - position = firstProperty.getStart(); - } - replacements.push( - new Replacement( - projectFile, - new TextUpdate({ - position, - end: position, - toInsert, - }), - ), - ); - } -} - function addZoneCDModule( providersText: string, projectFile: ProjectFile, @@ -862,7 +811,6 @@ function getSpecifiers(sourceFile: ts.SourceFile) { if ( !createApplicationSpecifier && !bootstrapAppSpecifier && - !platformBrowserDynamicSpecifier && !platformBrowserSpecifier && !testBedSpecifier && !ngModuleSpecifier && @@ -882,54 +830,6 @@ function getSpecifiers(sourceFile: ts.SourceFile) { }; } -/** - * Removes duplicate replacements and for replacements at the same position, takes the longest one. - */ -function deduplicateReplacements(replacements: Replacement[]): Replacement[] { - if (replacements.length <= 1) { - return replacements; - } - - // Group replacements by file and position - const groupedByFileAndPosition = new Map>(); - - for (const replacement of replacements) { - const fileKey = replacement.projectFile.id; - const position = replacement.update.data.position; - - if (!groupedByFileAndPosition.has(fileKey)) { - groupedByFileAndPosition.set(fileKey, new Map()); - } - - const fileReplacements = groupedByFileAndPosition.get(fileKey)!; - if (!fileReplacements.has(position)) { - fileReplacements.set(position, []); - } - - fileReplacements.get(position)!.push(replacement); - } - - const result: Replacement[] = []; - - for (const fileReplacements of groupedByFileAndPosition.values()) { - for (const positionReplacements of fileReplacements.values()) { - if (positionReplacements.length === 1) { - result.push(positionReplacements[0]); - } else { - // For multiple replacements at the same position, take the one with the longest content - const longestReplacement = positionReplacements.reduce((longest, current) => { - const longestLength = longest.update.data.toInsert.length; - const currentLength = current.update.data.toInsert.length; - return currentLength > longestLength ? current : longest; - }); - result.push(longestReplacement); - } - } - } - - return result; -} - /** * In the case we're looking to insert a new ZoneChangeDetectionModule, we need to check if we already inserted one. * @@ -951,3 +851,55 @@ function replacementsHaveZoneCdModule( ); }); } + +function removePropertiesFromLiteral( + projectFile: ProjectFile, + literal: ts.Node, + propertyNames: string[], + replacements: Replacement[], +) { + const syntaxList = literal.getChildren().find((ch) => ch.kind === ts.SyntaxKind.SyntaxList)!; + const optionsElements = syntaxList.getChildren(); + + const optionsToRemove: {start: number; end: number}[] = []; + optionsElements.forEach((node, i, children) => { + if ( + ts.isPropertyAssignment(node) && + ts.isIdentifier(node.name) && + propertyNames.includes(node.name.text) + ) { + // Look ahead for comma + const next = children[i + 1]; + if (next && next.kind === ts.SyntaxKind.CommaToken) { + optionsToRemove.push({start: node.getStart(), end: next.getEnd()}); + } else { + optionsToRemove.push({start: node.getStart(), end: node.getEnd()}); + } + } + }); + optionsToRemove.forEach((toRemove) => { + replacements.push( + new Replacement( + projectFile, + new TextUpdate({position: toRemove.start, end: toRemove.end, toInsert: ''}), + ), + ); + }); +} + +function onlyBootstrapOptions(literal: ts.Node | undefined): boolean { + if (!literal || !ts.isObjectLiteralExpression(literal)) { + return false; + } + + for (const element of literal.properties) { + if ( + ts.isPropertyAssignment(element) && + ts.isIdentifier(element.name) && + !BOOTSTRAP_OPTIONS.includes(element.name.text) + ) { + return false; + } + } + return true; +}