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

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,9 +361,8 @@ 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', () => {
-        .it('should not cross ng-container boundaries with shallow queries', () => {
       const template = `<needs-content-children-shallow>
                           <ng-container>
                             <div #q></div>
@@ -372,7 +371,7 @@ describe('Query API', () => {
       const view = createTestCmpAndDetectChanges(MyComp0, template);
 
       const comp = view.debugElement.children[0].injector.get(NeedsContentChildrenShallow);
-          expect(comp.children.length).toBe(0);
+      expect(comp.children.length).toBe(1);
     });
 
     it('should contain the first descendant content child templateRef', () => {

packages/core/test/render3/query_spec.ts

@@ -569,75 +569,6 @@ describe('query', () => {
         expect(qList.first.nativeElement).toEqual(elToQuery);
       });
 
-      /**
-       * BREAKING CHANGE: this tests asserts different behavior as compared to Renderer2 when it
-       * comes to descendants: false option and <ng-container>.
-       *
-       * Previous behavior: queries with descendants: false would descend into <ng-container>.
-       * New behavior: queries with descendants: false would NOT descend into <ng-container>.
-       *
-       * Reasoning: the Renderer2 behavior is inconsistent and hard to explain to users when it
-       * comes to descendants: false interpretation (see
-       * https://github.com/angular/angular/issues/14769#issuecomment-356609267) so we are changing
-       * it in ngIvy.
-       *
-       * In ngIvy implementation queries with the descendants: false option are interpreted as
-       * "don't descend" into children of a given element when looking for matches. In other words
-       * only direct children of a given component / directive are checked for matches. This applies
-       * to both regular elements (ex. <div>) and grouping elements (<ng-container>,
-       * <ng-template>)).
-       *
-       * Grouping elements (<ng-container>, <ng-template>) are treated as regular elements since we
-       * can query for <ng-container> and <ng-template>, so they behave like regular elements from
-       * this point of view.
-       */
-      it('should not descend into <ng-container> when descendants: false', () => {
-        let elToQuery;
-
-        /**
-          * <ng-container>
-          *    <div #foo></div>
-          * </ng-container>
-          * class Cmpt {
-          *  @ViewChildren('foo') deep;
-          *  @ViewChildren('foo', {descendants: false}) shallow;
-          * }
-          */
-        const Cmpt = createComponent(
-            'cmpt',
-            function(rf: RenderFlags, ctx: any) {
-              if (rf & RenderFlags.Create) {
-                ɵɵelementContainerStart(0);
-                {
-                  ɵɵelement(1, 'div', null, 0);
-                  elToQuery = getNativeByIndex(3, getLView());
-                }
-                ɵɵelementContainerEnd();
-              }
-            },
-            3, 0, [], [],
-            function(rf: RenderFlags, ctx: any) {
-              if (rf & RenderFlags.Create) {
-                ɵɵviewQuery(['foo'], true, ElementRef);
-                ɵɵviewQuery(['foo'], false, ElementRef);
-              }
-              if (rf & RenderFlags.Update) {
-                let tmp: any;
-                ɵɵqueryRefresh(tmp = ɵɵloadQuery<QueryList<any>>()) &&
-                    (ctx.deep = tmp as QueryList<any>);
-                ɵɵqueryRefresh(tmp = ɵɵloadQuery<QueryList<any>>()) &&
-                    (ctx.shallow = tmp as QueryList<any>);
-              }
-            },
-            [], [], undefined, [['foo', '']]);
-
-        const fixture = new ComponentFixture(Cmpt);
-        const deepQList = fixture.component.deep;
-        const shallowQList = fixture.component.shallow;
-        expect(deepQList.length).toBe(1);
-        expect(shallowQList.length).toBe(0);
-      });
-
       it('should read ViewContainerRef from element nodes when explicitly asked for', () => {
         /**
          * <div #foo></div>