Elgato_dark/angular
1
0
Fork 0
mirror of https://github.com/angular/angular synced 2026-05-24 09:28:37 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

fix(migrations): cf migration - improve import declaration handling (#53622)

Browse source 
This should make the import declaration symbol removal a bit more robust and handle more than just CommonModule safely.

PR Close #53622
This commit is contained in:
Jessica Janiuk 2023-12-18 15:42:15 -05:00
parent 8460a4380c
commit b40bb22a66
5 changed files with 74 additions and 9 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
packages/core/schematics/ng-generate/control-flow-migration/identifier-lookup.ts
View file

@ -9,10 +9,10 @@
import ts from 'typescript';
export function lookupIdentifiersInSourceFile(
sourceFile: ts.SourceFile, name: string): Set<ts.Identifier> {
sourceFile: ts.SourceFile, names: string[]): Set<ts.Identifier> {
const results = new Set<ts.Identifier>();
const visit = (node: ts.Node): void => {
if (ts.isIdentifier(node) && node.text === name) {
if (ts.isIdentifier(node) && names.includes(node.text)) {
results.add(node);
}
ts.forEachChild(node, visit);

1
packages/core/schematics/ng-generate/control-flow-migration/migration.ts
View file

@ -82,6 +82,7 @@ export function migrateTemplate(
// and in that case we can't safely remove the common module import.
componentFile.verifyCanRemoveImports();
}
file.verifyCanRemoveImports();
errors = [
...ifResult.errors,

9
packages/core/schematics/ng-generate/control-flow-migration/types.ts
View file

@ -23,6 +23,13 @@ export const endMarker = '✢';
export const startI18nMarker = '⚈';
export const endI18nMarker = '⚉';
export const importRemovals = [
'NgIf', 'NgIfElse', 'NgIfThenElse', 'NgFor', 'NgForOf', 'NgForTrackBy', 'NgSwitch',
'NgSwitchCase', 'NgSwitchDefault'
];
export const importWithCommonRemovals = [...importRemovals, 'CommonModule'];
function allFormsOf(selector: string): string[] {
return [
selector,
@ -308,7 +315,7 @@ export class AnalyzedFile {
// skip this check entirely
if (this.removeCommonModule) {
const importDeclaration = this.importRanges.find(r => r.type === 'importDeclaration');
const instances = lookupIdentifiersInSourceFile(this.sourceFile, 'CommonModule');
const instances = lookupIdentifiersInSourceFile(this.sourceFile, importWithCommonRemovals);
let foundImportDeclaration = false;
let count = 0;
for (let range of this.importRanges) {

7
packages/core/schematics/ng-generate/control-flow-migration/util.ts
View file

@ -10,13 +10,8 @@ import {Attribute, Element, HtmlParser, Node, ParseTreeResult, visitAll} from '@
import {dirname, join} from 'path';
import ts from 'typescript';
import {AnalyzedFile, CommonCollector, ElementCollector, ElementToMigrate, endI18nMarker, endMarker, i18nCollector, ParseResult, startI18nMarker, startMarker, Template, TemplateCollector} from './types';
import {AnalyzedFile, CommonCollector, ElementCollector, ElementToMigrate, endI18nMarker, endMarker, i18nCollector, importRemovals, importWithCommonRemovals, ParseResult, startI18nMarker, startMarker, Template, TemplateCollector} from './types';
const importRemovals = [
'NgIf', 'NgIfElse', 'NgIfThenElse', 'NgFor', 'NgForOf', 'NgForTrackBy', 'NgSwitch',
'NgSwitchCase', 'NgSwitchDefault'
];
const importWithCommonRemovals = [...importRemovals, 'CommonModule'];
const startMarkerRegex = new RegExp(startMarker, 'gm');
const endMarkerRegex = new RegExp(endMarker, 'gm');
const startI18nMarkerRegex = new RegExp(startI18nMarker, 'gm');

62
packages/core/schematics/test/control_flow_migration_spec.ts
View file

@ -5087,6 +5087,68 @@ describe('control flow migration', () => {
expect(actual).toBe(expected);
});
it('should not remove imports when mismatch in counts', async () => {
writeFile('/comp.ts', [
`import {CommonModule} from '@angular/common';`,
`import {Component, NgModule, Pipe, PipeTransform} from '@angular/core';`,
`@Component({`,
` selector: 'description',`,
` template: \`<span>{{getDescription()}}</span>\`,`,
`})`,
`export class DescriptionController {`,
` getDescription(): string {`,
` return 'stuff';`,
` }`,
`}`,
``,
`@Pipe({name: 'description'})`,
`export class DescriptionPipe implements PipeTransform {`,
` transform(nameString?: string): string {`,
` return nameString ?? '';`,
` }`,
`}`,
`@NgModule({`,
` declarations: [DescriptionController, DescriptionPipe],`,
` imports: [CommonModule],`,
` providers: [],`,
` exports: [DescriptionController, DescriptionPipe],`,
`})`,
`export class DescriptionModule {}`,
].join('\n'));
await runMigration();
const actual = tree.readContent('/comp.ts');
const expected = [
`import {CommonModule} from '@angular/common';`,
`import {Component, NgModule, Pipe, PipeTransform} from '@angular/core';`,
`@Component({`,
` selector: 'description',`,
` template: \`<span>{{getDescription()}}</span>\`,`,
`})`,
`export class DescriptionController {`,
` getDescription(): string {`,
` return 'stuff';`,
` }`,
`}`,
``,
`@Pipe({name: 'description'})`,
`export class DescriptionPipe implements PipeTransform {`,
` transform(nameString?: string): string {`,
` return nameString ?? '';`,
` }`,
`}`,
`@NgModule({`,
` declarations: [DescriptionController, DescriptionPipe],`,
` imports: [CommonModule],`,
` providers: [],`,
` exports: [DescriptionController, DescriptionPipe],`,
`})`,
`export class DescriptionModule {}`,
].join('\n');
expect(actual).toBe(expected);
});
});
describe('no migration needed', () => {