mirror of
https://github.com/angular/angular
synced 2026-05-24 09:28:37 +00:00
fix(migrations): don't copy animations modules into the imports of test components (#49147)
Since we have less information about how to copy test components, we copy all the `imports` from the `configureTestingModule` call into the component's `imports`. It fixes some tests, but it can cause issues with animations modules, because they throw errors if they're imported multiple times. These changes add an exception for animations modules imported in testing modules. PR Close #49147
This commit is contained in:
parent
6966a043c0
commit
2268278ce9
4 changed files with 129 additions and 37 deletions
|
|
@ -16,7 +16,7 @@ import {getAngularDecorators} from '../../utils/ng_decorators';
|
|||
import {closestNode} from '../../utils/typescript/nodes';
|
||||
|
||||
import {ComponentImportsRemapper, convertNgModuleDeclarationToStandalone, extractDeclarationsFromModule, findTestObjectsToMigrate, migrateTestDeclarations} from './to-standalone';
|
||||
import {ChangeTracker, findClassDeclaration, findLiteralProperty, getNodeLookup, getRelativeImportPath, ImportRemapper, NamedClassDeclaration, NodeLookup, offsetsToNodes, ReferenceResolver, UniqueItemTracker} from './util';
|
||||
import {ChangeTracker, closestOrSelf, findClassDeclaration, findLiteralProperty, getNodeLookup, getRelativeImportPath, ImportRemapper, isClassReferenceInAngularModule, NamedClassDeclaration, NodeLookup, offsetsToNodes, ReferenceResolver, UniqueItemTracker} from './util';
|
||||
|
||||
/** Information extracted from a `bootstrapModule` call necessary to migrate it. */
|
||||
interface BootstrapCallAnalysis {
|
||||
|
|
@ -658,16 +658,6 @@ function isExported(node: ts.Node): node is ts.Node {
|
|||
false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Gets the closest node that matches a predicate, including the node that the search started from.
|
||||
* @param node Node from which to start the search.
|
||||
* @param predicate Predicate that the result needs to pass.
|
||||
*/
|
||||
function closestOrSelf<T extends ts.Node>(node: ts.Node, predicate: (n: ts.Node) => n is T): T|
|
||||
null {
|
||||
return predicate(node) ? node : closestNode(node, predicate);
|
||||
}
|
||||
|
||||
/**
|
||||
* Asserts that a node is an exportable declaration, which means that it can either be exported or
|
||||
* it can be safely copied into another file.
|
||||
|
|
@ -680,29 +670,6 @@ function isExportableDeclaration(node: ts.Node): node is ts.EnumDeclaration|ts.C
|
|||
ts.isTypeAliasDeclaration(node);
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks whether a node is referring to a specific class declaration.
|
||||
* @param node Node that is being checked.
|
||||
* @param className Name of the class that the node might be referring to.
|
||||
* @param moduleName Name of the Angular module that should contain the class.
|
||||
* @param typeChecker
|
||||
*/
|
||||
function isClassReferenceInAngularModule(
|
||||
node: ts.Node, className: string, moduleName: string, typeChecker: ts.TypeChecker): boolean {
|
||||
const symbol = typeChecker.getTypeAtLocation(node).getSymbol();
|
||||
const externalName = `@angular/${moduleName}`;
|
||||
const internalName = `angular2/rc/packages/${moduleName}`;
|
||||
|
||||
return !!symbol?.declarations?.some(decl => {
|
||||
const closestClass = closestOrSelf(decl, ts.isClassDeclaration);
|
||||
const closestClassFileName = closestClass?.getSourceFile().fileName;
|
||||
return closestClass && closestClassFileName && closestClass.name &&
|
||||
ts.isIdentifier(closestClass.name) && closestClass.name.text === className &&
|
||||
(closestClassFileName.includes(externalName) ||
|
||||
closestClassFileName.includes(internalName));
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Gets the index after the last import in a file. Can be used to insert new code into the file.
|
||||
* @param sourceFile File in which to search for imports.
|
||||
|
|
|
|||
|
|
@ -15,7 +15,7 @@ import {getImportSpecifier} from '../../utils/typescript/imports';
|
|||
import {closestNode} from '../../utils/typescript/nodes';
|
||||
import {isReferenceToImport} from '../../utils/typescript/symbol';
|
||||
|
||||
import {ChangesByFile, ChangeTracker, findClassDeclaration, findLiteralProperty, ImportRemapper, NamedClassDeclaration} from './util';
|
||||
import {ChangesByFile, ChangeTracker, findClassDeclaration, findLiteralProperty, ImportRemapper, isClassReferenceInAngularModule, NamedClassDeclaration} from './util';
|
||||
|
||||
/**
|
||||
* Function that can be used to prcess the dependencies that
|
||||
|
|
@ -602,8 +602,16 @@ function analyzeTestingModules(
|
|||
|
||||
const importsProp = findLiteralProperty(obj, 'imports');
|
||||
const importElements = importsProp && hasNgModuleMetadataElements(importsProp) ?
|
||||
// Filter out calls since they may be a `ModuleWithProviders`.
|
||||
importsProp.initializer.elements.filter(el => !ts.isCallExpression(el)) :
|
||||
importsProp.initializer.elements.filter(el => {
|
||||
// Filter out calls since they may be a `ModuleWithProviders`.
|
||||
return !ts.isCallExpression(el) &&
|
||||
// Also filter out the animations modules since they throw errors if they're imported
|
||||
// multiple times and it's common for apps to use the `NoopAnimationsModule` to
|
||||
// disable animations in screenshot tests.
|
||||
!isClassReferenceInAngularModule(
|
||||
el, /^BrowserAnimationsModule|NoopAnimationsModule$/,
|
||||
'platform-browser/animations', typeChecker);
|
||||
}) :
|
||||
null;
|
||||
|
||||
for (const decl of declarations) {
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ import {dirname, relative} from 'path';
|
|||
import ts from 'typescript';
|
||||
|
||||
import {ImportManager} from '../../utils/import_manager';
|
||||
import {closestNode} from '../../utils/typescript/nodes';
|
||||
|
||||
/** Mapping between a source file and the changes that have to be applied to it. */
|
||||
export type ChangesByFile = ReadonlyMap<ts.SourceFile, PendingChange[]>;
|
||||
|
|
@ -391,3 +392,43 @@ export function knownInternalAliasRemapper(imports: PotentialImport[]) {
|
|||
{...current, symbolName: 'NgFor'} :
|
||||
current);
|
||||
}
|
||||
|
||||
/**
|
||||
* Gets the closest node that matches a predicate, including the node that the search started from.
|
||||
* @param node Node from which to start the search.
|
||||
* @param predicate Predicate that the result needs to pass.
|
||||
*/
|
||||
export function closestOrSelf<T extends ts.Node>(
|
||||
node: ts.Node, predicate: (n: ts.Node) => n is T): T|null {
|
||||
return predicate(node) ? node : closestNode(node, predicate);
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks whether a node is referring to a specific class declaration.
|
||||
* @param node Node that is being checked.
|
||||
* @param className Name of the class that the node might be referring to.
|
||||
* @param moduleName Name of the Angular module that should contain the class.
|
||||
* @param typeChecker
|
||||
*/
|
||||
export function isClassReferenceInAngularModule(
|
||||
node: ts.Node, className: string|RegExp, moduleName: string,
|
||||
typeChecker: ts.TypeChecker): boolean {
|
||||
const symbol = typeChecker.getTypeAtLocation(node).getSymbol();
|
||||
const externalName = `@angular/${moduleName}`;
|
||||
const internalName = `angular2/rc/packages/${moduleName}`;
|
||||
|
||||
return !!symbol?.declarations?.some(decl => {
|
||||
const closestClass = closestOrSelf(decl, ts.isClassDeclaration);
|
||||
const closestClassFileName = closestClass?.getSourceFile().fileName;
|
||||
|
||||
if (!closestClass || !closestClassFileName || !closestClass.name ||
|
||||
!ts.isIdentifier(closestClass.name) ||
|
||||
(!closestClassFileName.includes(externalName) &&
|
||||
!closestClassFileName.includes(internalName))) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return typeof className === 'string' ? closestClass.name.text === className :
|
||||
className.test(closestClass.name.text);
|
||||
});
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1274,6 +1274,82 @@ describe('standalone migration', () => {
|
|||
`));
|
||||
});
|
||||
|
||||
it('should not copy over the NoopAnimationsModule into the imports of a test component',
|
||||
async () => {
|
||||
writeFile('app.spec.ts', `
|
||||
import {NgModule, Component} from '@angular/core';
|
||||
import {TestBed} from '@angular/core/testing';
|
||||
import {MatCardModule} from '@angular/material/card';
|
||||
import {NoopAnimationsModule} from '@angular/platform-browser/animations';
|
||||
|
||||
describe('bootstrapping an app', () => {
|
||||
it('should work', () => {
|
||||
TestBed.configureTestingModule({
|
||||
imports: [MatCardModule, NoopAnimationsModule],
|
||||
declarations: [App]
|
||||
});
|
||||
const fixture = TestBed.createComponent(App);
|
||||
expect(fixture.nativeElement.innerHTML).toBe('<hello>Hello</hello>');
|
||||
});
|
||||
});
|
||||
|
||||
@Component({template: 'hello'})
|
||||
class App {}
|
||||
`);
|
||||
|
||||
await runMigration('convert-to-standalone');
|
||||
|
||||
const content = stripWhitespace(tree.readContent('app.spec.ts'));
|
||||
|
||||
expect(content).toContain(stripWhitespace(`
|
||||
TestBed.configureTestingModule({
|
||||
imports: [MatCardModule, NoopAnimationsModule, App]
|
||||
});
|
||||
`));
|
||||
expect(content).toContain(stripWhitespace(`
|
||||
@Component({template: 'hello', standalone: true, imports: [MatCardModule]})
|
||||
class App {}
|
||||
`));
|
||||
});
|
||||
|
||||
it('should not copy over the BrowserAnimationsModule into the imports of a test component',
|
||||
async () => {
|
||||
writeFile('app.spec.ts', `
|
||||
import {NgModule, Component} from '@angular/core';
|
||||
import {TestBed} from '@angular/core/testing';
|
||||
import {MatCardModule} from '@angular/material/card';
|
||||
import {BrowserAnimationsModule} from '@angular/platform-browser/animations';
|
||||
|
||||
describe('bootstrapping an app', () => {
|
||||
it('should work', () => {
|
||||
TestBed.configureTestingModule({
|
||||
imports: [MatCardModule, BrowserAnimationsModule],
|
||||
declarations: [App]
|
||||
});
|
||||
const fixture = TestBed.createComponent(App);
|
||||
expect(fixture.nativeElement.innerHTML).toBe('<hello>Hello</hello>');
|
||||
});
|
||||
});
|
||||
|
||||
@Component({template: 'hello'})
|
||||
class App {}
|
||||
`);
|
||||
|
||||
await runMigration('convert-to-standalone');
|
||||
|
||||
const content = stripWhitespace(tree.readContent('app.spec.ts'));
|
||||
|
||||
expect(content).toContain(stripWhitespace(`
|
||||
TestBed.configureTestingModule({
|
||||
imports: [MatCardModule, BrowserAnimationsModule, App]
|
||||
});
|
||||
`));
|
||||
expect(content).toContain(stripWhitespace(`
|
||||
@Component({template: 'hello', standalone: true, imports: [MatCardModule]})
|
||||
class App {}
|
||||
`));
|
||||
});
|
||||
|
||||
it('should not move declarations that are not being migrated out of the declarations array',
|
||||
async () => {
|
||||
const appComponentContent = `
|
||||
|
|
|
|||
Loading…
Reference in a new issue