From 32cf4e5cb989f365296d519dddf72fb38ca47c40 Mon Sep 17 00:00:00 2001
From: Kristiyan Kostadinov <crisbeto@abv.bg>
Date: Sun, 5 Feb 2023 11:23:35 +0100
Subject: [PATCH] fix(migrations): avoid internal modules when generating
 imports (#48958)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Adds some logic to prefer non-Angular-internal modules when generating imports. This allows us to generate better code for some cases like the `ɵInternalFormsSharedModule` in Forms.

Also adds some logic to prefer symbols that are already in the same file.

Fixes #48942.

PR Close #48958
---
 .../standalone-migration/to-standalone.ts     | 10 +++-
 .../test/standalone_migration_spec.ts         | 46 +++++++++++++++++++
 2 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/packages/core/schematics/ng-generate/standalone-migration/to-standalone.ts b/packages/core/schematics/ng-generate/standalone-migration/to-standalone.ts
index f5ee3c2073c..104cfba8e98 100644
--- a/packages/core/schematics/ng-generate/standalone-migration/to-standalone.ts
+++ b/packages/core/schematics/ng-generate/standalone-migration/to-standalone.ts
@@ -310,6 +310,7 @@ function findImportLocation(
     target: Reference<NamedClassDeclaration>, inComponent: Reference<ts.ClassDeclaration>,
     importMode: PotentialImportMode, typeChecker: TemplateTypeChecker): PotentialImport|null {
   const importLocations = typeChecker.getPotentialImportsFor(target, inComponent.node, importMode);
+  let firstSameFileImport: PotentialImport|null = null;
   let firstModuleImport: PotentialImport|null = null;
 
   for (const location of importLocations) {
@@ -318,12 +319,17 @@ function findImportLocation(
     if (location.kind === PotentialImportKind.Standalone) {
       return location;
     }
-    if (location.kind === PotentialImportKind.NgModule && !firstModuleImport) {
+    if (!location.moduleSpecifier && !firstSameFileImport) {
+      firstSameFileImport = location;
+    }
+    if (location.kind === PotentialImportKind.NgModule && !firstModuleImport &&
+        // ɵ is used for some internal Angular modules that we want to skip over.
+        !location.symbolName.startsWith('ɵ')) {
       firstModuleImport = location;
     }
   }
 
-  return firstModuleImport;
+  return firstSameFileImport || firstModuleImport || importLocations[0] || null;
 }
 
 /**
diff --git a/packages/core/schematics/test/standalone_migration_spec.ts b/packages/core/schematics/test/standalone_migration_spec.ts
index 881380af2e1..a25f145c844 100644
--- a/packages/core/schematics/test/standalone_migration_spec.ts
+++ b/packages/core/schematics/test/standalone_migration_spec.ts
@@ -970,6 +970,52 @@ describe('standalone migration', () => {
     expect(myCompContent).toContain('imports: [ButtonModule]');
   });
 
+  it('should not reference internal modules', async () => {
+    writeFile('./should-migrate/module.ts', `
+      import {NgModule} from '@angular/core';
+      import {MyComp} from './comp';
+      import {ɵButtonModule} from '../do-not-migrate/button.module';
+
+      @NgModule({imports: [ɵButtonModule], declarations: [MyComp]})
+      export class Mod {}
+    `);
+
+    writeFile('./should-migrate/comp.ts', `
+      import {Component} from '@angular/core';
+
+      @Component({selector: 'my-comp', template: '<my-button>Hello</my-button>'})
+      export class MyComp {}
+    `);
+
+    writeFile('./do-not-migrate/button.module.ts', `
+      import {NgModule, forwardRef} from '@angular/core';
+      import {MyButton} from './button';
+
+      @NgModule({
+        imports: [forwardRef(() => ɵButtonModule)],
+        exports: [forwardRef(() => ɵButtonModule)]
+      })
+      export class ExporterModule {}
+
+      @NgModule({declarations: [MyButton], exports: [MyButton]})
+      export class ɵButtonModule {}
+    `);
+
+    writeFile('./do-not-migrate/button.ts', `
+      import {Component} from '@angular/core';
+
+      @Component({selector: 'my-button', template: '<ng-content></ng-content>'})
+      export class MyButton {}
+    `);
+
+    await runMigration('convert-to-standalone', './should-migrate');
+
+    const myCompContent = tree.readContent('./should-migrate/comp.ts');
+    expect(myCompContent)
+        .toContain(`import { ExporterModule } from '../do-not-migrate/button.module';`);
+    expect(myCompContent).toContain('imports: [ExporterModule]');
+  });
+
   it('should migrate tests with a component declared through TestBed', async () => {
     writeFile('app.spec.ts', `
       import {NgModule, Component} from '@angular/core';