fix(ivy): ensure views created in constructors dont break queries (#29983)

Previous to this change, we assumed embedded views could only be created after
their parent template node had completed processing. As a result, we only set
up query logic for containers after directives on the node were created.
However, this assumption didn't take into account the case where a directive
on a template node could create views in its constructor.

This commit fixes query logic to work with views created in constructors.
In that case, we need to create a query container before the new view is
rendered so query results in the view aren't lost. But since the query container
is created before directives have completed processing, we also have to ensure
that query results gathered later on the template node are inserted before that
query container. Otherwise, query results in embedded views will clobber query
results on template nodes.

This splice mode may be slightly slower than the normal matching for queries on
containers, but we should only fall back to this strategy in the edge case where
views are created in constructors. (We should encourage developers to create
views in ngOnInit instead).

PR Close #29983

packages/core/src/render3/instructions/container.ts

@@ -144,9 +144,17 @@ export function ɵɵcontainerRefreshEnd(): void {
 function addTContainerToQueries(lView: LView, tContainerNode: TContainerNode): void {
   const queries = lView[QUERIES];
   if (queries) {
-    queries.addNode(tContainerNode);
     const lContainer = lView[tContainerNode.index];
-    lContainer[QUERIES] = queries.container();
+    if (lContainer[QUERIES]) {
+      // Query container should only exist if it was created through a dynamic view
+      // in a directive constructor. In this case, we must splice the template
+      // matches in before the view matches to ensure query results in embedded views
+      // don't clobber query results on the template node itself.
+      queries.insertNodeBeforeViews(tContainerNode);
+    } else {
+      queries.addNode(tContainerNode);
+      lContainer[QUERIES] = queries.container();
+    }
   }
 }
 

packages/core/src/render3/interfaces/query.ts

@@ -37,6 +37,13 @@ export interface LQueries {
    */
   addNode(tNode: TElementNode|TContainerNode|TElementContainerNode): void;
 
+  /**
+   * Notify `LQueries` that a new `TNode` has been created and needs to be added to query results
+   * if matching query predicate. This is a special mode invoked if the query container has to
+   * be created out of order (e.g. view created in the constructor of a directive).
+   */
+  insertNodeBeforeViews(tNode: TElementNode|TContainerNode|TElementContainerNode): void;
+
   /**
    * Notify `LQueries` that a new LContainer was added to ivy data structures. As a result we need
    * to prepare room for views that might be inserted into this container.

packages/core/src/render3/query.ts

@@ -123,8 +123,13 @@ export class LQueries_ implements LQueries {
   }
 
   addNode(tNode: TElementNode|TContainerNode|TElementContainerNode): void {
-    add(this.deep, tNode);
-    add(this.shallow, tNode);
+    add(this.deep, tNode, false);
+    add(this.shallow, tNode, false);
+  }
+
+  insertNodeBeforeViews(tNode: TElementNode|TContainerNode|TElementContainerNode): void {
+    add(this.deep, tNode, true);
+    add(this.shallow, tNode, true);
   }
 
   removeView(): void {
@@ -277,8 +282,18 @@ function queryRead(tNode: TNode, currentView: LView, read: any, matchingIdx: num
   return queryByTNodeType(tNode, currentView);
 }
 
+/**
+ * Add query matches for a given node.
+ *
+ * @param query The first query in the linked list
+ * @param tNode The TNode to match against queries
+ * @param insertBeforeContainer Whether or not we should add matches before the last
+ * container array. This mode is necessary if the query container had to be created
+ * out of order (e.g. a view was created in a constructor)
+ */
 function add(
-    query: LQuery<any>| null, tNode: TElementNode | TContainerNode | TElementContainerNode) {
+    query: LQuery<any>| null, tNode: TElementNode | TContainerNode | TElementContainerNode,
+    insertBeforeContainer: boolean) {
   const currentView = getLView();
 
   while (query) {
@@ -295,7 +310,7 @@ function add(
         }
       }
       if (result !== null) {
-        addMatch(query, result);
+        addMatch(query, result, insertBeforeContainer);
       }
     } else {
       const selector = predicate.selector !;
@@ -304,7 +319,7 @@ function add(
         if (matchingIdx !== null) {
           const result = queryRead(tNode, currentView, predicate.read, matchingIdx);
           if (result !== null) {
-            addMatch(query, result);
+            addMatch(query, result, insertBeforeContainer);
           }
         }
       }
@@ -313,8 +328,12 @@ function add(
   }
 }
 
-function addMatch(query: LQuery<any>, matchingValue: any): void {
-  query.values.push(matchingValue);
+function addMatch(query: LQuery<any>, matchingValue: any, insertBeforeViewMatches: boolean): void {
+  // Views created in constructors may have their container values created too early. In this case,
+  // ensure template node results are spliced before container results. Otherwise, results inside
+  // embedded views will appear before results on parent template nodes when flattened.
+  insertBeforeViewMatches ? query.values.splice(-1, 0, matchingValue) :
+                            query.values.push(matchingValue);
   query.list.setDirty();
 }
 

packages/core/src/render3/view_engine_compatibility.ts

@@ -106,6 +106,12 @@ export function createTemplateRef<T>(
 
       createEmbeddedView(context: T, container?: LContainer, index?: number):
           viewEngine_EmbeddedViewRef<T> {
+        const currentQueries = this._declarationParentView[QUERIES];
+        // Query container may be missing if this view was created in a directive
+        // constructor. Create it now to avoid losing results in embedded views.
+        if (currentQueries && this._hostLContainer[QUERIES] == null) {
+          this._hostLContainer[QUERIES] = currentQueries !.container();
+        }
         const lView = createEmbeddedViewAndNode(
             this._tView, context, this._declarationParentView, this._hostLContainer[QUERIES],
             this._injectorIndex);

packages/core/test/acceptance/view_container_ref_spec.ts

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {Component, Directive, NO_ERRORS_SCHEMA, QueryList, TemplateRef, ViewChild, ViewChildren, ViewContainerRef, ɵi18nConfigureLocalize} from '@angular/core';
+import {Component, Directive, ElementRef, NO_ERRORS_SCHEMA, QueryList, TemplateRef, ViewChild, ViewChildren, ViewContainerRef, ɵi18nConfigureLocalize} from '@angular/core';
 import {TestBed} from '@angular/core/testing';
 import {expect} from '@angular/platform-browser/testing/src/matchers';
 import {ivyEnabled, onlyInIvy} from '@angular/private/testing';
@@ -23,8 +23,31 @@ describe('ViewContainerRef', () => {
 
   beforeEach(() => {
     ɵi18nConfigureLocalize({translations: TRANSLATIONS});
-    TestBed.configureTestingModule(
-        {declarations: [StructDir, ViewContainerRefComp, ViewContainerRefApp, DestroyCasesComp]});
+    TestBed.configureTestingModule({
+      declarations: [
+        StructDir, ViewContainerRefComp, ViewContainerRefApp, DestroyCasesComp, ConstructorDir,
+        ConstructorApp, ConstructorAppWithQueries
+      ]
+    });
+  });
+
+  describe('create', () => {
+
+    it('should support view queries inside embedded views created in dir constructors', () => {
+      const fixture = TestBed.createComponent(ConstructorApp);
+      fixture.detectChanges();
+      expect(fixture.componentInstance.foo).toBeAnInstanceOf(ElementRef);
+      expect(fixture.componentInstance.foo.nativeElement)
+          .toEqual(fixture.debugElement.nativeElement.querySelector('span'));
+    });
+
+    it('should ensure results in views created in constructors do not appear before template node results',
+       () => {
+         const fixture = TestBed.createComponent(ConstructorAppWithQueries);
+         fixture.detectChanges();
+         expect(fixture.componentInstance.foo).toBeAnInstanceOf(TemplateRef);
+       });
+
   });
 
   describe('insert', () => {
@@ -265,3 +288,34 @@ export class StructDir {
 class DestroyCasesComp {
   @ViewChildren(StructDir) structDirs !: QueryList<StructDir>;
 }
+
+@Directive({selector: '[constructorDir]'})
+class ConstructorDir {
+  constructor(vcref: ViewContainerRef, tplRef: TemplateRef<any>) {
+    vcref.createEmbeddedView(tplRef);
+  }
+}
+
+@Component({
+  selector: 'constructor-app',
+  template: `    
+    <div *constructorDir>
+      <span *constructorDir #foo></span>
+    </div>
+  `
+})
+class ConstructorApp {
+  @ViewChild('foo') foo !: ElementRef;
+}
+
+@Component({
+  selector: 'constructor-app-with-queries',
+  template: `
+    <ng-template constructorDir #foo>
+      <div #foo></div>
+    </ng-template>
+  `
+})
+class ConstructorAppWithQueries {
+  @ViewChild('foo') foo !: TemplateRef<any>;
+}

packages/core/test/linker/query_integration_spec.ts

@@ -100,9 +100,18 @@ describe('Query API', () => {
 
     it('should contain the first content child when target is on <ng-template> with embedded view (issue #16568)',
        () => {
-         const template =
-             '<div directive-needs-content-child><ng-template text="foo" [ngIf]="true"><div text="bar"></div></ng-template></div>' +
-             '<needs-content-child #q><ng-template text="foo" [ngIf]="true"><div text="bar"></div></ng-template></needs-content-child>';
+         const template = `
+          <div directive-needs-content-child>
+            <ng-template text="foo" [ngIf]="true">
+              <div text="bar"></div>
+             </ng-template>
+           </div>
+           <needs-content-child #q>
+              <ng-template text="foo" [ngIf]="true">
+                <div text="bar"></div>
+              </ng-template>
+           </needs-content-child>
+         `;
          const view = createTestCmp(MyComp0, template);
          view.detectChanges();
          const q: NeedsContentChild = view.debugElement.children[1].references !['q'];