From b6d6a34eef9bd3c8434cde8dcee9dcbd47e753b6 Mon Sep 17 00:00:00 2001
From: JoostK <joost.koehoorn@gmail.com>
Date: Wed, 2 Jun 2021 17:27:03 +0200
Subject: [PATCH] fix(compiler-cli): exclude type-only imports from cycle
 analysis (#42453)

Type-only imports are known to be elided by TypeScript, so the compiler
can be certain that such imports do not contribute to potential import
cycles. As such, type-only imports are no longer considered during cycle
analysis.

Regular import statements that would eventually be fully elided by
TypeScript during emit if none of the imported symbols are used in a
value position continue to be included in the cycle analysis, as the
cycle analyzer is unaware of these elision opportunities. Only explicit
`import type` statements are excluded.

PR Close #42453
---
 .../src/ngtsc/cycles/src/imports.ts           |  7 ++++
 .../src/ngtsc/cycles/test/analyzer_spec.ts    | 11 ++++++
 .../src/ngtsc/cycles/test/util.ts             |  5 +++
 .../compiler-cli/test/ngtsc/ngtsc_spec.ts     | 35 +++++++++++++++++++
 4 files changed, 58 insertions(+)

diff --git a/packages/compiler-cli/src/ngtsc/cycles/src/imports.ts b/packages/compiler-cli/src/ngtsc/cycles/src/imports.ts
index a04f0449653..3bd544432ee 100644
--- a/packages/compiler-cli/src/ngtsc/cycles/src/imports.ts
+++ b/packages/compiler-cli/src/ngtsc/cycles/src/imports.ts
@@ -110,6 +110,13 @@ export class ImportGraph {
           continue;
         }
 
+        if (ts.isImportDeclaration(stmt) && stmt.importClause !== undefined &&
+            stmt.importClause.isTypeOnly) {
+          // Exclude type-only imports as they are always elided, so they don't contribute to
+          // cycles.
+          continue;
+        }
+
         const symbol = this.checker.getSymbolAtLocation(stmt.moduleSpecifier);
         if (symbol === undefined || symbol.valueDeclaration === undefined) {
           // No symbol could be found to skip over this import/export.
diff --git a/packages/compiler-cli/src/ngtsc/cycles/test/analyzer_spec.ts b/packages/compiler-cli/src/ngtsc/cycles/test/analyzer_spec.ts
index 85b058dd8fc..29152580d69 100644
--- a/packages/compiler-cli/src/ngtsc/cycles/test/analyzer_spec.ts
+++ b/packages/compiler-cli/src/ngtsc/cycles/test/analyzer_spec.ts
@@ -70,6 +70,17 @@ runInEachFileSystem(() => {
       expect(cycle).toBeInstanceOf(Cycle);
       expect(importPath(cycle!.getPath())).toEqual('b,c,b');
     });
+
+    it('should not consider type-only imports', () => {
+      const {program, analyzer} = makeAnalyzer('a:b,c!;b;c');
+      const a = getSourceFileOrError(program, (_('/a.ts')));
+      const b = getSourceFileOrError(program, (_('/b.ts')));
+      const c = getSourceFileOrError(program, (_('/c.ts')));
+      expect(analyzer.wouldCreateCycle(c, a)).toBe(null);
+      const cycle = analyzer.wouldCreateCycle(b, a);
+      expect(cycle).toBeInstanceOf(Cycle);
+      expect(importPath(cycle!.getPath())).toEqual('b,a,b');
+    });
   });
 
   function makeAnalyzer(graph: string): {program: ts.Program, analyzer: CycleAnalyzer} {
diff --git a/packages/compiler-cli/src/ngtsc/cycles/test/util.ts b/packages/compiler-cli/src/ngtsc/cycles/test/util.ts
index c4a8ca4ed42..67868b936e7 100644
--- a/packages/compiler-cli/src/ngtsc/cycles/test/util.ts
+++ b/packages/compiler-cli/src/ngtsc/cycles/test/util.ts
@@ -30,6 +30,8 @@ import {makeProgram} from '../../testing';
  * "a:*b,c;b;c"
  *
  * represents a program where a.ts exports from b.ts and imports from c.ts.
+ *
+ * An import can be suffixed with ! to make it a type-only import.
  */
 export function makeProgramFromGraph(fs: PathManipulation, graph: string): {
   program: ts.Program,
@@ -43,6 +45,9 @@ export function makeProgramFromGraph(fs: PathManipulation, graph: string): {
                            if (i.startsWith('*')) {
                              const sym = i.substr(1);
                              return `export {${sym}} from './${sym}';`;
+                           } else if (i.endsWith('!')) {
+                             const sym = i.substr(0, i.length - 1);
+                             return `import type {${sym}} from './${sym}';`;
                            } else {
                              return `import {${i}} from './${i}';`;
                            }
diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
index 494b6045de7..d8849461747 100644
--- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
+++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
@@ -4836,6 +4836,41 @@ function allTests(os: string) {
         expect(jsContents).not.toContain('setComponentScope');
       });
 
+      it('should not consider type-only imports during cycle detection', () => {
+        env.write('test.ts', `
+        import {NgModule} from '@angular/core';
+        import {ACmp} from './a';
+        import {BCmp} from './b';
+
+        @NgModule({declarations: [ACmp, BCmp]})
+        export class Module {}
+      `);
+        env.write('a.ts', `
+        import {Component} from '@angular/core';
+
+        @Component({
+          selector: 'a-cmp',
+          template: '<b-cmp></b-cmp>',
+        })
+        export class ACmp {}
+      `);
+        env.write('b.ts', `
+        import {Component} from '@angular/core';
+        import type {ACmp} from './a';
+
+        @Component({
+          selector: 'b-cmp',
+          template: 'does not use a-cmp',
+        })
+        export class BCmp {
+          a: ACmp;
+        }
+      `);
+        env.driveMain();
+        const jsContents = env.getContents('test.js');
+        expect(jsContents).not.toContain('setComponentScope');
+      });
+
       it('should only pass components actually used to setComponentScope', () => {
         env.write('test.ts', `
           import {Component, NgModule} from '@angular/core';