fix(ivy): queries should register matches from top to bottom (#28319)

PR Close #28319

packages/core/src/render3/instructions.ts

@@ -500,6 +500,11 @@ export function elementContainerStart(
   appendChild(native, tNode, lView);
   createDirectivesAndLocals(tView, lView, localRefs);
   attachPatchData(native, lView);
+
+  const currentQueries = lView[QUERIES];
+  if (currentQueries) {
+    currentQueries.addNode(tNode);
+  }
 }
 
 /** Mark the end of the <ng-container>. */
@@ -518,7 +523,8 @@ export function elementContainerEnd(): void {
   ngDevMode && assertNodeType(previousOrParentTNode, TNodeType.ElementContainer);
   const currentQueries = lView[QUERIES];
   if (currentQueries) {
-    lView[QUERIES] = currentQueries.addNode(previousOrParentTNode as TElementContainerNode);
+    lView[QUERIES] =
+        isContentQueryHost(previousOrParentTNode) ? currentQueries.parent : currentQueries;
   }
 
   registerPostOrderHooks(tView, previousOrParentTNode);
@@ -592,6 +598,11 @@ export function elementStart(
   if (tNode.stylingTemplate && (tNode.flags & TNodeFlags.hasClassInput) === 0) {
     renderInitialStylesAndClasses(native, tNode.stylingTemplate, lView[RENDERER]);
   }
+
+  const currentQueries = lView[QUERIES];
+  if (currentQueries) {
+    currentQueries.addNode(tNode);
+  }
 }
 
 /**
@@ -1027,7 +1038,8 @@ export function elementEnd(): void {
   const lView = getLView();
   const currentQueries = lView[QUERIES];
   if (currentQueries) {
-    lView[QUERIES] = currentQueries.addNode(previousOrParentTNode as TElementNode);
+    lView[QUERIES] =
+        isContentQueryHost(previousOrParentTNode) ? currentQueries.parent : currentQueries;
   }
 
   registerPostOrderHooks(getLView()[TVIEW], previousOrParentTNode);

packages/core/test/linker/query_integration_spec.ts

@@ -53,10 +53,8 @@ describe('Query API', () => {
   }));
 
   describe('querying by directive type', () => {
-    fixmeIvy(
+    it('should contain all direct child directives in the light dom (constructor)', () => {
-        'FW-781 - Directives invocation sequence on root and nested elements is different in Ivy')
+      const template = `
-        .it('should contain all direct child directives in the light dom (constructor)', () => {
-          const template = `
             <div text="1"></div>
             <needs-query text="2">
               <div text="3">
@@ -65,10 +63,10 @@ describe('Query API', () => {
             </needs-query>
             <div text="4"></div>
           `;
-          const view = createTestCmpAndDetectChanges(MyComp0, template);
+      const view = createTestCmpAndDetectChanges(MyComp0, template);
 
-          expect(asNativeElements(view.debugElement.children)).toHaveText('2|3|');
+      expect(asNativeElements(view.debugElement.children)).toHaveText('2|3|');
-        });
+    });
 
     it('should contain all direct child directives in the content dom', () => {
       const template = '<needs-content-children #q><div text="foo"></div></needs-content-children>';
@@ -97,7 +95,7 @@ describe('Query API', () => {
     });
 
     fixmeIvy(
-        'FW-781 - Directives invocation sequence on root and nested elements is different in Ivy')
+        'FW-982: For queries, ng-template own directives should be registered before the ones from inside the template')
         .it('should contain the first content child when target is on <ng-template> with embedded view (issue #16568)',
             () => {
               const template =
@@ -168,43 +166,37 @@ describe('Query API', () => {
       expect(q.logs).toEqual([['setter', null], ['check', null]]);
     });
 
-    fixmeIvy(
+    it('should contain all directives in the light dom when descendants flag is used', () => {
-        'FW-781 - Directives invocation sequence on root and nested elements is different in Ivy')
+      const template = '<div text="1"></div>' +
-        .it('should contain all directives in the light dom when descendants flag is used', () => {
+          '<needs-query-desc text="2"><div text="3">' +
-          const template = '<div text="1"></div>' +
+          '<div text="4"></div>' +
-              '<needs-query-desc text="2"><div text="3">' +
+          '</div></needs-query-desc>' +
-              '<div text="4"></div>' +
+          '<div text="5"></div>';
-              '</div></needs-query-desc>' +
+      const view = createTestCmpAndDetectChanges(MyComp0, template);
-              '<div text="5"></div>';
-          const view = createTestCmpAndDetectChanges(MyComp0, template);
 
-          expect(asNativeElements(view.debugElement.children)).toHaveText('2|3|4|');
+      expect(asNativeElements(view.debugElement.children)).toHaveText('2|3|4|');
-        });
+    });
 
-    fixmeIvy(
+    it('should contain all directives in the light dom', () => {
-        'FW-781 - Directives invocation sequence on root and nested elements is different in Ivy')
+      const template = '<div text="1"></div>' +
-        .it('should contain all directives in the light dom', () => {
+          '<needs-query text="2"><div text="3"></div></needs-query>' +
-          const template = '<div text="1"></div>' +
+          '<div text="4"></div>';
-              '<needs-query text="2"><div text="3"></div></needs-query>' +
+      const view = createTestCmpAndDetectChanges(MyComp0, template);
-              '<div text="4"></div>';
-          const view = createTestCmpAndDetectChanges(MyComp0, template);
 
-          expect(asNativeElements(view.debugElement.children)).toHaveText('2|3|');
+      expect(asNativeElements(view.debugElement.children)).toHaveText('2|3|');
-        });
+    });
 
-    fixmeIvy(
+    it('should reflect dynamically inserted directives', () => {
-        'FW-781 - Directives invocation sequence on root and nested elements is different in Ivy')
+      const template = '<div text="1"></div>' +
-        .it('should reflect dynamically inserted directives', () => {
+          '<needs-query text="2"><div *ngIf="shouldShow" [text]="\'3\'"></div></needs-query>' +
-          const template = '<div text="1"></div>' +
+          '<div text="4"></div>';
-              '<needs-query text="2"><div *ngIf="shouldShow" [text]="\'3\'"></div></needs-query>' +
+      const view = createTestCmpAndDetectChanges(MyComp0, template);
-              '<div text="4"></div>';
+      expect(asNativeElements(view.debugElement.children)).toHaveText('2|');
-          const view = createTestCmpAndDetectChanges(MyComp0, template);
-          expect(asNativeElements(view.debugElement.children)).toHaveText('2|');
 
-          view.componentInstance.shouldShow = true;
+      view.componentInstance.shouldShow = true;
-          view.detectChanges();
+      view.detectChanges();
-          expect(asNativeElements(view.debugElement.children)).toHaveText('2|3|');
+      expect(asNativeElements(view.debugElement.children)).toHaveText('2|3|');
-        });
+    });
 
     it('should be cleanly destroyed when a query crosses view boundaries', () => {
       const template = '<div text="1"></div>' +
@@ -217,19 +209,17 @@ describe('Query API', () => {
       view.destroy();
     });
 
-    fixmeIvy(
+    it('should reflect moved directives', () => {
-        'FW-781 - Directives invocation sequence on root and nested elements is different in Ivy')
+      const template = '<div text="1"></div>' +
-        .it('should reflect moved directives', () => {
+          '<needs-query text="2"><div *ngFor="let  i of list" [text]="i"></div></needs-query>' +
-          const template = '<div text="1"></div>' +
+          '<div text="4"></div>';
-              '<needs-query text="2"><div *ngFor="let  i of list" [text]="i"></div></needs-query>' +
+      const view = createTestCmpAndDetectChanges(MyComp0, template);
-              '<div text="4"></div>';
+      expect(asNativeElements(view.debugElement.children)).toHaveText('2|1d|2d|3d|');
-          const view = createTestCmpAndDetectChanges(MyComp0, template);
-          expect(asNativeElements(view.debugElement.children)).toHaveText('2|1d|2d|3d|');
 
-          view.componentInstance.list = ['3d', '2d'];
+      view.componentInstance.list = ['3d', '2d'];
-          view.detectChanges();
+      view.detectChanges();
-          expect(asNativeElements(view.debugElement.children)).toHaveText('2|3d|2d|');
+      expect(asNativeElements(view.debugElement.children)).toHaveText('2|3d|2d|');
-        });
+    });
 
     it('should throw with descriptive error when query selectors are not present', () => {
       TestBed.configureTestingModule({declarations: [MyCompBroken0, HasNullQueryCondition]});
@@ -472,25 +462,19 @@ describe('Query API', () => {
   });
 
   describe('querying in the view', () => {
-    fixmeIvy(
+    it('should contain all the elements in the view with that have the given directive', () => {
-        'FW-781 - Directives invocation sequence on root and nested elements is different in Ivy')
+      const template = '<needs-view-query #q><div text="ignoreme"></div></needs-view-query>';
-        .it('should contain all the elements in the view with that have the given directive',
+      const view = createTestCmpAndDetectChanges(MyComp0, template);
-            () => {
+      const q: NeedsViewQuery = view.debugElement.children[0].references !['q'];
-              const template =
+      expect(q.query.map((d: TextDirective) => d.text)).toEqual(['1', '2', '3', '4']);
-                  '<needs-view-query #q><div text="ignoreme"></div></needs-view-query>';
+    });
-              const view = createTestCmpAndDetectChanges(MyComp0, template);
-              const q: NeedsViewQuery = view.debugElement.children[0].references !['q'];
-              expect(q.query.map((d: TextDirective) => d.text)).toEqual(['1', '2', '3', '4']);
-            });
 
-    fixmeIvy(
+    it('should not include directive present on the host element', () => {
-        'FW-781 - Directives invocation sequence on root and nested elements is different in Ivy')
+      const template = '<needs-view-query #q text="self"></needs-view-query>';
-        .it('should not include directive present on the host element', () => {
+      const view = createTestCmpAndDetectChanges(MyComp0, template);
-          const template = '<needs-view-query #q text="self"></needs-view-query>';
+      const q: NeedsViewQuery = view.debugElement.children[0].references !['q'];
-          const view = createTestCmpAndDetectChanges(MyComp0, template);
+      expect(q.query.map((d: TextDirective) => d.text)).toEqual(['1', '2', '3', '4']);
-          const q: NeedsViewQuery = view.debugElement.children[0].references !['q'];
+    });
-          expect(q.query.map((d: TextDirective) => d.text)).toEqual(['1', '2', '3', '4']);
-        });
 
     it('should reflect changes in the component', () => {
       const template = '<needs-view-query-if #q></needs-view-query-if>';

packages/core/test/render3/query_spec.ts

@@ -2774,4 +2774,124 @@ describe('query', () => {
          expect(deepInstance !.foos.length).toBe(2);
        });
   });
+
+  describe('order', () => {
+    class TextDirective {
+      value !: string;
+
+      static ngDirectiveDef = defineDirective({
+        type: TextDirective,
+        selectors: [['', 'text', '']],
+        factory: () => new TextDirective(),
+        inputs: {value: 'text'}
+      });
+    }
+
+    it('should register content matches from top to bottom', () => {
+      let contentQueryDirective: ContentQueryDirective;
+
+      class ContentQueryDirective {
+        // @ContentChildren(TextDirective)
+        texts !: QueryList<TextDirective>;
+
+        static ngComponentDef = defineDirective({
+          type: ContentQueryDirective,
+          selectors: [['', 'content-query', '']],
+          factory: () => contentQueryDirective = new ContentQueryDirective(),
+          contentQueries:
+              (dirIndex) => { registerContentQuery(query(TextDirective, true), dirIndex); },
+          contentQueriesRefresh: (dirIndex: number, queryStartIdx: number) => {
+            let tmp: any;
+            const instance = load<ContentQueryDirective>(dirIndex);
+            queryRefresh(tmp = loadQueryList<TextDirective>(queryStartIdx)) &&
+                (instance.texts = tmp);
+          }
+        });
+      }
+
+      const AppComponent = createComponent(
+          'app-component',
+          /**
+          * <div content-query>
+          *    <span text="A"></span>
+          *    <div text="B">
+          *       <span text="C">
+          *         <span text="D"></span>
+          *       </span>
+          *    </div>
+          *    <span text="E"></span>
+          * </div>
+          */
+          function(rf: RenderFlags, ctx: any) {
+            if (rf & RenderFlags.Create) {
+              elementStart(0, 'div', ['content-query']);
+              {
+                element(1, 'span', ['text', 'A']);
+                elementStart(2, 'div', ['text', 'B']);
+                elementStart(3, 'span', ['text', 'C']);
+                { element(4, 'span', ['text', 'D']); }
+                elementEnd();
+                elementEnd();
+                element(5, 'span', ['text', 'E']);
+              }
+              elementEnd();
+            }
+          },
+          6, 0, [TextDirective, ContentQueryDirective]);
+
+      new ComponentFixture(AppComponent);
+      expect(contentQueryDirective !.texts.map(item => item.value)).toEqual([
+        'A', 'B', 'C', 'D', 'E'
+      ]);
+    });
+
+    it('should register view matches from top to bottom', () => {
+      /**
+        *    <span text="A"></span>
+        *    <div text="B">
+        *       <span text="C">
+        *         <span text="D"></span>
+        *       </span>
+        *    </div>
+        *    <span text="E"></span>
+        */
+      class ViewQueryComponent {
+        // @ViewChildren(TextDirective)
+        texts !: QueryList<TextDirective>;
+
+        static ngComponentDef = defineComponent({
+          type: ViewQueryComponent,
+          selectors: [['view-query']],
+          factory: () => new ViewQueryComponent(),
+          template: function(rf: RenderFlags, ctx: ViewQueryComponent) {
+            if (rf & RenderFlags.Create) {
+              element(0, 'span', ['text', 'A']);
+              elementStart(1, 'div', ['text', 'B']);
+              elementStart(2, 'span', ['text', 'C']);
+              { element(3, 'span', ['text', 'D']); }
+              elementEnd();
+              elementEnd();
+              element(4, 'span', ['text', 'E']);
+            }
+          },
+          consts: 5,
+          vars: 0,
+          viewQuery: function(rf: RenderFlags, ctx: ViewQueryComponent) {
+            let tmp: any;
+            if (rf & RenderFlags.Create) {
+              viewQuery(TextDirective, true);
+            }
+            if (rf & RenderFlags.Update) {
+              queryRefresh(tmp = loadViewQuery<QueryList<TextDirective>>()) &&
+                  (ctx.texts = tmp as QueryList<TextDirective>);
+            }
+          },
+          directives: [TextDirective]
+        });
+      }
+
+      const fixture = new ComponentFixture(ViewQueryComponent);
+      expect(fixture.component.texts.map(item => item.value)).toEqual(['A', 'B', 'C', 'D', 'E']);
+    });
+  });
 });