fix(migrations): prevent duplicate imports in common-to-standalone migration

tomer953 · thePunderWoman · commit 62d7533ca62c · 2026-01-09T22:30:52.000Z
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 008e20c)
diff --git 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
@@ -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 () => {
