fix(core): prevent omission of deferred pipes in full compilation (angular#60571)

This prevents a bug where pipes would be excluded from defer dependency generation.

PR Close angular#60571

Author: thePunderWoman

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

@@ -1254,21 +1254,27 @@ export class ComponentDecoratorHandler
         }
       }
 
-      // Set up the R3TargetBinder, as well as a 'directives' array and a 'pipes' map that are
-      // later fed to the TemplateDefinitionBuilder.
+      // Set up the R3TargetBinder.
       const binder = createTargetBinder(dependencies);
-      const pipes = extractPipes(dependencies);
 
       let allDependencies = dependencies;
       let deferBlockBinder = binder;
 
       // If there are any explicitly deferred dependencies (via `@Component.deferredImports`),
-      // re-compute the list of dependencies and create a new binder for defer blocks.
+      // re-compute the list of dependencies and create a new binder for defer blocks. This
+      // is because we have deferred dependencies that are not in the standard imports list
+      // and need to be referenced later when determining what dependencies need to be in a
+      // defer function / instruction call. Otherwise they end up treated as a standard
+      // import, which is wrong.
       if (explicitlyDeferredDependencies.length > 0) {
         allDependencies = [...explicitlyDeferredDependencies, ...dependencies];
         deferBlockBinder = createTargetBinder(allDependencies);
       }
 
+      // Set up the pipes map that is later used to determine which dependencies are used in
+      // the template.
+      const pipes = extractPipes(allDependencies);
+
       // Next, the component template AST is bound using the R3TargetBinder. This produces a
       // BoundTarget, which is similar to a ts.TypeChecker.
       const bound = binder.bind({template: metadata.template.nodes});
@@ -1313,10 +1319,10 @@ export class ComponentDecoratorHandler
       // including all defer blocks.
       const wholeTemplateUsed = new Set<ClassDeclaration>(eagerlyUsed);
       for (const bound of deferBlocks.values()) {
-        for (const dir of bound.getEagerlyUsedDirectives()) {
+        for (const dir of bound.getUsedDirectives()) {
           wholeTemplateUsed.add(dir.ref.node);
         }
-        for (const name of bound.getEagerlyUsedPipes()) {
+        for (const name of bound.getUsedPipes()) {
           if (!pipes.has(name)) {
             continue;
           }

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

@@ -884,7 +884,6 @@ runInEachFileSystem(() => {
           'deferred-a.ts',
           `
           import {Component} from '@angular/core';
-
           @Component({
             standalone: true,
             selector: 'deferred-cmp-a',
@@ -899,7 +898,6 @@ runInEachFileSystem(() => {
           'deferred-b.ts',
           `
           import {Component} from '@angular/core';
-
           @Component({
             standalone: true,
             selector: 'deferred-cmp-b',
@@ -910,58 +908,81 @@ runInEachFileSystem(() => {
         `,
         );
 
+        env.write(
+          'pipe-a.ts',
+          `
+          import {Pipe} from '@angular/core';
+          @Pipe({
+            name: 'pipea',
+          })
+          export class PipeA {
+          }
+        `,
+        );
+
         env.write(
           'test.ts',
           `
           import {Component} from '@angular/core';
           import {DeferredCmpA} from './deferred-a';
           import {DeferredCmpB} from './deferred-b';
-
+          import {PipeA} from './pipe-a';
           @Component({
             standalone: true,
             // @ts-ignore
-            deferredImports: [DeferredCmpA, DeferredCmpB],
+            deferredImports: [DeferredCmpA, DeferredCmpB, PipeA],
             template: \`
-              @defer {
-                <deferred-cmp-a />
-              }
-              @defer {
-                <deferred-cmp-b />
+              @for (item of items; track item) {
+                @if (true) {
+                  @defer {
+                    {{ 'Hi!' | pipea }}
+                    <deferred-cmp-a />
+                  }
+                  @defer {
+                    <deferred-cmp-b />
+                  }
+                }
               }
             \`,
           })
           export class AppCmp {
+             items = [1,2,3];
           }
         `,
         );
 
         env.driveMain();
         const jsContents = env.getContents('test.js');
 
-        // Expect that all deferrableImports become dynamic imports.
+        // Expect that all deferrableImports in local compilation mode
+        // are located in a single function (since we can't detect in
+        // the local mode which components belong to which block).
         expect(jsContents).toContain(
-          'const AppCmp_Defer_1_DepsFn = () => [' +
-            'import("./deferred-a").then(m => m.DeferredCmpA)];',
+          'const AppCmp_For_1_Conditional_0_Defer_1_DepsFn = () => [' +
+            'import("./deferred-a").then(m => m.DeferredCmpA), ' +
+            'import("./pipe-a").then(m => m.PipeA)];',
         );
         expect(jsContents).toContain(
-          'const AppCmp_Defer_4_DepsFn = () => [' +
+          'const AppCmp_For_1_Conditional_0_Defer_4_DepsFn = () => [' +
             'import("./deferred-b").then(m => m.DeferredCmpB)];',
         );
 
         // Make sure there are no eager imports present in the output.
         expect(jsContents).not.toContain(`from './deferred-a'`);
         expect(jsContents).not.toContain(`from './deferred-b'`);
+        expect(jsContents).not.toContain(`from './pipe-a'`);
 
-        // Defer instructions have different dependency functions in full mode.
-        expect(jsContents).toContain('ɵɵdefer(1, 0, AppCmp_Defer_1_DepsFn);');
-        expect(jsContents).toContain('ɵɵdefer(4, 3, AppCmp_Defer_4_DepsFn);');
+        // There's 2 separate defer instructions due to the two separate defer blocks
+        expect(jsContents).toContain('ɵɵdefer(1, 0, AppCmp_For_1_Conditional_0_Defer_1_DepsFn);');
+        expect(jsContents).toContain('ɵɵdefer(4, 3, AppCmp_For_1_Conditional_0_Defer_4_DepsFn);');
 
         // Expect `ɵsetClassMetadataAsync` to contain dynamic imports too.
         expect(jsContents).toContain(
           'ɵsetClassMetadataAsync(AppCmp, () => [' +
             'import("./deferred-a").then(m => m.DeferredCmpA), ' +
+            'import("./pipe-a").then(m => m.PipeA), ' +
             'import("./deferred-b").then(m => m.DeferredCmpB)], ' +
-            '(DeferredCmpA, DeferredCmpB) => {',
+            '(DeferredCmpA, PipeA, DeferredCmpB) => {',
         );
       });