Skip to content

Commit

Permalink
refactor(migrations): use common import manager for schematics (angul…
Browse files Browse the repository at this point in the history 
…ar#57096)

Updates the schematics to reuse the common `ImportManager`, instead of having to maintain a separate one.

PR Close angular#57096
  • Loading branch information
@crisbeto @atscott
crisbeto authored and atscott committed Jul 23, 2024
1 parent b464c3d commit bb977e0
Show file tree
Hide file tree
Showing 6 changed files with 126 additions and 497 deletions.
1 change: 1 addition & 0 deletions packages/compiler-cli/private/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ ts_library(
"//packages/compiler-cli/src/ngtsc/reflection",
"//packages/compiler-cli/src/ngtsc/sourcemaps",
"//packages/compiler-cli/src/ngtsc/transform/jit",
"//packages/compiler-cli/src/ngtsc/translator",
"//packages/compiler-cli/src/ngtsc/typecheck/api",
"@npm//typescript",
],
Expand Down
1 change: 1 addition & 0 deletions packages/compiler-cli/private/migrations.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,4 @@ export {
PotentialImportMode,
TemplateTypeChecker,
} from '../src/ngtsc/typecheck/api';
export {ImportManager} from '../src/ngtsc/translator';
2 changes: 1 addition & 1 deletion packages/core/schematics/ng-generate/standalone-migration/standalone-bootstrap.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -670,7 +670,7 @@ function addNodesToCopy(
const symbolName = importSpecifier.propertyName
? importSpecifier.propertyName.text
: importSpecifier.name.text;
const alias = importSpecifier.propertyName ? importSpecifier.name.text : null;
const alias = importSpecifier.propertyName ? importSpecifier.name.text : undefined;
tracker.addImport(targetFile, symbolName, moduleName, alias);
continue;
}
Expand Down
90 changes: 45 additions & 45 deletions packages/core/schematics/test/standalone_migration_spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3370,9 +3370,9 @@ describe('standalone migration', () => {
expect(stripWhitespace(tree.readContent('main.ts'))).toBe(
stripWhitespace(`
import {platformBrowser, bootstrapApplication} from '@angular/platform-browser';
import {importProvidersFrom} from '@angular/core';
import {AppComponent} from './app/app.component';
import {CommonModule} from '@angular/common';
import {AppComponent} from './app/app.component';
import {importProvidersFrom} from '@angular/core';
bootstrapApplication(AppComponent, {
providers: [importProvidersFrom(CommonModule)]
Expand All @@ -3383,8 +3383,8 @@ describe('standalone migration', () => {
expect(stripWhitespace(tree.readContent('./app/app.component.ts'))).toBe(
stripWhitespace(`
import {Component} from '@angular/core';
import {Dir} from './dir';
import {NgIf} from '@angular/common';
import {Dir} from './dir';
@Component({
template: '<div *ngIf="show" dir>hello</div>',
Expand Down Expand Up @@ -3545,11 +3545,11 @@ describe('standalone migration', () => {
stripWhitespace(`
import {exportedToken, exportedExtraProviders, ExportedClass, exportedFactory, AppComponent} from './app/app.module';
import {platformBrowser, bootstrapApplication} from '@angular/platform-browser';
import {ExternalInterface} from '@external/interfaces';
import {externalToken as aliasedExternalToken} from './app/externals/other-token';
import {externalToken} from './app/externals/token';
import {InternalInterface} from './app/interfaces/internal-interface';
import {InjectionToken} from '@angular/core';
import {InternalInterface} from './app/interfaces/internal-interface';
import {externalToken} from './app/externals/token';
import {externalToken as aliasedExternalToken} from './app/externals/other-token';
import {ExternalInterface} from '@external/interfaces';
const internalToken = new InjectionToken<string>('internalToken');
const unexportedExtraProviders = [
Expand Down Expand Up @@ -3774,10 +3774,10 @@ describe('standalone migration', () => {
stripWhitespace(`
import {SameFileModule, AppComponent} from './app/app.module';
import {platformBrowser, bootstrapApplication} from '@angular/platform-browser';
import {token} from './token';
import {NgModule, importProvidersFrom} from '@angular/core';
import {InternalModule} from './modules/internal.module';
import {CommonModule} from '@angular/common';
import {InternalModule} from './modules/internal.module';
import {NgModule, importProvidersFrom} from '@angular/core';
import {token} from './token';
bootstrapApplication(AppComponent, {
providers: [importProvidersFrom(CommonModule, InternalModule, SameFileModule)]
Expand Down Expand Up @@ -3874,8 +3874,8 @@ describe('standalone migration', () => {
stripWhitespace(`
import {AppComponent} from './app/app.module';
import {platformBrowser, bootstrapApplication} from '@angular/platform-browser';
import {APP_ROUTES} from './app/routes';
import {provideRouter} from '@angular/router';
import {APP_ROUTES} from './app/routes';
bootstrapApplication(AppComponent, {
providers: [provideRouter(APP_ROUTES)]
Expand Down Expand Up @@ -3922,8 +3922,8 @@ describe('standalone migration', () => {
stripWhitespace(`
import {AppComponent} from './app/app.module';
import {platformBrowser, bootstrapApplication} from '@angular/platform-browser';
import {of} from 'rxjs';
import {withPreloading, provideRouter} from '@angular/router';
import {of} from 'rxjs';
bootstrapApplication(AppComponent, {
providers: [provideRouter([], withPreloading(() => of(true)))]
Expand Down Expand Up @@ -4349,8 +4349,8 @@ describe('standalone migration', () => {
stripWhitespace(`
import {AppComponent} from './app/app.module';
import {platformBrowser, bootstrapApplication} from '@angular/platform-browser';
import {importProvidersFrom} from '@angular/core';
import {RouterModule} from '@angular/router';
import {importProvidersFrom} from '@angular/core';
const extraOptions = {useHash: true};
Expand Down Expand Up @@ -4443,8 +4443,8 @@ describe('standalone migration', () => {
stripWhitespace(`
import {AppComponent} from './app/app.module';
import {platformBrowser, bootstrapApplication} from '@angular/platform-browser';
import {importProvidersFrom} from '@angular/core';
import {BrowserAnimationsModule} from '@angular/platform-browser/animations';
import {importProvidersFrom} from '@angular/core';
bootstrapApplication(AppComponent, {
providers: [importProvidersFrom(BrowserAnimationsModule.withConfig({disableAnimations: true}))]
Expand Down Expand Up @@ -4575,8 +4575,8 @@ describe('standalone migration', () => {
stripWhitespace(`
import {AppComponent} from './app/app.module';
import {platformBrowser, bootstrapApplication} from '@angular/platform-browser';
import {importProvidersFrom} from '@angular/core';
import {CommonModule} from '@angular/common';
import {importProvidersFrom} from '@angular/core';
bootstrapApplication(AppComponent, {
providers: [importProvidersFrom(CommonModule)]
Expand All @@ -4596,59 +4596,59 @@ describe('standalone migration', () => {
writeFile(
'main.ts',
`
import {AppModule} from './app/app.module';
import {platformBrowser} from '@angular/platform-browser';
import {AppModule} from './app/app.module';
import {platformBrowser} from '@angular/platform-browser';
platformBrowser().bootstrapModule(AppModule).catch(e => console.error(e));
`,
platformBrowser().bootstrapModule(AppModule).catch(e => console.error(e));
`,
);

writeFile(
'./app/root.module.ts',
`
import {NgModule, Component, InjectionToken} from '@angular/core';
import {NgModule, Component, InjectionToken} from '@angular/core';
const token = new InjectionToken<string>('token');
const token = new InjectionToken<string>('token');
@Component({selector: 'root-comp', template: '', standalone: true})
export class Root {}
@Component({selector: 'root-comp', template: '', standalone: true})
export class Root {}
@NgModule({
imports: [Root],
exports: [Root],
providers: [{provide: token, useValue: 'hello'}]
})
export class RootModule {}
`,
@NgModule({
imports: [Root],
exports: [Root],
providers: [{provide: token, useValue: 'hello'}]
})
export class RootModule {}
`,
);

writeFile(
'./app/app.module.ts',
`
import {NgModule, Component} from '@angular/core';
import {RootModule, Root} from './root.module';
import {NgModule, Component} from '@angular/core';
import {RootModule, Root} from './root.module';
@NgModule({
imports: [RootModule],
bootstrap: [Root]
})
export class AppModule {}
`,
@NgModule({
imports: [RootModule],
bootstrap: [Root]
})
export class AppModule {}
`,
);

await runMigration('standalone-bootstrap');

expect(tree.exists('./app/app.module.ts')).toBe(false);
expect(stripWhitespace(tree.readContent('main.ts'))).toBe(
stripWhitespace(`
import {platformBrowser, bootstrapApplication} from '@angular/platform-browser';
import {importProvidersFrom} from '@angular/core';
import {RootModule, Root} from './app/root.module';
import {platformBrowser, bootstrapApplication} from '@angular/platform-browser';
import {RootModule, Root} from './app/root.module';
import {importProvidersFrom} from '@angular/core';
bootstrapApplication(Root, {
providers: [importProvidersFrom(RootModule)]
}).catch(e => console.error(e));
`),
bootstrapApplication(Root, {
providers: [importProvidersFrom(RootModule)]
}).catch(e => console.error(e));
`),
);
});

Expand Down
100 changes: 78 additions & 22 deletions packages/core/schematics/utils/change_tracker.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@
*/

import ts from 'typescript';

import {ImportManager} from './import_manager';
import {ImportManager} from '@angular/compiler-cli/private/migrations';

/** Function that can be used to remap a generated import. */
export type ImportRemapper = (moduleName: string, inFile: string) => string;
Expand All @@ -29,23 +28,25 @@ export interface PendingChange {
text: string;
}

/** Supported quotes for generated imports. */
const enum QuoteKind {
SINGLE,
DOUBLE,
}

/** Tracks changes that have to be made for specific files. */
export class ChangeTracker {
private readonly _changes = new Map<ts.SourceFile, PendingChange[]>();
private readonly _importManager: ImportManager;
private readonly _quotesCache = new WeakMap<ts.SourceFile, QuoteKind>();

constructor(
private _printer: ts.Printer,
private _importRemapper?: ImportRemapper,
) {
this._importManager = new ImportManager(
(currentFile) => ({
addNewImport: (start, text) => this.insertText(currentFile, start, text),
updateExistingImport: (namedBindings, text) =>
this.replaceText(currentFile, namedBindings.getStart(), namedBindings.getWidth(), text),
}),
this._printer,
);
this._importManager = new ImportManager({
shouldUseSingleQuotes: (file) => this._getQuoteKind(file) === QuoteKind.SINGLE,
});
}

/**
Expand Down Expand Up @@ -111,14 +112,12 @@ export class ChangeTracker {
* @param symbolName Symbol being imported.
* @param moduleName Module from which the symbol is imported.
* @param alias Alias to use for the import.
* @param keepSymbolName Whether to keep the symbol name in the import.
*/
addImport(
sourceFile: ts.SourceFile,
symbolName: string,
moduleName: string,
alias: string | null = null,
keepSymbolName = false,
alias?: string,
): ts.Expression {
if (this._importRemapper) {
moduleName = this._importRemapper(moduleName, sourceFile.fileName);
Expand All @@ -129,22 +128,24 @@ export class ChangeTracker {
// paths will also cause TS to escape the forward slashes.
moduleName = normalizePath(moduleName);

return this._importManager.addImportToSourceFile(
sourceFile,
symbolName,
moduleName,
alias,
false,
keepSymbolName,
);
if (!this._changes.has(sourceFile)) {
this._changes.set(sourceFile, []);
}

return this._importManager.addImport({
requestedFile: sourceFile,
exportSymbolName: symbolName,
exportModuleSpecifier: moduleName,
unsafeAliasOverride: alias,
});
}

/**
* Gets the changes that should be applied to all the files in the migration.
* The changes are sorted in the order in which they should be applied.
*/
recordChanges(): ChangesByFile {
this._importManager.recordChanges();
this._recordImports();
return this._changes;
}

Expand Down Expand Up @@ -178,6 +179,61 @@ export class ChangeTracker {
this._changes.set(file, [change]);
}
}

/** Determines what kind of quotes to use for a specific file. */
private _getQuoteKind(sourceFile: ts.SourceFile): QuoteKind {
if (this._quotesCache.has(sourceFile)) {
return this._quotesCache.get(sourceFile)!;
}

let kind = QuoteKind.SINGLE;

for (const statement of sourceFile.statements) {
if (ts.isImportDeclaration(statement) && ts.isStringLiteral(statement.moduleSpecifier)) {
kind = statement.moduleSpecifier.getText()[0] === '"' ? QuoteKind.DOUBLE : QuoteKind.SINGLE;
this._quotesCache.set(sourceFile, kind);
break;
}
}

return kind;
}

/** Records the pending import changes from the import manager. */
private _recordImports(): void {
const {newImports, updatedImports} = this._importManager.finalize();

for (const [original, replacement] of updatedImports) {
this.replaceNode(original, replacement);
}

for (const [sourceFile] of this._changes) {
const importsToAdd = newImports.get(sourceFile.fileName);

if (!importsToAdd) {
continue;
}

const importLines: string[] = [];
let lastImport: ts.ImportDeclaration | null = null;

for (const statement of sourceFile.statements) {
if (ts.isImportDeclaration(statement)) {
lastImport = statement;
}
}

for (const decl of importsToAdd) {
importLines.push(this._printer.printNode(ts.EmitHint.Unspecified, decl, sourceFile));
}

this.insertText(
sourceFile,
lastImport ? lastImport.getEnd() : 0,
(lastImport ? '\n' : '') + importLines.join('\n'),
);
}
}
}

/** Normalizes a path to use posix separators. */
Expand Down
Loading

0 comments on commit bb977e0

Please sign in to comment.