fix(ivy): queries should match elements inside ng-container with the …

…descendants: false option (#35384)

Before this change content queries with the `descendants: false` option, as implemented in ivy,
would not descendinto `<ng-container>` elements. This behaviour was different from the way the
View Engine worked. This change alligns ngIvy and VE behaviours when it comes to queries and the
`<ng-container>` elements and fixes a common bugs where a query target was placed inside the
`<ng-container>` element with a * directive on it.

Before:

```html
<needs-target>
  <ng-container *ngIf="condition">
    <div #target>...</div>  <!-- this node would NOT match -->
  </ng-container>
</needs-target>
```

After:

```html
<needs-target>
  <ng-container *ngIf="condition">
    <div #target>...</div>  <!-- this node WILL match -->
  </ng-container>
</needs-target>
```

Fixes #34768

PR Close #35384

aio/scripts/_payload-limits.json

@@ -26,4 +26,4 @@
       }
     }
   }
-}
+}

integration/_payload-limits.json

@@ -64,4 +64,4 @@
       }
     }
   }
-}
+}

packages/core/src/render3/query.ts

@@ -188,7 +188,23 @@ class TQuery_ implements TQuery {
 
   private isApplyingToNode(tNode: TNode): boolean {
     if (this._appliesToNextNode && this.metadata.descendants === false) {
-      return this._declarationNodeIndex === (tNode.parent ? tNode.parent.index : -1);
+      const declarationNodeIdx = this._declarationNodeIndex;
+      let parent = tNode.parent;
+      // Determine if a given TNode is a "direct" child of a node on which a content query was
+      // declared (only direct children of query's host node can match with the descendants: false
+      // option). There are 3 main use-case / conditions to consider here:
+      // - <needs-target><i #target></i></needs-target>: here <i #target> parent node is a query
+      // host node;
+      // - <needs-target><ng-template [ngIf]="true"><i #target></i></ng-template></needs-target>:
+      // here <i #target> parent node is null;
+      // - <needs-target><ng-container><i #target></i></ng-container></needs-target>: here we need
+      // to go past `<ng-container>` to determine <i #target> parent node (but we shouldn't traverse
+      // up past the query's host node!).
+      while (parent !== null && parent.type === TNodeType.ElementContainer &&
+             parent.index !== declarationNodeIdx) {
+        parent = parent.parent;
+      }
+      return declarationNodeIdx === (parent !== null ? parent.index : -1);
     }
     return this._appliesToNextNode;
   }

packages/core/test/acceptance/query_spec.ts

@@ -690,7 +690,14 @@ describe('query logic', () => {
     expect(fixture.componentInstance.contentChildren.length).toBe(0);
   });
 
-  describe('descendants', () => {
+  describe('descendants: false (default)', () => {
+
+    /**
+     * A helper function to check if a given object looks like ElementRef. It is used in place of
+     * the `instanceof ElementRef` check since ivy returns a type that looks like ElementRef (have
+     * the same properties but doesn't pass the instanceof ElementRef test)
+     */
+    function isElementRefLike(result: any): boolean { return result.nativeElement != null; }
 
     it('should match directives on elements that used to be wrapped by a required parent in HTML parser',
        () => {
@@ -715,8 +722,246 @@ describe('query logic', () => {
          fixture.detectChanges();
          expect(cmptWithQuery.myDefs.length).toBe(1);
        });
+
+    it('should match elements with local refs inside <ng-container>', () => {
+
+      @Component({selector: 'needs-target', template: ``})
+      class NeedsTarget {
+        @ContentChildren('target') targets !: QueryList<ElementRef>;
+      }
+      @Component({
+        selector: 'test-cmpt',
+        template: `
+          <needs-target>
+            <ng-container>
+              <tr #target></tr>
+            </ng-container>
+          </needs-target>
+        `,
+      })
+      class TestCmpt {
+      }
+
+      TestBed.configureTestingModule({declarations: [TestCmpt, NeedsTarget]});
+      const fixture = TestBed.createComponent(TestCmpt);
+      const cmptWithQuery = fixture.debugElement.children[0].injector.get(NeedsTarget);
+
+      fixture.detectChanges();
+      expect(cmptWithQuery.targets.length).toBe(1);
+      expect(isElementRefLike(cmptWithQuery.targets.first)).toBeTruthy();
+    });
+
+    it('should match elements with local refs inside nested <ng-container>', () => {
+
+      @Component({selector: 'needs-target', template: ``})
+      class NeedsTarget {
+        @ContentChildren('target') targets !: QueryList<ElementRef>;
+      }
+
+      @Component({
+        selector: 'test-cmpt',
+        template: `
+          <needs-target>
+            <ng-container>
+              <ng-container>
+                <ng-container>
+                  <tr #target></tr>
+                </ng-container>  
+              </ng-container>
+            </ng-container>
+          </needs-target>
+        `,
+      })
+      class TestCmpt {
+      }
+
+      TestBed.configureTestingModule({declarations: [TestCmpt, NeedsTarget]});
+      const fixture = TestBed.createComponent(TestCmpt);
+      const cmptWithQuery = fixture.debugElement.children[0].injector.get(NeedsTarget);
+
+      fixture.detectChanges();
+      expect(cmptWithQuery.targets.length).toBe(1);
+      expect(isElementRefLike(cmptWithQuery.targets.first)).toBeTruthy();
+    });
+
+    it('should match directives inside <ng-container>', () => {
+      @Directive({selector: '[targetDir]'})
+      class TargetDir {
+      }
+
+      @Component({selector: 'needs-target', template: ``})
+      class NeedsTarget {
+        @ContentChildren(TargetDir) targets !: QueryList<HTMLElement>;
+      }
+
+      @Component({
+        selector: 'test-cmpt',
+        template: `
+          <needs-target>
+            <ng-container>
+              <tr targetDir></tr>
+            </ng-container>
+          </needs-target>
+        `,
+      })
+      class TestCmpt {
+      }
+
+      TestBed.configureTestingModule({declarations: [TestCmpt, NeedsTarget, TargetDir]});
+      const fixture = TestBed.createComponent(TestCmpt);
+      const cmptWithQuery = fixture.debugElement.children[0].injector.get(NeedsTarget);
+
+      fixture.detectChanges();
+      expect(cmptWithQuery.targets.length).toBe(1);
+      expect(cmptWithQuery.targets.first).toBeAnInstanceOf(TargetDir);
+    });
+
+    it('should match directives inside nested <ng-container>', () => {
+      @Directive({selector: '[targetDir]'})
+      class TargetDir {
+      }
+
+      @Component({selector: 'needs-target', template: ``})
+      class NeedsTarget {
+        @ContentChildren(TargetDir) targets !: QueryList<HTMLElement>;
+      }
+
+      @Component({
+        selector: 'test-cmpt',
+        template: `
+          <needs-target>
+            <ng-container>
+              <ng-container>
+                <ng-container>
+                  <tr targetDir></tr>
+                </ng-container>
+              </ng-container>
+            </ng-container>
+          </needs-target>
+        `,
+      })
+      class TestCmpt {
+      }
+
+      TestBed.configureTestingModule({declarations: [TestCmpt, NeedsTarget, TargetDir]});
+      const fixture = TestBed.createComponent(TestCmpt);
+      const cmptWithQuery = fixture.debugElement.children[0].injector.get(NeedsTarget);
+
+      fixture.detectChanges();
+      expect(cmptWithQuery.targets.length).toBe(1);
+      expect(cmptWithQuery.targets.first).toBeAnInstanceOf(TargetDir);
+    });
+
+    it('should cross child ng-container when query is declared on ng-container', () => {
+      @Directive({selector: '[targetDir]'})
+      class TargetDir {
+      }
+
+      @Directive({selector: '[needs-target]'})
+      class NeedsTarget {
+        @ContentChildren(TargetDir) targets !: QueryList<HTMLElement>;
+      }
+
+      @Component({
+        selector: 'test-cmpt',
+        template: `
+          <ng-container targetDir>
+            <ng-container needs-target>
+              <ng-container>
+                <tr targetDir></tr>
+              </ng-container>
+            </ng-container>
+          </ng-container>
+        `,
+      })
+      class TestCmpt {
+      }
+
+      TestBed.configureTestingModule({declarations: [TestCmpt, NeedsTarget, TargetDir]});
+      const fixture = TestBed.createComponent(TestCmpt);
+      const cmptWithQuery = fixture.debugElement.children[0].injector.get(NeedsTarget);
+
+      fixture.detectChanges();
+      expect(cmptWithQuery.targets.length).toBe(1);
+      expect(cmptWithQuery.targets.first).toBeAnInstanceOf(TargetDir);
+    });
+
+    it('should match nodes when using structural directives (*syntax) on <ng-container>', () => {
+      @Directive({selector: '[targetDir]'})
+      class TargetDir {
+      }
+
+      @Component({selector: 'needs-target', template: ``})
+      class NeedsTarget {
+        @ContentChildren(TargetDir) dirTargets !: QueryList<TargetDir>;
+        @ContentChildren('target') localRefsTargets !: QueryList<ElementRef>;
+      }
+
+      @Component({
+        selector: 'test-cmpt',
+        template: `
+          <needs-target>
+            <ng-container *ngIf="true">
+              <div targetDir></div>
+              <div #target></div>
+            </ng-container>
+          </needs-target>
+        `,
+      })
+      class TestCmpt {
+      }
+
+      TestBed.configureTestingModule({declarations: [TestCmpt, NeedsTarget, TargetDir]});
+      const fixture = TestBed.createComponent(TestCmpt);
+      const cmptWithQuery = fixture.debugElement.children[0].injector.get(NeedsTarget);
+
+      fixture.detectChanges();
+      expect(cmptWithQuery.dirTargets.length).toBe(1);
+      expect(cmptWithQuery.dirTargets.first).toBeAnInstanceOf(TargetDir);
+      expect(cmptWithQuery.localRefsTargets.length).toBe(1);
+      expect(isElementRefLike(cmptWithQuery.localRefsTargets.first)).toBeTruthy();
+    });
+
+    onlyInIvy(
+        'VE uses injectors hierarchy to determine if node matches, ivy uses elements as written in a template')
+        .it('should match directives on <ng-container> when crossing nested <ng-container>', () => {
+          @Directive({selector: '[targetDir]'})
+          class TargetDir {
+          }
+
+          @Component({selector: 'needs-target', template: ``})
+          class NeedsTarget {
+            @ContentChildren(TargetDir) targets !: QueryList<HTMLElement>;
+          }
+
+          @Component({
+            selector: 'test-cmpt',
+            template: `
+          <needs-target>
+            <ng-container>
+              <ng-container targetDir>
+                <ng-container targetDir>
+                  <tr targetDir></tr>
+                </ng-container>
+              </ng-container>
+            </ng-container>
+          </needs-target>
+        `,
+          })
+          class TestCmpt {
+          }
+
+          TestBed.configureTestingModule({declarations: [TestCmpt, NeedsTarget, TargetDir]});
+          const fixture = TestBed.createComponent(TestCmpt);
+          const cmptWithQuery = fixture.debugElement.children[0].injector.get(NeedsTarget);
+
+          fixture.detectChanges();
+          expect(cmptWithQuery.targets.length).toBe(3);
+        });
   });
 
+
+
   describe('observable interface', () => {
 
     it('should allow observing changes to query list', () => {

packages/core/test/linker/query_integration_spec.ts

@@ -361,19 +361,18 @@ describe('Query API', () => {
          expect(comp.children.length).toBe(1);
        });
 
-    onlyInIvy(
-        'Shallow queries don\'t cross ng-container boundaries in ivy (ng-container is treated as a regular element')
-        .it('should not cross ng-container boundaries with shallow queries', () => {
-          const template = `<needs-content-children-shallow>
+
+    it('should not cross ng-container boundaries with shallow queries', () => {
+      const template = `<needs-content-children-shallow>
                           <ng-container>
                             <div #q></div>
                           </ng-container>
                         </needs-content-children-shallow>`;
-          const view = createTestCmpAndDetectChanges(MyComp0, template);
+      const view = createTestCmpAndDetectChanges(MyComp0, template);
 
-          const comp = view.debugElement.children[0].injector.get(NeedsContentChildrenShallow);
-          expect(comp.children.length).toBe(0);
-        });
+      const comp = view.debugElement.children[0].injector.get(NeedsContentChildrenShallow);
+      expect(comp.children.length).toBe(1);
+    });
 
     it('should contain the first descendant content child templateRef', () => {
       const template = '<needs-content-child-template-ref-app>' +