refactor(compiler): drop regular imports when symbols can be defer-lo…

…aded

This commit updates the logic to drop regular imports when all symbols that it brings can be defer-loaded.
The change ensures that there is no mix of regular and dynamic imports present in a source file.

packages/compiler-cli/src/ngtsc/annotations/common/src/util.ts

@@ -331,7 +331,8 @@ export function createSourceSpan(node: ts.Node): ParseSourceSpan {
  */
 export function compileResults(
     fac: CompileResult, def: R3CompiledExpression, metadataStmt: Statement|null, propName: string,
-    additionalFields: CompileResult[]|null): CompileResult[] {
+    additionalFields: CompileResult[]|null,
+    deferrableImports?: Set<ts.ImportDeclaration>): CompileResult[] {
   const statements = def.statements;
   if (metadataStmt !== null) {
     statements.push(metadataStmt);
@@ -344,6 +345,7 @@ export function compileResults(
       initializer: def.expression,
       statements: def.statements,
       type: def.type,
+      deferrableImports,
     },
   ];
 

packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts

@@ -1006,7 +1006,8 @@ export class ComponentDecoratorHandler implements
     const classMetadata = analysis.classMetadata !== null ?
         compileClassMetadata(analysis.classMetadata).toStmt() :
         null;
-    return compileResults(fac, def, classMetadata, 'ɵcmp', inputTransformFields);
+    const deferrableImports = this.deferredSymbolTracker.getDeferrableImportDecls();
+    return compileResults(fac, def, classMetadata, 'ɵcmp', inputTransformFields, deferrableImports);
   }
 
   compilePartial(

packages/compiler-cli/src/ngtsc/transform/src/api.ts

@@ -248,6 +248,7 @@ export interface CompileResult {
   initializer: Expression|null;
   statements: Statement[];
   type: Type;
+  deferrableImports?: Set<ts.ImportDeclaration>;
 }
 
 export interface ResolveResult<R> {

packages/compiler-cli/src/ngtsc/transform/src/transform.ts

@@ -56,6 +56,7 @@ export function ivyTransformFactory(
  */
 class IvyCompilationVisitor extends Visitor {
   public classCompilationMap = new Map<ts.ClassDeclaration, CompileResult[]>();
+  public deferrableImports = new Set<ts.ImportDeclaration>();
 
   constructor(private compilation: TraitCompiler, private constantPool: ConstantPool) {
     super();
@@ -68,6 +69,16 @@ class IvyCompilationVisitor extends Visitor {
     const result = this.compilation.compile(node, this.constantPool);
     if (result !== null) {
       this.classCompilationMap.set(node, result);
+
+      // Collect all deferrable imports declarations into a single set,
+      // so that we can pass it to the transform visitor that will drop
+      // corresponding regular import declarations.
+      for (const classResult of result) {
+        if (classResult.deferrableImports && classResult.deferrableImports.size > 0) {
+          classResult.deferrableImports.forEach(
+              importDecl => this.deferrableImports.add(importDecl));
+        }
+      }
     }
     return {node};
   }
@@ -83,7 +94,8 @@ class IvyTransformationVisitor extends Visitor {
       private classCompilationMap: Map<ts.ClassDeclaration, CompileResult[]>,
       private reflector: ReflectionHost, private importManager: ImportManager,
       private recordWrappedNodeExpr: RecordWrappedNodeFn<ts.Expression>,
-      private isClosureCompilerEnabled: boolean, private isCore: boolean) {
+      private isClosureCompilerEnabled: boolean, private isCore: boolean,
+      private deferrableImports: Set<ts.ImportDeclaration>) {
     super();
   }
 
@@ -153,6 +165,16 @@ class IvyTransformationVisitor extends Visitor {
     return {node, after: statements};
   }
 
+  override visitOtherNode<T extends ts.Node>(node: T): T {
+    if (ts.isImportDeclaration(node) && this.deferrableImports.has(node)) {
+      // Return `null` as an indication that this node should not be present
+      // in the final AST. Symbols from this import would be imported via
+      // dynamic imports.
+      return null!;
+    }
+    return node;
+  }
+
   /**
    * Return all decorators on a `Declaration` which are from @angular/core, or an empty set if none
    * are.
@@ -281,7 +303,7 @@ function transformIvySourceFile(
   // results obtained at Step 1.
   const transformationVisitor = new IvyTransformationVisitor(
       compilation, compilationVisitor.classCompilationMap, reflector, importManager,
-      recordWrappedNode, isClosureCompilerEnabled, isCore);
+      recordWrappedNode, isClosureCompilerEnabled, isCore, compilationVisitor.deferrableImports);
   let sf = visit(file, transformationVisitor, context);
 
   // Generate the constant statements first, as they may involve adding additional imports

packages/compiler-cli/src/ngtsc/util/src/visitor.ts

@@ -78,7 +78,7 @@ export abstract class Visitor {
     // is completed.
     let visitedNode: T|null = null;
 
-    node = ts.visitEachChild(node, child => this._visit(child, context), context) as T;
+    node = ts.visitEachChild(node, child => child && this._visit(child, context), context) as T;
 
     if (ts.isClassDeclaration(node)) {
       visitedNode =
@@ -90,7 +90,7 @@ export abstract class Visitor {
 
     // If the visited node has a `statements` array then process them, maybe replacing the visited
     // node and adding additional statements.
-    if (ts.isBlock(visitedNode) || ts.isSourceFile(visitedNode)) {
+    if (visitedNode && (ts.isBlock(visitedNode) || ts.isSourceFile(visitedNode))) {
       visitedNode = this._maybeProcessStatements(visitedNode);
     }
 

packages/compiler-cli/test/ngtsc/ngtsc_spec.ts

@@ -8873,10 +8873,10 @@ function allTests(os: string) {
     });
 
     describe('deferred blocks', () => {
-      it('should not error for deferred blocks', () => {
+      it('should handle deferred blocks', () => {
         env.tsconfig({_enabledBlockTypes: ['defer']});
         env.write('cmp-a.ts', `
-          import {Component} from '@angular/core';
+          import { Component } from '@angular/core';
 
           @Component({
             standalone: true,
@@ -8887,8 +8887,8 @@ function allTests(os: string) {
         `);
 
         env.write('/test.ts', `
-          import {Component} from '@angular/core';
-          import {CmpA} from './cmp-a';
+          import { Component } from '@angular/core';
+          import { CmpA } from './cmp-a';
 
           @Component({
             selector: 'local-dep',
@@ -8914,10 +8914,177 @@ function allTests(os: string) {
         env.driveMain();
 
         const jsContents = env.getContents('test.js');
+
         expect(jsContents).toContain('ɵɵdefer(0, TestCmp_Defer_0_DepsFn)');
         expect(jsContents)
             .toContain(
                 'function TestCmp_Defer_0_DepsFn() { return [import("./cmp-a").then(function (m) { return m.CmpA; }), LocalDep]; }');
+
+        // The `CmpA` symbol wasn't referenced elsewhere, so it can be defer-loaded
+        // via dynamic imports and an original import can be removed.
+        expect(jsContents).not.toContain('import { CmpA }');
+      });
+
+      describe('imports', () => {
+        it('should retain regular imports when symbol is eagerly referenced', () => {
+          env.tsconfig({_enabledBlockTypes: ['defer']});
+          env.write('cmp-a.ts', `
+            import { Component } from '@angular/core';
+
+            @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';
+
+            @Component({
+              selector: 'test-cmp',
+              standalone: true,
+              imports: [CmpA],
+              template: \`
+                {#defer}
+                  <cmp-a />
+                {/defer}
+              \`,
+            })
+            export class TestCmp {
+              constructor() {
+                // This line retains the regular import of CmpA,
+                // since it's eagerly referenced in the code.
+                console.log(CmpA);
+              }
+            }
+          `);
+
+          env.driveMain();
+
+          const jsContents = env.getContents('test.js');
+
+          expect(jsContents).toContain('ɵɵdefer(0, TestCmp_Defer_0_DepsFn)');
+
+          // The dependency function doesn't have a dynamic import, because `CmpA`
+          // was eagerly referenced in component's code, thus regular import can not be removed.
+          expect(jsContents).toContain('function TestCmp_Defer_0_DepsFn() { return [CmpA]; }');
+          expect(jsContents).toContain('import { CmpA }');
+        });
+
+        it('should retain regular imports when one of the symbols is eagerly referenced', () => {
+          env.tsconfig({_enabledBlockTypes: ['defer']});
+          env.write('cmp-a.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, CmpB } from './cmp-a';
+
+            @Component({
+              selector: 'test-cmp',
+              standalone: true,
+              imports: [CmpA, CmpB],
+              template: \`
+                {#defer}
+                  <cmp-a />
+                  <cmp-b />
+                {/defer}
+              \`,
+            })
+            export class TestCmp {
+              constructor() {
+                // This line retains the regular import of CmpA,
+                // since it's eagerly referenced in the code.
+                console.log(CmpA);
+              }
+            }
+          `);
+
+          env.driveMain();
+
+          const jsContents = env.getContents('test.js');
+
+          expect(jsContents).toContain('ɵɵdefer(0, TestCmp_Defer_0_DepsFn)');
+
+          // The dependency function doesn't have a dynamic import, because `CmpA`
+          // was eagerly referenced in component's code, thus regular import can not be removed.
+          // This also affects `CmpB`, since it was extracted from the same import.
+          expect(jsContents)
+              .toContain('function TestCmp_Defer_0_DepsFn() { return [CmpA, CmpB]; }');
+          expect(jsContents).toContain('import { CmpA, CmpB }');
+        });
+
+        it('should drop regular imports when none of the symbols are eagerly referenced', () => {
+          env.tsconfig({_enabledBlockTypes: ['defer']});
+          env.write('cmp-a.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, CmpB } from './cmp-a';
+
+            @Component({
+              selector: 'test-cmp',
+              standalone: true,
+              imports: [CmpA, CmpB],
+              template: \`
+                {#defer}
+                  <cmp-a />
+                  <cmp-b />
+                {/defer}
+              \`,
+            })
+            export class TestCmp {}
+          `);
+
+          env.driveMain();
+
+          const jsContents = env.getContents('test.js');
+
+          expect(jsContents).toContain('ɵɵdefer(0, TestCmp_Defer_0_DepsFn)');
+
+          // Both `CmpA` and `CmpB` were used inside the `{#defer}` and were not
+          // referenced elsewhere, so we generate dynamic imports and drop a regular one.
+          expect(jsContents)
+              .toContain(
+                  'function TestCmp_Defer_0_DepsFn() { return [' +
+                  'import("./cmp-a").then(function (m) { return m.CmpA; }), ' +
+                  'import("./cmp-a").then(function (m) { return m.CmpB; })]; }');
+          expect(jsContents).not.toContain('import { CmpA, CmpB }');
+        });
       });
     });
   });