fix(ivy): unable to inherit view queries into component from directive (#29203)

Fixes components not being able to inherit their view queries from a directive.

This PR resolves FW-1146.

PR Close #29203

packages/compiler-cli/src/ngtsc/annotations/src/component.ts

@@ -145,7 +145,7 @@ export class ComponentDecoratorHandler implements
     }
 
     // Next, read the `@Component`-specific fields.
-    const {decoratedElements, decorator: component, metadata} = directiveResult;
+    const {decorator: component, metadata} = directiveResult;
 
     // Go through the root directories for this project, and select the one with the smallest
     // relative path representation.
@@ -223,22 +223,6 @@ export class ComponentDecoratorHandler implements
       });
     }
 
-    // Construct the list of view queries.
-    const coreModule = this.isCore ? undefined : '@angular/core';
-    const viewChildFromFields = queriesFromFields(
-        filterToMembersWithDecorator(decoratedElements, 'ViewChild', coreModule), this.reflector,
-        this.evaluator);
-    const viewChildrenFromFields = queriesFromFields(
-        filterToMembersWithDecorator(decoratedElements, 'ViewChildren', coreModule), this.reflector,
-        this.evaluator);
-    const viewQueries = [...viewChildFromFields, ...viewChildrenFromFields];
-
-    if (component.has('queries')) {
-      const queriesFromDecorator = extractQueriesFromDecorator(
-          component.get('queries') !, this.reflector, this.evaluator, this.isCore);
-      viewQueries.push(...queriesFromDecorator.view);
-    }
-
     // Figure out the set of styles. The ordering here is important: external resources (styleUrls)
     // precede inline styles, and styles defined in the template override styles defined in the
     // component.
