fix(compiler-cli): correctly detect deferred dependencies across scoped nodes (#54499)

This is based on an internal issue report.

An earlier change introduced a diagnostic to report cases where a symbol is in the `deferredImports` array, but is used eagerly. The check worked by looking through the deferred blocks in a scope, resolving the scope for each and checking if the element is within the scope. The problem is that resolving the scope won't work across scoped node boundaries. For example, if there's a control flow statement around the block or within the block but around the deferred dependency, it won't be able to resolve the scope since it isn't a direct child, e.g.

```
@if (true) {
  @defer {
   <deferred-dep/>
  }
}
```

To fix this the case where the deferred block is inside a scoped node, I've changed the `R3BoundTarget.deferBlocks` to be a `Map` holding both the deferred block and its corresponding scope. Then to resolve the case where the dependency is within a scoped node inside the deferred block, I've added a depth-first traversal through the scopes within the deferred block.

PR Close #54499

Author: crisbeto

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

@@ -10063,6 +10063,86 @@ function allTests(os: string) {
                expect(diags.length).toBe(1);
                expect(diags[0].code).toBe(ngErrorCode(ErrorCode.DEFERRED_PIPE_USED_EAGERLY));
              });
+
+          it('should not produce an error when a deferred block is wrapped in a conditional',
+             () => {
+               env.write('deferred-a.ts', `
+                  import {Component} from '@angular/core';
+                  @Component({
+                    standalone: true,
+                    selector: 'deferred-cmp-a',
+                    template: 'DeferredCmpA contents',
+                  })
+                  export class DeferredCmpA {
+                  }
+                `);
+
+               env.write('test.ts', `
+                 import {Component} from '@angular/core';
+                 import {DeferredCmpA} from './deferred-a';
+                 @Component({
+                   standalone: true,
+                   deferredImports: [DeferredCmpA],
+                   template: \`
+                     @if (true) {
+                       @if (true) {
+                        @if (true) {
+                          @defer {
+                            <deferred-cmp-a />
+                          }
+                        }
+                       }
+                     }
+                   \`,
+                 })
+                 export class AppCmp {
+                  condition = true;
+                 }
+               `);
+
+               const diags = env.driveDiagnostics();
+               expect(diags).toEqual([]);
+             });
+
+          it('should not produce an error when a dependency is wrapped in a condition inside of a deferred block',
+             () => {
+               env.write('deferred-a.ts', `
+                  import {Component} from '@angular/core';
+                  @Component({
+                    standalone: true,
+                    selector: 'deferred-cmp-a',
+                    template: 'DeferredCmpA contents',
+                  })
+                  export class DeferredCmpA {
+                  }
+                `);
+
+               env.write('test.ts', `
+                 import {Component} from '@angular/core';
+                 import {DeferredCmpA} from './deferred-a';
+                 @Component({
+                   standalone: true,
+                   deferredImports: [DeferredCmpA],
+                   template: \`
+                      @defer {
+                        @if (true) {
+                          @if (true) {
+                            @if (true) {
+                              <deferred-cmp-a />
+                            }
+                          }
+                        }
+                      }
+                   \`,
+                 })
+                 export class AppCmp {
+                  condition = true;
+                 }
+               `);
+
+               const diags = env.driveDiagnostics();
+               expect(diags).toEqual([]);
+             });
         });
       });
 

packages/compiler/src/render3/view/t2_binder.ts

@@ -51,7 +51,7 @@ export class R3TargetBinder<DirectiveT extends DirectiveMeta> implements TargetB
         TemplateBinder.applyWithScope(target.template, scope);
     return new R3BoundTarget(
         target, directives, eagerDirectives, bindings, references, expressions, symbols,
-        nestingLevel, scopedNodeEntities, usedPipes, eagerPipes, deferBlocks, scope);
+        nestingLevel, scopedNodeEntities, usedPipes, eagerPipes, deferBlocks);
   }
 }
 
