fix(migrations): migrate tests when switching to standalone bootstrap API (#48987)

The first step of the migration updates the tests of the component being migrated, however it also skips any bootstraped declarations. We get back to the these declarations when converting to the standalone bootstrapping APIs, but we weren't updating their tests.

These changes rework some of the logic so that we migrate any remaining tests as a part of the final step.

Relates to #48944.

PR Close #48987
This commit is contained in:
Kristiyan Kostadinov
2023-02-07 11:14:55 +01:00
committed by Pawel Kozlowski
parent 6377487b1a
commit 770191cf1f
3 changed files with 195 additions and 111 deletions
@@ -15,7 +15,7 @@ import ts from 'typescript';
import {getAngularDecorators} from '../../utils/ng_decorators';
import {closestNode} from '../../utils/typescript/nodes';
import {convertNgModuleDeclarationToStandalone} from './to-standalone';
import {convertNgModuleDeclarationToStandalone, extractDeclarationsFromModule, findTestObjectsToMigrate, migrateTestDeclarations} from './to-standalone';
import {ChangeTracker, createLanguageService, findClassDeclaration, findLiteralProperty, getNodeLookup, getRelativeImportPath, ImportRemapper, NamedClassDeclaration, NodeLookup, offsetsToNodes, UniqueItemTracker} from './util';
/** Information extracted from a `bootstrapModule` call necessary to migrate it. */
@@ -28,6 +28,8 @@ interface BootstrapCallAnalysis {
metadata: ts.ObjectLiteralExpression;
/** Component that the module is bootstrapping. */
component: NamedClassDeclaration;
/** Classes declared by the bootstrapped module. */
declarations: Reference<ts.ClassDeclaration>[];
}
export function toStandaloneBootstrap(
@@ -38,22 +40,41 @@ export function toStandaloneBootstrap(
const templateTypeChecker = program.compiler.getTemplateTypeChecker();
const languageService = createLanguageService(program, host, rootFileNames, basePath);
const bootstrapCalls: BootstrapCallAnalysis[] = [];
const testObjects: ts.ObjectLiteralExpression[] = [];
sourceFiles.forEach(function walk(node: ts.Node) {
if (ts.isCallExpression(node) && ts.isPropertyAccessExpression(node.expression) &&
node.expression.name.text === 'bootstrapModule' &&
isClassReferenceInAngularModule(node.expression, 'PlatformRef', 'core', typeChecker)) {
const call = analyzeBootstrapCall(node, typeChecker);
for (const sourceFile of sourceFiles) {
sourceFile.forEachChild(function walk(node) {
if (ts.isCallExpression(node) && ts.isPropertyAccessExpression(node.expression) &&
node.expression.name.text === 'bootstrapModule' &&
isClassReferenceInAngularModule(node.expression, 'PlatformRef', 'core', typeChecker)) {
const call = analyzeBootstrapCall(node, typeChecker, templateTypeChecker);
if (call) {
bootstrapCalls.push(call);
if (call) {
bootstrapCalls.push(call);
}
}
}
node.forEachChild(walk);
});
node.forEachChild(walk);
});
for (const call of bootstrapCalls) {
migrateBootstrapCall(call, tracker, languageService, typeChecker, templateTypeChecker, printer);
testObjects.push(...findTestObjectsToMigrate(sourceFile, typeChecker));
}
if (bootstrapCalls.length > 0) {
const allDeclarations: Reference<ts.ClassDeclaration>[] = [];
for (const call of bootstrapCalls) {
allDeclarations.push(...call.declarations);
migrateBootstrapCall(call, tracker, languageService, typeChecker, printer);
}
if (allDeclarations.length > 0) {
// The previous migrations explicitly skip over bootstrapped
// declarations so we have to migrate them now.
allDeclarations.forEach(
decl => convertNgModuleDeclarationToStandalone(
decl, allDeclarations, tracker, templateTypeChecker));
migrateTestDeclarations(testObjects, allDeclarations, tracker, typeChecker);
}
}
return tracker.recordChanges();
@@ -64,9 +85,11 @@ export function toStandaloneBootstrap(
* necessary to convert it to `bootstrapApplication`.
* @param call Call to be analyzed.
* @param typeChecker
* @param templateTypeChecker
*/
function analyzeBootstrapCall(
call: ts.CallExpression, typeChecker: ts.TypeChecker): BootstrapCallAnalysis|null {
call: ts.CallExpression, typeChecker: ts.TypeChecker,
templateTypeChecker: TemplateTypeChecker): BootstrapCallAnalysis|null {
if (call.arguments.length === 0 || !ts.isIdentifier(call.arguments[0])) {
return null;
}
@@ -98,7 +121,13 @@ function analyzeBootstrapCall(
const component = findClassDeclaration(bootstrapProp.initializer.elements[0], typeChecker);
if (component && component.name && ts.isIdentifier(component.name)) {
return {module: declaration, metadata, component: component as NamedClassDeclaration, call};
return {
module: declaration,
metadata,
component: component as NamedClassDeclaration,
call,
declarations: extractDeclarationsFromModule(declaration, templateTypeChecker)
};
}
return null;
@@ -110,12 +139,11 @@ function analyzeBootstrapCall(
* @param tracker Tracker in which to register the changes.
* @param languageService
* @param typeChecker
* @param templateTypeChecker
* @param printer
*/
function migrateBootstrapCall(
analysis: BootstrapCallAnalysis, tracker: ChangeTracker, languageService: ts.LanguageService,
typeChecker: ts.TypeChecker, templateTypeChecker: TemplateTypeChecker, printer: ts.Printer) {
typeChecker: ts.TypeChecker, printer: ts.Printer) {
const sourceFile = analysis.call.getSourceFile();
const moduleSourceFile = analysis.metadata.getSourceFile();
const providers = findLiteralProperty(analysis.metadata, 'providers');
@@ -125,10 +153,6 @@ function migrateBootstrapCall(
const moduleImportsInNewCall: ts.Expression[] = [];
let nodeLookup: NodeLookup|null = null;
// The previous migrations explicitly skip over modules that bootstrap a
// component so we have to convert it as a part of this migration instead.
convertBootstrappedModuleToStandalone(analysis, tracker, templateTypeChecker);
// We can't reuse the module pruning logic, because we would have to recreate the entire program.
// Instead we comment out the module's metadata so that the user doesn't get compilation errors
// for the classes that are left in the `declarations` array. This should allow the app to
@@ -223,40 +247,6 @@ function replaceBootstrapCallExpression(
undefined, analysis.metadata.getSourceFile());
}
/**
* Converts the declarations of a bootstrapped module to standalone. These declarations are
* skipped in the `convert-to-standalone` phase so they need to be migrated when converting
* to `bootstrapApplication`.
* @param analysis Result of the analysis of the NgModule.
* @param tracker
* @param templateTypeChecker
*/
function convertBootstrappedModuleToStandalone(
analysis: BootstrapCallAnalysis, tracker: ChangeTracker,
templateTypeChecker: TemplateTypeChecker) {
const metadata = templateTypeChecker.getNgModuleMetadata(analysis.module);
if (!metadata) {
throw new Error(`Cannot resolve NgModule metadata for class ${
analysis.module.name?.getText()}. Cannot switch to standalone bootstrap API.`);
}
const classDeclarations =
metadata.declarations.filter(decl => ts.isClassDeclaration(decl.node)) as
Reference<ts.ClassDeclaration>[];
if (!classDeclarations.some(decl => decl.node === analysis.component)) {
throw new Error(`Bootstrapped component is not in the declarations array of NgModule ${
analysis.module.name?.getText()}. Cannot switch to standalone bootstrap API.`);
}
for (const decl of classDeclarations) {
if (ts.isClassDeclaration(decl.node)) {
convertNgModuleDeclarationToStandalone(decl, classDeclarations, tracker, templateTypeChecker);
}
}
}
/**
* Processes the `imports` of an NgModule so that they can be used in the `bootstrapApplication`
* call inside of a different file.
@@ -34,15 +34,17 @@ export function toStandalone(
const tracker = new ChangeTracker(printer, importRemapper);
for (const sourceFile of sourceFiles) {
const {modules, testObjects} = findModulesToMigrate(sourceFile, typeChecker);
const modules = findNgModuleClassesToMigrate(sourceFile, typeChecker);
const testObjects = findTestObjectsToMigrate(sourceFile, typeChecker);
for (const module of modules) {
const moduleDeclarations =
extractDeclarationsFromModule(module, typeChecker, templateTypeChecker);
const allModuleDeclarations = extractDeclarationsFromModule(module, templateTypeChecker);
const unbootstrappedDeclarations = filterNonBootstrappedDeclarations(
allModuleDeclarations, module, templateTypeChecker, typeChecker);
if (moduleDeclarations.length > 0) {
if (unbootstrappedDeclarations.length > 0) {
modulesToMigrate.push(module);
declarations.push(...moduleDeclarations);
declarations.push(...unbootstrappedDeclarations);
}
}
@@ -351,41 +353,57 @@ function hasNgModuleMetadataElements(node: ts.Node): node is ts.PropertyAssignme
}
/** Finds all modules whose declarations can be migrated. */
function findModulesToMigrate(sourceFile: ts.SourceFile, typeChecker: ts.TypeChecker) {
function findNgModuleClassesToMigrate(sourceFile: ts.SourceFile, typeChecker: ts.TypeChecker) {
const modules: ts.ClassDeclaration[] = [];
if (getImportSpecifier(sourceFile, '@angular/core', 'NgModule')) {
sourceFile.forEachChild(function walk(node) {
if (ts.isClassDeclaration(node)) {
const decorator = getAngularDecorators(typeChecker, ts.getDecorators(node) || [])
.find(current => current.name === 'NgModule');
const metadata = decorator ? extractMetadataLiteral(decorator.node) : null;
if (metadata) {
const declarations = findLiteralProperty(metadata, 'declarations');
if (declarations != null && hasNgModuleMetadataElements(declarations)) {
modules.push(node);
}
}
}
node.forEachChild(walk);
});
}
return modules;
}
/** Finds all testing object literals that need to be migrated. */
export function findTestObjectsToMigrate(sourceFile: ts.SourceFile, typeChecker: ts.TypeChecker) {
const testObjects: ts.ObjectLiteralExpression[] = [];
const testBedImport = getImportSpecifier(sourceFile, '@angular/core/testing', 'TestBed');
const catalystImport = getImportSpecifier(sourceFile, /testing\/catalyst$/, 'setupModule');
sourceFile.forEachChild(function walk(node) {
if (ts.isClassDeclaration(node)) {
const decorator = getAngularDecorators(typeChecker, ts.getDecorators(node) || [])
.find(current => current.name === 'NgModule');
const metadata = decorator ? extractMetadataLiteral(decorator.node) : null;
if (metadata) {
const declarations = findLiteralProperty(metadata, 'declarations');
if (declarations != null && hasNgModuleMetadataElements(declarations)) {
modules.push(node);
if (testBedImport || catalystImport) {
sourceFile.forEachChild(function walk(node) {
if (ts.isCallExpression(node) && node.arguments.length > 0 &&
// `arguments[0]` is the testing module config.
ts.isObjectLiteralExpression(node.arguments[0])) {
if ((testBedImport && ts.isPropertyAccessExpression(node.expression) &&
node.expression.name.text === 'configureTestingModule' &&
isReferenceToImport(typeChecker, node.expression.expression, testBedImport)) ||
(catalystImport && ts.isIdentifier(node.expression) &&
isReferenceToImport(typeChecker, node.expression, catalystImport))) {
testObjects.push(node.arguments[0]);
}
}
} else if (
ts.isCallExpression(node) && node.arguments.length > 0 &&
ts.isObjectLiteralExpression(node.arguments[0])) {
if ((testBedImport && ts.isPropertyAccessExpression(node.expression) &&
node.expression.name.text === 'configureTestingModule' &&
isReferenceToImport(typeChecker, node.expression.expression, testBedImport)) ||
(catalystImport && ts.isIdentifier(node.expression) &&
isReferenceToImport(typeChecker, node.expression, catalystImport))) {
testObjects.push(node.arguments[0]);
}
}
node.forEachChild(walk);
});
node.forEachChild(walk);
});
}
return {modules, testObjects};
return testObjects;
}
/**
@@ -422,27 +440,29 @@ function findTemplateDependencies(
return results;
}
/** Extracts classes that are referred to in a module's `declarations` array. */
function extractDeclarationsFromModule(
ngModule: ts.ClassDeclaration, typeChecker: ts.TypeChecker,
templateTypeChecker: TemplateTypeChecker): Reference<ts.ClassDeclaration>[] {
/**
* Removes any declarations that are a part of a module's `bootstrap`
* array from an array of declarations.
* @param declarations Anaalyzed declarations of the module.
* @param ngModule Module whote declarations are being filtered.
* @param templateTypeChecker
* @param typeChecker
*/
function filterNonBootstrappedDeclarations(
declarations: Reference<ts.ClassDeclaration>[], ngModule: ts.ClassDeclaration,
templateTypeChecker: TemplateTypeChecker, typeChecker: ts.TypeChecker) {
const metadata = templateTypeChecker.getNgModuleMetadata(ngModule);
if (!metadata) {
return [];
}
const allDeclarations = metadata.declarations.filter(decl => ts.isClassDeclaration(decl.node)) as
Reference<ts.ClassDeclaration>[];
const metaLiteral = metadata.decorator ? extractMetadataLiteral(metadata.decorator) : null;
const metaLiteral =
metadata && metadata.decorator ? extractMetadataLiteral(metadata.decorator) : null;
const bootstrapProp = metaLiteral ? findLiteralProperty(metaLiteral, 'bootstrap') : null;
// If there's no `bootstrap`, we can migrate all the declarations.
// If there's no `bootstrap`, we can't filter.
if (!bootstrapProp) {
return allDeclarations;
return declarations;
}
// If we can't analyze the `bootstrap` property, we can't safely migrate the module.
// If we can't analyze the `bootstrap` property, we can't safely determine which
// declarations aren't bootstrapped so we assume that all of them are.
if (!ts.isPropertyAssignment(bootstrapProp) ||
!ts.isArrayLiteralExpression(bootstrapProp.initializer)) {
return [];
@@ -453,8 +473,8 @@ function extractDeclarationsFromModule(
for (const el of bootstrapProp.initializer.elements) {
const referencedClass = ts.isIdentifier(el) ? findClassDeclaration(el, typeChecker) : null;
// If we manage to resolve the class, we can use it to filter out declarations that are
// being bootstrapped. Otherwise we risk breaking the app so we skip the entire module.
// If we can resolve an element to a class, we can filter it out,
// otherwise assume that the array isn't static.
if (referencedClass) {
bootstrappedClasses.add(referencedClass);
} else {
@@ -462,15 +482,21 @@ function extractDeclarationsFromModule(
}
}
return allDeclarations.filter(ref => !bootstrappedClasses.has(ref.node));
return declarations.filter(ref => !bootstrappedClasses.has(ref.node));
}
/** Extracts the metadata object literal from an Angular decorator. */
function extractMetadataLiteral(decorator: ts.Decorator): ts.ObjectLiteralExpression|null {
return ts.isCallExpression(decorator.expression) && decorator.expression.arguments.length === 1 &&
ts.isObjectLiteralExpression(decorator.expression.arguments[0]) ?
decorator.expression.arguments[0] :
null;
/**
* Extracts all classes that are referenced in a module's `declarations` array.
* @param ngModule Module whose declarations are being extraced.
* @param templateTypeChecker
*/
export function extractDeclarationsFromModule(
ngModule: ts.ClassDeclaration,
templateTypeChecker: TemplateTypeChecker): Reference<ts.ClassDeclaration>[] {
const metadata = templateTypeChecker.getNgModuleMetadata(ngModule);
return metadata ? metadata.declarations.filter(decl => ts.isClassDeclaration(decl.node)) as
Reference<ts.ClassDeclaration>[] :
[];
}
/**
@@ -479,7 +505,7 @@ function extractMetadataLiteral(decorator: ts.Decorator): ts.ObjectLiteralExpres
* @param tracker
* @param typeChecker
*/
function migrateTestDeclarations(
export function migrateTestDeclarations(
testObjects: ts.ObjectLiteralExpression[],
declarationsOutsideOfTestFiles: Reference<ts.ClassDeclaration>[], tracker: ChangeTracker,
typeChecker: ts.TypeChecker) {
@@ -600,3 +626,12 @@ function extractDeclarationsFromTestObject(
return results;
}
/** Extracts the metadata object literal from an Angular decorator. */
function extractMetadataLiteral(decorator: ts.Decorator): ts.ObjectLiteralExpression|null {
// `arguments[0]` is the metadata object literal.
return ts.isCallExpression(decorator.expression) && decorator.expression.arguments.length === 1 &&
ts.isObjectLiteralExpression(decorator.expression.arguments[0]) ?
decorator.expression.arguments[0] :
null;
}
@@ -2270,6 +2270,65 @@ describe('standalone migration', () => {
`));
});
it('should migrate the root component tests when converting to standalone', async () => {
writeFile('main.ts', `
import {AppModule} from './app/app.module';
import {platformBrowser} from '@angular/platform-browser';
platformBrowser().bootstrapModule(AppModule).catch(e => console.error(e));
`);
writeFile('./app/app.component.ts', `
import {Component} from '@angular/core';
@Component({template: 'hello'})
export class AppComponent {}
`);
writeFile('./app/app.component.spec.ts', `
import {TestBed} from '@angular/core/testing';
import {AppComponent} from './app.component';
describe('bootrstrapping an app', () => {
it('should work', () => {
TestBed.configureTestingModule({declarations: [AppComponent]});
const fixture = TestBed.createComponent(AppComponent);
expect(fixture.nativeElement.innerHTML).toBe('hello');
});
});
`);
writeFile('./app/app.module.ts', `
import {NgModule} from '@angular/core';
import {AppComponent} from './app.component';
@NgModule({declarations: [AppComponent], bootstrap: [AppComponent]})
export class AppModule {}
`);
await runMigration('standalone-bootstrap');
expect(stripWhitespace(tree.readContent('./app/app.component.ts'))).toBe(stripWhitespace(`
import {Component} from '@angular/core';
@Component({template: 'hello', standalone: true})
export class AppComponent {}
`));
expect(stripWhitespace(tree.readContent('./app/app.component.spec.ts'))).toBe(stripWhitespace(`
import {TestBed} from '@angular/core/testing';
import {AppComponent} from './app.component';
describe('bootrstrapping an app', () => {
it('should work', () => {
TestBed.configureTestingModule({imports: [AppComponent]});
const fixture = TestBed.createComponent(AppComponent);
expect(fixture.nativeElement.innerHTML).toBe('hello');
});
});
`));
});
it('should copy providers and the symbols they depend on to the main file', async () => {
writeFile('main.ts', `
import {AppModule} from './app/app.module';