From e4bfa5c9e7feec48d3c4e9425a21a2ccf6532bdb Mon Sep 17 00:00:00 2001 From: tomer953 Date: Tue, 25 Nov 2025 23:45:04 +0200 Subject: [PATCH] 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 008e20ca56a1b881c695fd68399c45e84287f45c) --- .../common-to-standalone-migration/util.ts | 16 +++-- .../common_to_standalone_migration_spec.ts | 70 ++++++++++++++++--- 2 files changed, 72 insertions(+), 14 deletions(-) diff --git a/packages/core/schematics/migrations/common-to-standalone-migration/util.ts b/packages/core/schematics/migrations/common-to-standalone-migration/util.ts index 93897c5d967..14ad792a44d 100644 --- a/packages/core/schematics/migrations/common-to-standalone-migration/util.ts +++ b/packages/core/schematics/migrations/common-to-standalone-migration/util.ts @@ -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; diff --git a/packages/core/schematics/test/common_to_standalone_migration_spec.ts b/packages/core/schematics/test/common_to_standalone_migration_spec.ts index ec31c99e164..d8e27c4971d 100644 --- a/packages/core/schematics/test/common_to_standalone_migration_spec.ts +++ b/packages/core/schematics/test/common_to_standalone_migration_spec.ts @@ -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: \`
{{ data$ | async }}
\` + }) + 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 () => {