fix(compiler): account for type-only imports in defer blocks (#52343)

crisbeto · dylhunn · commit b6b5adca384e · 2023-10-24T09:20:49.000-07:00
Fixes that `@defer` blocks didn't account for type-only imports which could cause the import to be considered as not deferrable. PR Close #52343
diff --git a/packages/compiler-cli/src/ngtsc/imports/src/deferred_symbol_tracker.ts b/packages/compiler-cli/src/ngtsc/imports/src/deferred_symbol_tracker.ts
@@ -44,12 +44,19 @@ export class DeferredSymbolTracker {
       throw new Error(`Provided import declaration doesn't have any symbols.`);
     }
 
+    // If the entire import is a type-only import, none of the symbols can be eager.
+    if (importDecl.importClause.isTypeOnly) {
+      return symbolMap;
+    }
+
     if (importDecl.importClause.namedBindings !== undefined) {
       const bindings = importDecl.importClause.namedBindings;
       if (ts.isNamedImports(bindings)) {
         // Case 1: `import {a, b as B} from 'a'`
         for (const element of bindings.elements) {
-          symbolMap.set(element.name.text, AssumeEager);
+          if (!element.isTypeOnly) {
+            symbolMap.set(element.name.text, AssumeEager);
+          }
         }
       } else {
         // Case 2: `import X from 'a'`
diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
@@ -9107,6 +9107,230 @@ function allTests(os: string) {
           // via dynamic imports and an original import can be removed.
           expect(jsContents).not.toContain('import { CmpA }');
         });
+
+        it('should drop imports when one is deferrable and the rest are type-only imports', () => {
+          env.write('cmp-a.ts', `
+            import { Component } from '@angular/core';
+
+            export class Foo {}
+
+            @Component({
+              standalone: true,
+              selector: 'cmp-a',
+              template: 'CmpA!'
+            })
+            export class CmpA {}
+          `);
+
+          env.write('/test.ts', `
+            import { Component } from '@angular/core';
+            import { CmpA, type Foo } from './cmp-a';
+
+            export const foo: Foo = {};
+
+            @Component({
+              selector: 'test-cmp',
+              standalone: true,
+              imports: [CmpA],
+              template: \`
+                @defer {
+                  <cmp-a />
+                }
+              \`,
+            })
+            export class TestCmp {}
+          `);
+
+          env.driveMain();
+
+          const jsContents = env.getContents('test.js');
+
+          expect(jsContents).toContain('ɵɵdefer(1, 0, TestCmp_Defer_1_DepsFn)');
+          expect(jsContents).toContain('() => [import("./cmp-a").then(m => m.CmpA)]');
+          expect(jsContents).not.toContain('import { CmpA }');
+        });
+
+        it('should drop multiple imports to the same file when one is deferrable and the other has a single type-only element',
+           () => {
+             env.write('cmp-a.ts', `
+                import { Component } from '@angular/core';
+
+                export class Foo {}
+
+                @Component({
+                  standalone: true,
+                  selector: 'cmp-a',
+                  template: 'CmpA!'
+                })
+                export class CmpA {}
+              `);
+
+             env.write('/test.ts', `
+                import { Component } from '@angular/core';
+                import { CmpA } from './cmp-a';
+                import { type Foo } from './cmp-a';
+
+                export const foo: Foo = {};
+
+                @Component({
+                  selector: 'test-cmp',
+                  standalone: true,
+                  imports: [CmpA],
+                  template: \`
+                    @defer {
+                      <cmp-a />
+                    }
+                  \`,
+                })
+                export class TestCmp {}
+              `);
+
+             env.driveMain();
+
+             const jsContents = env.getContents('test.js');
+
+             expect(jsContents).toContain('ɵɵdefer(1, 0, TestCmp_Defer_1_DepsFn)');
+             expect(jsContents).toContain('() => [import("./cmp-a").then(m => m.CmpA)]');
+             expect(jsContents).not.toContain('import { CmpA }');
+           });
+
+        it('should drop multiple imports to the same file when one is deferrable and the other is type-only at the declaration level',
+           () => {
+             env.write('cmp-a.ts', `
+                import { Component } from '@angular/core';
+
+                export class Foo {}
+
+                @Component({
+                  standalone: true,
+                  selector: 'cmp-a',
+                  template: 'CmpA!'
+                })
+                export class CmpA {}
+              `);
+
+             env.write('/test.ts', `
+                import { Component } from '@angular/core';
+                import { CmpA } from './cmp-a';
+                import type { Foo, CmpA as CmpAlias } from './cmp-a';
+
+                export const foo: Foo|CmpAlias = {};
+
+                @Component({
+                  selector: 'test-cmp',
+                  standalone: true,
+                  imports: [CmpA],
+                  template: \`
+                    @defer {
+                      <cmp-a />
+                    }
+                  \`,
+                })
+                export class TestCmp {}
+              `);
+
+             env.driveMain();
+
+             const jsContents = env.getContents('test.js');
+
+             expect(jsContents).toContain('ɵɵdefer(1, 0, TestCmp_Defer_1_DepsFn)');
+             expect(jsContents).toContain('() => [import("./cmp-a").then(m => m.CmpA)]');
+             expect(jsContents).not.toContain('import { CmpA }');
+           });
+
+        it('should drop multiple imports to the same file when one is deferrable and the other is a type-only import of all symbols',
+           () => {
+             env.write('cmp-a.ts', `
+                import { Component } from '@angular/core';
+
+                export class Foo {}
+
+                @Component({
+                  standalone: true,
+                  selector: 'cmp-a',
+                  template: 'CmpA!'
+                })
+                export class CmpA {}
+              `);
+
+             env.write('/test.ts', `
+                import { Component } from '@angular/core';
+                import { CmpA } from './cmp-a';
+                import type * as allCmpA from './cmp-a';
+
+                export const foo: allCmpA.Foo|allCmpA.CmpA = {};
+
+                @Component({
+                  selector: 'test-cmp',
+                  standalone: true,
+                  imports: [CmpA],
+                  template: \`
+                    @defer {
+                      <cmp-a />
+                    }
+                  \`,
+                })
+                export class TestCmp {}
+              `);
+
+             env.driveMain();
+
+             const jsContents = env.getContents('test.js');
+
+             expect(jsContents).toContain('ɵɵdefer(1, 0, TestCmp_Defer_1_DepsFn)');
+             expect(jsContents).toContain('() => [import("./cmp-a").then(m => m.CmpA)]');
+             expect(jsContents).not.toContain('import { CmpA }');
+           });
+
+        it('should drop multiple imports of deferrable symbols from the same file', () => {
+          env.write('cmps.ts', `
+            import { Component } from '@angular/core';
+
+            @Component({
+              standalone: true,
+              selector: 'cmp-a',
+              template: 'CmpA!'
+            })
+            export class CmpA {}
+
+            @Component({
+              standalone: true,
+              selector: 'cmp-b',
+              template: 'CmpB!'
+            })
+            export class CmpB {}
+          `);
+
+          env.write('/test.ts', `
+            import { Component } from '@angular/core';
+            import { CmpA } from './cmps';
+            import { CmpB } from './cmps';
+
+            @Component({
+              selector: 'test-cmp',
+              standalone: true,
+              imports: [CmpA, CmpB],
+              template: \`
+                @defer {
+                  <cmp-a />
+                  <cmp-b />
+                }
+              \`,
+            })
+            export class TestCmp {}
+          `);
+
+          env.driveMain();
+
+          const jsContents = env.getContents('test.js');
+
+          expect(jsContents).toContain('ɵɵdefer(1, 0, TestCmp_Defer_1_DepsFn)');
+          expect(jsContents)
+              .toContain(
+                  '() => [import("./cmps").then(m => m.CmpA), import("./cmps").then(m => m.CmpB)]');
+          expect(jsContents).not.toContain('import { CmpA }');
+          expect(jsContents).not.toContain('import { CmpB }');
+        });
       });
 
       it('should detect pipe used in the `when` trigger as an eager dependency', () => {
@@ -9327,7 +9551,7 @@ function allTests(os: string) {
            }));
            env.write(`test.ts`, `
             import {Component} from '@angular/core';
-    
+
             @Component({
               template: '...',
             })
@@ -9350,7 +9574,7 @@ function allTests(os: string) {
            }));
            env.write(`test.ts`, `
             import {Component} from '@angular/core';
-     
+
             @Component({
               standalone: true,
               template: '...',
@@ -9368,7 +9592,7 @@ function allTests(os: string) {
          () => {
            env.write(`test.ts`, `
           import {Component} from '@angular/core';
-   
+
           @Component({
             template: '...',
           })