@@ -288,7 +272,6 @@ export class ComponentDecoratorHandler implements
         meta: {
           ...metadata,
           template,
-          viewQueries,
           encapsulation,
           interpolation: template.interpolation,
           styles: styles || [],

packages/compiler-cli/src/ngtsc/annotations/src/directive.ts

@@ -160,10 +160,20 @@ export function extractDirectiveMetadata(
 
   const queries = [...contentChildFromFields, ...contentChildrenFromFields];
 
+  // Construct the list of view queries.
+  const viewChildFromFields = queriesFromFields(
+      filterToMembersWithDecorator(decoratedElements, 'ViewChild', coreModule), reflector,
+      evaluator);
+  const viewChildrenFromFields = queriesFromFields(
+      filterToMembersWithDecorator(decoratedElements, 'ViewChildren', coreModule), reflector,
+      evaluator);
+  const viewQueries = [...viewChildFromFields, ...viewChildrenFromFields];
+
   if (directive.has('queries')) {
     const queriesFromDecorator =
         extractQueriesFromDecorator(directive.get('queries') !, reflector, evaluator, isCore);
     queries.push(...queriesFromDecorator.content);
+    viewQueries.push(...queriesFromDecorator.view);
   }
 
   // Parse the selector.
@@ -213,7 +223,7 @@ export function extractDirectiveMetadata(
         usesOnChanges,
     },
     inputs: {...inputsFromMeta, ...inputsFromFields},
-    outputs: {...outputsFromMeta, ...outputsFromFields}, queries, selector,
+    outputs: {...outputsFromMeta, ...outputsFromFields}, queries, viewQueries, selector,
     type: new WrappedNodeExpr(clazz.name !),
     typeArgumentCount: reflector.getGenericArityOfClass(clazz) || 0,
     typeSourceSpan: null !, usesInheritance, exportAs, providers

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

@@ -983,7 +983,7 @@ describe('ngtsc behavioral tests', () => {
            env.tsconfig({strictInjectionParameters: true});
            env.write('test.ts', `
             import {Injectable} from '@angular/core';
-    
+
             @Injectable()
             export class Test {
               constructor(private notInjectable: string) {}
@@ -1000,7 +1000,7 @@ describe('ngtsc behavioral tests', () => {
            env.tsconfig({strictInjectionParameters: true});
            env.write('test.ts', `
             import {Injectable} from '@angular/core';
-    
+
             @Injectable()
             export class Test {
               constructor(private notInjectable: string) {}
@@ -1017,7 +1017,7 @@ describe('ngtsc behavioral tests', () => {
            env.tsconfig({strictInjectionParameters: true});
            env.write('test.ts', `
                import {Injectable} from '@angular/core';
-       
+
                @Injectable({
                  providedIn: 'root',
                  useValue: '42',
@@ -1358,6 +1358,41 @@ describe('ngtsc behavioral tests', () => {
     expect(jsContents).toMatch(viewQueryRegExp(true));
   });
 
+  it('should generate queries for directives', () => {
+    env.tsconfig();
+    env.write(`test.ts`, `
+        import {Directive, ContentChild, ContentChildren, TemplateRef, ViewChild} from '@angular/core';
+
+        @Directive({
+          selector: '[test]',
+          queries: {
+            'mview': new ViewChild('test1'),
+            'mcontent': new ContentChild('test2'),
+          }
+        })
+        class FooCmp {
+          @ContentChild('bar', {read: TemplateRef}) child: any;
+          @ContentChildren(TemplateRef) children: any;
+          get aview(): any { return null; }
+          @ViewChild('accessor') set aview(value: any) {}
+        }
+    `);
+
+    env.driveMain();
+    const jsContents = env.getContents('test.js');
+    expect(jsContents).toMatch(varRegExp('bar'));
+    expect(jsContents).toMatch(varRegExp('test1'));
+    expect(jsContents).toMatch(varRegExp('test2'));
+    expect(jsContents).toMatch(varRegExp('accessor'));
+    // match `i0.ɵcontentQuery(dirIndex, _c1, true, TemplateRef)`
+    expect(jsContents).toMatch(contentQueryRegExp('\\w+', true, 'TemplateRef'));
+
+    // match `i0.ɵviewQuery(_c2, true, null)`
+    // Note that while ViewQuery doesn't necessarily make sense on a directive, because it doesn't
+    // have a view, we still need to handle it because a component could extend the directive.
+    expect(jsContents).toMatch(viewQueryRegExp(true));
+  });
+
   it('should handle queries that use forwardRef', () => {
     env.tsconfig();
     env.write(`test.ts`, `
@@ -1931,7 +1966,7 @@ describe('ngtsc behavioral tests', () => {
   it('should be able to compile an app using the factory shim', () => {
     env.tsconfig({'allowEmptyCodegenFiles': true});
 
-    env.write('test.ts', `        
+    env.write('test.ts', `
         export {MyModuleNgFactory} from './my-module.ngfactory';
     `);
 
@@ -2217,7 +2252,7 @@ describe('ngtsc behavioral tests', () => {
             import {Component} from '@angular/core';
             import {Other} from './types';
             import Default from './types';
-   
+
             @Component({selector: 'test', template: 'test'})
             export class SomeCmp {
               constructor(arg: Default, other: Other) {}

packages/compiler/src/compiler_facade_interface.ts

@@ -129,13 +129,13 @@ export interface R3DirectiveMetadataFacade {
   usesInheritance: boolean;
   exportAs: string[]|null;
   providers: Provider[]|null;
+  viewQueries: R3QueryMetadataFacade[];
 }
 
 export interface R3ComponentMetadataFacade extends R3DirectiveMetadataFacade {
   template: string;
   preserveWhitespaces: boolean;
   animations: any[]|undefined;
-  viewQueries: R3QueryMetadataFacade[];
   pipes: Map<string, any>;
   directives: {selector: string, expression: any}[];
   styles: string[];

packages/compiler/src/jit_compiler_facade.ts

@@ -133,7 +133,6 @@ export class CompilerFacadeImpl implements CompilerFacade {
           ...convertDirectiveFacadeToMetadata(facade),
           selector: facade.selector || this.elementSchemaRegistry.getDefaultComponentElementName(),
           template,
-          viewQueries: facade.viewQueries.map(convertToR3QueryMetadata),
           wrapDirectivesAndPipesInClosure: false,
           styles: facade.styles || [],
           encapsulation: facade.encapsulation as any,
@@ -235,6 +234,7 @@ function convertDirectiveFacadeToMetadata(facade: R3DirectiveMetadataFacade): R3
     outputs: {...outputsFromMetadata, ...outputsFromType},
     queries: facade.queries.map(convertToR3QueryMetadata),
     providers: facade.providers != null ? new WrappedNodeExpr(facade.providers) : null,
+    viewQueries: facade.viewQueries.map(convertToR3QueryMetadata),
   };
 }
 

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

@@ -53,6 +53,11 @@ export interface R3DirectiveMetadata {
    */
   queries: R3QueryMetadata[];
 
+  /**
+   * Information about the view queries made by the directive.
+   */
+  viewQueries: R3QueryMetadata[];
+
   /**
    * Mappings indicating how the directive interacts with its host element (host bindings,
    * listeners, etc).
@@ -128,11 +133,6 @@ export interface R3ComponentMetadata extends R3DirectiveMetadata {
     nodes: t.Node[];
   };
 
-  /**
-   * Information about the view queries made by the component.
-   */
-  viewQueries: R3QueryMetadata[];
-
   /**
    * A map of pipe names to an expression referencing the pipe type which are in the scope of the
    * compilation.

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

@@ -69,6 +69,10 @@ function baseDirectiveFields(
     definitionMap.set('contentQueries', createContentQueriesFunction(meta, constantPool));
   }
 
+  if (meta.viewQueries.length) {
+    definitionMap.set('viewQuery', createViewQueriesFunction(meta, constantPool));
+  }
+
   // Initialize hostVarsCount to number of bound host properties (interpolations illegal),
   // except 'style' and 'class' properties, since they should *not* allocate host var slots
   const hostVarsCount = Object.keys(meta.host.properties)
@@ -228,10 +232,6 @@ export function compileComponentFromMetadata(
     directiveMatcher = matcher;
   }
 
-  if (meta.viewQueries.length) {
-    definitionMap.set('viewQuery', createViewQueriesFunction(meta, constantPool));
-  }
-
   // e.g. `template: function MyComponent_Template(_ctx, _cm) {...}`
   const templateTypeName = meta.name;
   const templateName = templateTypeName ? `${templateTypeName}_Template` : null;
@@ -549,7 +549,7 @@ function createTypeForDef(meta: R3DirectiveMetadata, typeBase: o.ExternalReferen
 
 // Define and update any view queries
 function createViewQueriesFunction(
-    meta: R3ComponentMetadata, constantPool: ConstantPool): o.Expression {
+    meta: R3DirectiveMetadata, constantPool: ConstantPool): o.Expression {
   const createStatements: o.Statement[] = [];
   const updateStatements: o.Statement[] = [];
   const tempAllocator = temporaryAllocator(updateStatements, TEMPORARY_NAME);

packages/core/src/compiler/compiler_facade_interface.ts

@@ -129,13 +129,13 @@ export interface R3DirectiveMetadataFacade {
   usesInheritance: boolean;
   exportAs: string[]|null;
   providers: Provider[]|null;
+  viewQueries: R3QueryMetadataFacade[];
 }
 
 export interface R3ComponentMetadataFacade extends R3DirectiveMetadataFacade {
   template: string;
   preserveWhitespaces: boolean;
   animations: any[]|undefined;
-  viewQueries: R3QueryMetadataFacade[];
   pipes: Map<string, any>;
   directives: {selector: string, expression: any}[];
   styles: string[];

packages/core/src/render3/definition.ts

@@ -597,6 +597,12 @@ export const defineDirective = defineComponent as any as<T>(directiveDefinition:
    */
   contentQueries?: ContentQueriesFunction<T>;
 
+  /**
+   * Additional set of instructions specific to view query processing. This could be seen as a
+   * set of instructions to be inserted into the template function.
+   */
+  viewQuery?: ViewQueriesFunction<T>| null;
+
   /**
    * Defines the name that can be used in the template to assign this directive to a variable.
    *

packages/core/src/render3/features/inherit_definition_feature.ts

@@ -73,18 +73,17 @@ export function InheritDefinitionFeature(definition: DirectiveDef<any>| Componen
       }
 
       // Merge View Queries
-      if (isComponentDef(definition) && isComponentDef(superDef)) {
-        const prevViewQuery = definition.viewQuery;
-        const superViewQuery = superDef.viewQuery;
-        if (superViewQuery) {
-          if (prevViewQuery) {
-            definition.viewQuery = <T>(rf: RenderFlags, ctx: T): void => {
-              superViewQuery(rf, ctx);
-              prevViewQuery(rf, ctx);
-            };
-          } else {
-            definition.viewQuery = superViewQuery;
-          }
+      const prevViewQuery = definition.viewQuery;
+      const superViewQuery = superDef.viewQuery;
+
+      if (superViewQuery) {
+        if (prevViewQuery) {
+          definition.viewQuery = <T>(rf: RenderFlags, ctx: T): void => {
+            superViewQuery(rf, ctx);
+            prevViewQuery(rf, ctx);
+          };
+        } else {
+          definition.viewQuery = superViewQuery;
         }
       }
 

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

@@ -158,6 +158,13 @@ export interface DirectiveDef<T> extends BaseDef<T> {
    */
   contentQueries: ContentQueriesFunction<T>|null;
 
+  /**
+   * Query-related instructions for a directive. Note that while directives don't have a
+   * view and as such view queries won't necessarily do anything, there might be
+   * components that extend the directive.
+   */
+  viewQuery: ViewQueriesFunction<T>|null;
+
   /**
    * Refreshes host bindings on the associated directive.
    */

packages/core/src/render3/jit/directive.ts

@@ -63,8 +63,6 @@ export function compileComponent(type: Type<any>, metadata: Component): void {
           preserveWhitespaces: metadata.preserveWhitespaces || false,
           styles: metadata.styles || EMPTY_ARRAY,
           animations: metadata.animations,
-          viewQueries:
-              extractQueriesMetadata(type, getReflect().ownPropMetadata(type), isViewQuery),
           directives: [],
           changeDetection: metadata.changeDetection,
           pipes: new Map(),
@@ -157,6 +155,7 @@ export function directiveMetadata(type: Type<any>, metadata: Directive): R3Direc
     usesInheritance: !extendsDirectlyFromObject(type),
     exportAs: extractExportAs(metadata.exportAs),
     providers: metadata.providers || null,
+    viewQueries: extractQueriesMetadata(type, propMetadata, isViewQuery),
   };
 }
 

packages/core/test/acceptance/query_spec.ts

@@ -17,7 +17,8 @@ describe('query logic', () => {
       declarations: [
         AppComp, QueryComp, SimpleCompA, SimpleCompB, StaticViewQueryComp, TextDirective,
         SubclassStaticViewQueryComp, StaticContentQueryComp, SubclassStaticContentQueryComp,
-        QueryCompWithChanges, StaticContentQueryDir
+        QueryCompWithChanges, StaticContentQueryDir, SuperDirectiveQueryTarget, SuperDirective,
+        SubComponent
       ]
     });
   });
@@ -161,6 +162,17 @@ describe('query logic', () => {
       expect(secondComponent.setEvents).toEqual(['textDir set', 'foo set']);
     });
 
+    it('should allow for view queries to be inherited from a directive', () => {
+      const fixture = TestBed.createComponent(SubComponent);
+      const comp = fixture.componentInstance;
+      fixture.detectChanges();
+
+      expect(comp.headers).toBeTruthy();
+      expect(comp.headers.length).toBe(2);
+      expect(comp.headers.toArray().every(result => result instanceof SuperDirectiveQueryTarget))
+          .toBe(true);
+    });
+
   });
 
   describe('content queries', () => {
@@ -484,7 +496,7 @@ class StaticViewQueryComp {
   template: `
     <div [text]="text"></div>
     <span #foo></span>
-    
+
     <div #bar></div>
     <span #baz></span>
   `
@@ -564,3 +576,21 @@ export class QueryCompWithChanges {
 
   showing = false;
 }
+
+@Component({selector: 'query-target', template: '<ng-content></ng-content>'})
+class SuperDirectiveQueryTarget {
+}
+
+@Directive({selector: '[super-directive]'})
+class SuperDirective {
+  @ViewChildren(SuperDirectiveQueryTarget) headers !: QueryList<SuperDirectiveQueryTarget>;
+}
+
+@Component({
+  template: `
+    <query-target>One</query-target>
+    <query-target>Two</query-target>
+  `
+})
+class SubComponent extends SuperDirective {
+}

packages/core/test/render3/ivy/jit_spec.ts

@@ -349,16 +349,6 @@ ivyEnabled && describe('render3 jit', () => {
     expect((TestDirective as any).ngDirectiveDef.contentQueries).not.toBeNull();
   });
 
-  it('should not pick up view queries from directives', () => {
-    @Directive({selector: '[test]'})
-    class TestDirective {
-      @ViewChildren('foo') foos: QueryList<ElementRef>|undefined;
-    }
-
-    expect((TestDirective as any).ngDirectiveDef.contentQueries).toBeNull();
-    expect((TestDirective as any).ngDirectiveDef.viewQuery).toBeNull();
-  });
-
   it('should compile ViewChild query on a component', () => {
     @Component({selector: 'test', template: ''})
     class TestComponent {