mirror of
https://github.com/angular/angular
synced 2026-05-24 09:28:37 +00:00
fix(migrations): prevent duplicate imports in common-to-standalone migration
The common-to-standalone migration did not check for existing imports
when adding needed imports after removing CommonModule. This change adds deduplication
logic to filter out imports that already exist in the imports array
before adding them, ensuring each import appears only once.
(cherry picked from commit 008e20ca56)
This commit is contained in:
parent
5bc072ea3e
commit
e4bfa5c9e7
2 changed files with 72 additions and 14 deletions
|
|
@ -136,10 +136,18 @@ function createCommonModuleImportsArrayRemoval(
|
|||
return !isCommonModuleFromAngularCommon(typeChecker, el);
|
||||
});
|
||||
|
||||
const newElements = [
|
||||
...filteredElements,
|
||||
...neededImports.sort().map((imp) => ts.factory.createIdentifier(imp)),
|
||||
];
|
||||
// Get existing import names to avoid duplicates
|
||||
const existingImportNames = new Set(
|
||||
filteredElements.filter((el) => ts.isIdentifier(el)).map((el) => el.getText()),
|
||||
);
|
||||
|
||||
// Only add imports that don't already exist
|
||||
const importsToAdd = neededImports
|
||||
.filter((imp) => !existingImportNames.has(imp))
|
||||
.sort()
|
||||
.map((imp) => ts.factory.createIdentifier(imp));
|
||||
|
||||
const newElements = [...filteredElements, ...importsToAdd];
|
||||
|
||||
if (newElements.length === originalElements.length && neededImports.length === 0) {
|
||||
return null;
|
||||
|
|
|
|||
|
|
@ -29,16 +29,31 @@ function expectImportsToContain(content: string, ...importNames: string[]): void
|
|||
|
||||
// Helper function to check import declarations with flexible formatting
|
||||
function expectImportDeclarationToContain(content: string, ...importNames: string[]): void {
|
||||
// Create individual patterns for each import name with flexible spacing
|
||||
const sortedImports = importNames
|
||||
.sort()
|
||||
.map((name) => `\\s*${name}\\s*`)
|
||||
.join(',');
|
||||
// Create regex pattern that matches import with flexible spacing: import { NgIf } or import {NgIf}
|
||||
const importPattern = new RegExp(
|
||||
`import\\s*\\{${sortedImports}\\}\\s*from\\s*['"]@angular\\/common['"]`,
|
||||
);
|
||||
expect(content).toMatch(importPattern);
|
||||
// Sort the import names to match the sorted order in the actual imports
|
||||
const sortedImports = [...importNames].sort();
|
||||
|
||||
// Match both single-line and multi-line import formats
|
||||
// Pattern matches: import { A, B, C } from '@angular/common'
|
||||
// or: import {\n A,\n B,\n C\n} from '@angular/common'
|
||||
const importRegex = /import\s*\{([^}]+)\}\s*from\s*['"]@angular\/common['"]/;
|
||||
const match = content.match(importRegex);
|
||||
|
||||
expect(match).toBeTruthy('Should have an import from @angular/common');
|
||||
|
||||
// Extract and normalize the imported names
|
||||
const importedNames = match![1]
|
||||
.split(',')
|
||||
.map((name) => name.trim())
|
||||
.filter((name) => name.length > 0)
|
||||
.sort();
|
||||
|
||||
// Check that all expected imports are present
|
||||
for (const expectedImport of sortedImports) {
|
||||
expect(importedNames).toContain(expectedImport);
|
||||
}
|
||||
|
||||
// Check that we have the exact same imports (no extras)
|
||||
expect(importedNames).toEqual(sortedImports);
|
||||
}
|
||||
|
||||
describe('Common → standalone imports migration', () => {
|
||||
|
|
@ -1493,6 +1508,41 @@ describe('Common → standalone imports migration', () => {
|
|||
expect(content).not.toContain('CommonModule');
|
||||
});
|
||||
});
|
||||
it('should not create duplicate imports when imports already exist', async () => {
|
||||
writeFile(
|
||||
'/comp.ts',
|
||||
dedent`
|
||||
import {Component} from '@angular/core';
|
||||
import {CommonModule, AsyncPipe} from '@angular/common';
|
||||
import {SomeOtherModule} from './some-other-module';
|
||||
|
||||
@Component({
|
||||
selector: 'app-duplicate-test',
|
||||
imports: [CommonModule, AsyncPipe, SomeOtherModule],
|
||||
template: \`<div>{{ data$ | async }}</div>\`
|
||||
})
|
||||
export class DuplicateTestComponent {
|
||||
data$ = Promise.resolve('test');
|
||||
}
|
||||
`,
|
||||
);
|
||||
|
||||
await runMigration();
|
||||
const content = tree.readContent('/comp.ts');
|
||||
|
||||
// Should remove CommonModule and keep AsyncPipe without duplicating it
|
||||
expect(content).not.toContain('CommonModule');
|
||||
expectImportsToContain(content, 'AsyncPipe', 'SomeOtherModule');
|
||||
|
||||
// Verify AsyncPipe appears only once in the imports array
|
||||
const importsMatch = content.match(/imports:\s*\[([\s\S]*?)\]/);
|
||||
expect(importsMatch).toBeTruthy();
|
||||
const importsArray = importsMatch![1];
|
||||
const asyncPipeMatches = importsArray.match(/AsyncPipe/g);
|
||||
expect(asyncPipeMatches?.length).toBe(1);
|
||||
|
||||
expectImportDeclarationToContain(content, 'AsyncPipe');
|
||||
});
|
||||
|
||||
describe('Robustness and edge cases', () => {
|
||||
it('should handle very large templates without performance issues', async () => {
|
||||
|
|
|
|||
Loading…
Reference in a new issue