@@ -472,7 +472,7 @@ class TemplateBinder extends RecursiveAstVisitor implements Visitor {
   private constructor(
       private bindings: Map<AST, Reference|Variable>,
       private symbols: Map<Reference|Variable, ScopedNode>, private usedPipes: Set<string>,
-      private eagerPipes: Set<string>, private deferBlocks: Set<DeferredBlock>,
+      private eagerPipes: Set<string>, private deferBlocks: Map<DeferredBlock, Scope>,
       private nestingLevel: Map<ScopedNode, number>, private scope: Scope,
       private rootNode: ScopedNode|null, private level: number) {
     super();
@@ -510,15 +510,15 @@ class TemplateBinder extends RecursiveAstVisitor implements Visitor {
     nestingLevel: Map<ScopedNode, number>,
     usedPipes: Set<string>,
     eagerPipes: Set<string>,
-    deferBlocks: Set<DeferredBlock>,
+    deferBlocks: Map<DeferredBlock, Scope>,
   } {
     const expressions = new Map<AST, Reference|Variable>();
     const symbols = new Map<Variable|Reference, Template>();
     const nestingLevel = new Map<ScopedNode, number>();
     const usedPipes = new Set<string>();
     const eagerPipes = new Set<string>();
     const template = nodes instanceof Template ? nodes : null;
-    const deferBlocks = new Set<DeferredBlock>();
+    const deferBlocks = new Map<DeferredBlock, Scope>();
     // The top-level template has nesting level 0.
     const binder = new TemplateBinder(
         expressions, symbols, usedPipes, eagerPipes, deferBlocks, nestingLevel, scope, template, 0);
@@ -547,9 +547,17 @@ class TemplateBinder extends RecursiveAstVisitor implements Visitor {
       nodeOrNodes.trackBy.visit(this);
       nodeOrNodes.children.forEach(this.visitNode);
       this.nestingLevel.set(nodeOrNodes, this.level);
+    } else if (nodeOrNodes instanceof DeferredBlock) {
+      if (this.scope.rootNode !== nodeOrNodes) {
+        throw new Error(
+            `Assertion error: resolved incorrect scope for deferred block ${nodeOrNodes}`);
+      }
+      this.deferBlocks.set(nodeOrNodes, this.scope);
+      nodeOrNodes.children.forEach(node => node.visit(this));
+      this.nestingLevel.set(nodeOrNodes, this.level);
     } else if (
         nodeOrNodes instanceof SwitchBlockCase || nodeOrNodes instanceof ForLoopBlockEmpty ||
-        nodeOrNodes instanceof DeferredBlock || nodeOrNodes instanceof DeferredBlockError ||
+        nodeOrNodes instanceof DeferredBlockError ||
         nodeOrNodes instanceof DeferredBlockPlaceholder ||
         nodeOrNodes instanceof DeferredBlockLoading) {
       nodeOrNodes.children.forEach(node => node.visit(this));
@@ -616,9 +624,7 @@ class TemplateBinder extends RecursiveAstVisitor implements Visitor {
   }
 
   visitDeferredBlock(deferred: DeferredBlock) {
-    this.deferBlocks.add(deferred);
     this.ingestScopedNode(deferred);
-
     deferred.triggers.when?.value.visit(this);
     deferred.prefetchTriggers.when?.value.visit(this);
     deferred.placeholder && this.visitNode(deferred.placeholder);
@@ -738,7 +744,7 @@ export class R3BoundTarget<DirectiveT extends DirectiveMeta> implements BoundTar
       private nestingLevel: Map<ScopedNode, number>,
       private scopedNodeEntities: Map<ScopedNode|null, ReadonlySet<Reference|Variable>>,
       private usedPipes: Set<string>, private eagerPipes: Set<string>,
-      private deferredBlocks: Set<DeferredBlock>, private rootScope: Scope) {}
+      private deferBlocks: Map<DeferredBlock, Scope>) {}
 
   getEntitiesInScope(node: ScopedNode|null): ReadonlySet<Reference|Variable> {
     return this.scopedNodeEntities.get(node) ?? new Set();
@@ -789,7 +795,7 @@ export class R3BoundTarget<DirectiveT extends DirectiveMeta> implements BoundTar
   }
 
   getDeferBlocks(): DeferredBlock[] {
-    return Array.from(this.deferredBlocks);
+    return Array.from(this.deferBlocks.keys());
   }
 
   getDeferredTriggerTarget(block: DeferredBlock, trigger: DeferredTrigger): Element|null {
@@ -856,12 +862,20 @@ export class R3BoundTarget<DirectiveT extends DirectiveMeta> implements BoundTar
   }
 
   isDeferred(element: Element): boolean {
-    for (const deferBlock of this.deferredBlocks) {
-      const scope = this.rootScope.childScopes.get(deferBlock);
-      if (scope && scope.elementsInScope.has(element)) {
-        return true;
+    for (const deferredScope of this.deferBlocks.values()) {
+      const stack = [deferredScope];
+
+      while (stack.length > 0) {
+        const current = stack.pop()!;
+
+        if (current.elementsInScope.has(element)) {
+          return true;
+        }
+
+        stack.push(...current.childScopes.values());
       }
     }
+
     return false;
   }