From c7f1b0a97fcc1497f20838a35f1ecd9f5d321a63 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Sun, 21 Apr 2019 17:37:15 +0200 Subject: [PATCH] fix(ivy): queries not being inherited from undecorated classes (#30015) Fixes view and content queries not being inherited in Ivy, if the base class hasn't been annotated with an Angular decorator (e.g. `Component` or `Directive`). Also reworks the way the `ngBaseDef` is created so that it is added at the same point as the queries, rather than inside of the `Input` and `Output` decorators. This PR partially resolves FW-1275. Support for host bindings will be added in a follow-up, because this PR is somewhat large as it is. PR Close #30015 --- .../src/ngtsc/annotations/src/base_def.ts | 54 +++-- .../compliance/r3_compiler_compliance_spec.ts | 187 ++++++++++++++++ .../compiler/src/compiler_facade_interface.ts | 9 + packages/compiler/src/jit_compiler_facade.ts | 18 +- .../compiler/src/render3/view/compiler.ts | 29 ++- .../src/compiler/compiler_facade_interface.ts | 9 + packages/core/src/metadata/directives.ts | 46 +--- packages/core/src/render3/definition.ts | 19 +- .../features/inherit_definition_feature.ts | 65 +++--- .../core/src/render3/interfaces/definition.ts | 26 +-- packages/core/src/render3/jit/directive.ts | 76 ++++++- .../core/test/acceptance/integration_spec.ts | 58 ++++- packages/core/test/acceptance/query_spec.ts | 202 ++++++++++++++++++ packages/core/test/render3/ivy/jit_spec.ts | 26 --- tools/public_api_guard/core/core.d.ts | 4 + 15 files changed, 688 insertions(+), 140 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/base_def.ts b/packages/compiler-cli/src/ngtsc/annotations/src/base_def.ts index 93298e7770ff3..a93031661fc3d 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/base_def.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/base_def.ts @@ -6,12 +6,13 @@ * found in the LICENSE file at https://angular.io/license */ -import {R3BaseRefMetaData, compileBaseDefFromMetadata} from '@angular/compiler'; +import {ConstantPool, R3BaseRefMetaData, compileBaseDefFromMetadata} from '@angular/compiler'; import {PartialEvaluator} from '../../partial_evaluator'; import {ClassDeclaration, ClassMember, Decorator, ReflectionHost} from '../../reflection'; import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence} from '../../transform'; +import {queriesFromFields} from './directive'; import {isAngularDecorator} from './util'; function containsNgTopLevelDecorator(decorators: Decorator[] | null, isCore: boolean): boolean { @@ -44,17 +45,30 @@ export class BaseDefDecoratorHandler implements this.reflector.getMembersOfClass(node).forEach(property => { const {decorators} = property; - if (decorators) { - for (const decorator of decorators) { - if (isAngularDecorator(decorator, 'Input', this.isCore)) { - result = result || {}; - const inputs = result.inputs = result.inputs || []; - inputs.push({decorator, property}); - } else if (isAngularDecorator(decorator, 'Output', this.isCore)) { - result = result || {}; - const outputs = result.outputs = result.outputs || []; - outputs.push({decorator, property}); - } + if (!decorators) { + return; + } + for (const decorator of decorators) { + if (isAngularDecorator(decorator, 'Input', this.isCore)) { + result = result || {}; + const inputs = result.inputs = result.inputs || []; + inputs.push({decorator, property}); + } else if (isAngularDecorator(decorator, 'Output', this.isCore)) { + result = result || {}; + const outputs = result.outputs = result.outputs || []; + outputs.push({decorator, property}); + } else if ( + isAngularDecorator(decorator, 'ViewChild', this.isCore) || + isAngularDecorator(decorator, 'ViewChildren', this.isCore)) { + result = result || {}; + const viewQueries = result.viewQueries = result.viewQueries || []; + viewQueries.push({member: property, decorators}); + } else if ( + isAngularDecorator(decorator, 'ContentChild', this.isCore) || + isAngularDecorator(decorator, 'ContentChildren', this.isCore)) { + result = result || {}; + const queries = result.queries = result.queries || []; + queries.push({member: property, decorators}); } } }); @@ -110,11 +124,21 @@ export class BaseDefDecoratorHandler implements }); } + if (metadata.viewQueries) { + analysis.viewQueries = + queriesFromFields(metadata.viewQueries, this.reflector, this.evaluator); + } + + if (metadata.queries) { + analysis.queries = queriesFromFields(metadata.queries, this.reflector, this.evaluator); + } + return {analysis}; } - compile(node: ClassDeclaration, analysis: R3BaseRefMetaData): CompileResult[]|CompileResult { - const {expression, type} = compileBaseDefFromMetadata(analysis); + compile(node: ClassDeclaration, analysis: R3BaseRefMetaData, pool: ConstantPool): + CompileResult[]|CompileResult { + const {expression, type} = compileBaseDefFromMetadata(analysis, pool); return { name: 'ngBaseDef', @@ -127,4 +151,6 @@ export class BaseDefDecoratorHandler implements export interface R3BaseRefDecoratorDetection { inputs?: Array<{property: ClassMember, decorator: Decorator}>; outputs?: Array<{property: ClassMember, decorator: Decorator}>; + viewQueries?: {member: ClassMember, decorators: Decorator[]}[]; + queries?: {member: ClassMember, decorators: Decorator[]}[]; } diff --git a/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts b/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts index cf4d1325b40f1..705ab0e5a3ea2 100644 --- a/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts +++ b/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts @@ -2890,6 +2890,17 @@ describe('compiler compliance', () => { }); describe('inherited base classes', () => { + const directive = { + 'some.directive.ts': ` + import {Directive} from '@angular/core'; + + @Directive({ + selector: '[someDir]', + }) + export class SomeDirective { } + ` + }; + it('should add ngBaseDef if one or more @Input is present', () => { const files = { app: { @@ -3033,6 +3044,182 @@ describe('compiler compliance', () => { expectEmit(result.source, expectedOutput, 'Invalid base definition'); }); + it('should add ngBaseDef if a ViewChild query is present', () => { + const files = { + app: { + 'spec.ts': ` + import {Component, NgModule, ViewChild} from '@angular/core'; + export class BaseClass { + @ViewChild('something') something: any; + } + + @Component({ + selector: 'my-component', + template: '' + }) + export class MyComponent extends BaseClass { + } + + @NgModule({ + declarations: [MyComponent] + }) + export class MyModule {} + ` + } + }; + const expectedOutput = ` + const $e0_attrs$ = ["something"]; + // ... + BaseClass.ngBaseDef = i0.ɵɵdefineBase({ + viewQuery: function (rf, ctx) { + if (rf & 1) { + $r3$.ɵɵviewQuery($e0_attrs$, true, null); + } + if (rf & 2) { + var $tmp$; + ($r3$.ɵɵqueryRefresh(($tmp$ = $r3$.ɵɵloadViewQuery())) && (ctx.something = $tmp$.first)); + } + } + }); + // ... + `; + const result = compile(files, angularFiles); + expectEmit(result.source, expectedOutput, 'Invalid base definition'); + }); + + it('should add ngBaseDef if a ViewChildren query is present', () => { + const files = { + app: { + ...directive, + 'spec.ts': ` + import {Component, NgModule, ViewChildren} from '@angular/core'; + import {SomeDirective} from './some.directive'; + + export class BaseClass { + @ViewChildren(SomeDirective) something: QueryList; + } + + @Component({ + selector: 'my-component', + template: '' + }) + export class MyComponent extends BaseClass { + } + + @NgModule({ + declarations: [MyComponent, SomeDirective] + }) + export class MyModule {} + ` + } + }; + const expectedOutput = ` + // ... + BaseClass.ngBaseDef = i0.ɵɵdefineBase({ + viewQuery: function (rf, ctx) { + if (rf & 1) { + $r3$.ɵɵviewQuery(SomeDirective, true, null); + } + if (rf & 2) { + var $tmp$; + ($r3$.ɵɵqueryRefresh(($tmp$ = $r3$.ɵɵloadViewQuery())) && (ctx.something = $tmp$)); + } + } + }); + // ... + `; + const result = compile(files, angularFiles); + expectEmit(result.source, expectedOutput, 'Invalid base definition'); + }); + + it('should add ngBaseDef if a ContentChild query is present', () => { + const files = { + app: { + 'spec.ts': ` + import {Component, NgModule, ContentChild} from '@angular/core'; + export class BaseClass { + @ContentChild('something') something: any; + } + + @Component({ + selector: 'my-component', + template: '' + }) + export class MyComponent extends BaseClass { + } + + @NgModule({ + declarations: [MyComponent] + }) + export class MyModule {} + ` + } + }; + const expectedOutput = ` + const $e0_attrs$ = ["something"]; + // ... + BaseClass.ngBaseDef = i0.ɵɵdefineBase({ + contentQueries: function (rf, ctx, dirIndex) { + if (rf & 1) { + $r3$.ɵɵcontentQuery(dirIndex, $e0_attrs$, true, null); + } + if (rf & 2) { + var $tmp$; + ($r3$.ɵɵqueryRefresh(($tmp$ = $r3$.ɵɵloadContentQuery())) && (ctx.something = $tmp$.first)); + } + } + }); + // ... + `; + const result = compile(files, angularFiles); + expectEmit(result.source, expectedOutput, 'Invalid base definition'); + }); + + it('should add ngBaseDef if a ContentChildren query is present', () => { + const files = { + app: { + ...directive, + 'spec.ts': ` + import {Component, NgModule, ContentChildren} from '@angular/core'; + import {SomeDirective} from './some.directive'; + + export class BaseClass { + @ContentChildren(SomeDirective) something: QueryList; + } + + @Component({ + selector: 'my-component', + template: '' + }) + export class MyComponent extends BaseClass { + } + + @NgModule({ + declarations: [MyComponent, SomeDirective] + }) + export class MyModule {} + ` + } + }; + const expectedOutput = ` + // ... + BaseClass.ngBaseDef = i0.ɵɵdefineBase({ + contentQueries: function (rf, ctx, dirIndex) { + if (rf & 1) { + $r3$.ɵɵcontentQuery(dirIndex, SomeDirective, false, null); + } + if (rf & 2) { + var $tmp$; + ($r3$.ɵɵqueryRefresh(($tmp$ = $r3$.ɵɵloadContentQuery())) && (ctx.something = $tmp$)); + } + } + }); + // ... + `; + const result = compile(files, angularFiles); + expectEmit(result.source, expectedOutput, 'Invalid base definition'); + }); + it('should NOT add ngBaseDef if @Component is present', () => { const files = { app: { diff --git a/packages/compiler/src/compiler_facade_interface.ts b/packages/compiler/src/compiler_facade_interface.ts index a010cff02e2a6..97e8c07d5447b 100644 --- a/packages/compiler/src/compiler_facade_interface.ts +++ b/packages/compiler/src/compiler_facade_interface.ts @@ -37,6 +37,8 @@ export interface CompilerFacade { angularCoreEnv: CoreEnvironment, sourceMapUrl: string, meta: R3DirectiveMetadataFacade): any; compileComponent( angularCoreEnv: CoreEnvironment, sourceMapUrl: string, meta: R3ComponentMetadataFacade): any; + compileBase(angularCoreEnv: CoreEnvironment, sourceMapUrl: string, meta: R3BaseMetadataFacade): + any; createParseSourceSpan(kind: string, typeName: string, sourceUrl: string): ParseSourceSpan; @@ -146,6 +148,13 @@ export interface R3ComponentMetadataFacade extends R3DirectiveMetadataFacade { changeDetection?: ChangeDetectionStrategy; } +export interface R3BaseMetadataFacade { + inputs?: {[key: string]: string | [string, string]}; + outputs?: {[key: string]: string}; + queries?: R3QueryMetadataFacade[]; + viewQueries?: R3QueryMetadataFacade[]; +} + export type ViewEncapsulation = number; export type ChangeDetectionStrategy = number; diff --git a/packages/compiler/src/jit_compiler_facade.ts b/packages/compiler/src/jit_compiler_facade.ts index 91eece387eb28..b2d9396a246cd 100644 --- a/packages/compiler/src/jit_compiler_facade.ts +++ b/packages/compiler/src/jit_compiler_facade.ts @@ -7,7 +7,7 @@ */ -import {CompilerFacade, CoreEnvironment, ExportedCompilerFacade, R3ComponentMetadataFacade, R3DependencyMetadataFacade, R3DirectiveMetadataFacade, R3InjectableMetadataFacade, R3InjectorMetadataFacade, R3NgModuleMetadataFacade, R3PipeMetadataFacade, R3QueryMetadataFacade, StringMap, StringMapWithRename} from './compiler_facade_interface'; +import {CompilerFacade, CoreEnvironment, ExportedCompilerFacade, R3BaseMetadataFacade, R3ComponentMetadataFacade, R3DependencyMetadataFacade, R3DirectiveMetadataFacade, R3InjectableMetadataFacade, R3InjectorMetadataFacade, R3NgModuleMetadataFacade, R3PipeMetadataFacade, R3QueryMetadataFacade, StringMap, StringMapWithRename} from './compiler_facade_interface'; import {ConstantPool} from './constant_pool'; import {HostBinding, HostListener, Input, Output, Type} from './core'; import {compileInjectable} from './injectable_compiler_2'; @@ -21,7 +21,7 @@ import {R3InjectorMetadata, R3NgModuleMetadata, compileInjector, compileNgModule import {compilePipeFromMetadata} from './render3/r3_pipe_compiler'; import {R3Reference} from './render3/util'; import {R3DirectiveMetadata, R3QueryMetadata} from './render3/view/api'; -import {ParsedHostBindings, compileComponentFromMetadata, compileDirectiveFromMetadata, parseHostBindings, verifyHostBindings} from './render3/view/compiler'; +import {ParsedHostBindings, compileBaseDefFromMetadata, compileComponentFromMetadata, compileDirectiveFromMetadata, parseHostBindings, verifyHostBindings} from './render3/view/compiler'; import {makeBindingParser, parseTemplate} from './render3/view/template'; import {ResourceLoader} from './resource_loader'; import {DomElementSchemaRegistry} from './schema/dom_element_schema_registry'; @@ -151,6 +151,20 @@ export class CompilerFacadeImpl implements CompilerFacade { res.expression, angularCoreEnv, `ng:///${facade.name}.js`, preStatements); } + compileBase(angularCoreEnv: CoreEnvironment, sourceMapUrl: string, facade: R3BaseMetadataFacade): + any { + const constantPool = new ConstantPool(); + const meta = { + ...facade, + viewQueries: facade.viewQueries ? facade.viewQueries.map(convertToR3QueryMetadata) : + facade.viewQueries, + queries: facade.queries ? facade.queries.map(convertToR3QueryMetadata) : facade.queries + }; + const res = compileBaseDefFromMetadata(meta, constantPool); + return this.jitExpression( + res.expression, angularCoreEnv, sourceMapUrl, constantPool.statements); + } + createParseSourceSpan(kind: string, typeName: string, sourceUrl: string): ParseSourceSpan { return r3JitTypeSourceSpan(kind, typeName, sourceUrl); } diff --git a/packages/compiler/src/render3/view/compiler.ts b/packages/compiler/src/render3/view/compiler.ts index 606446dec4db9..a26424f0edeb8 100644 --- a/packages/compiler/src/render3/view/compiler.ts +++ b/packages/compiler/src/render3/view/compiler.ts @@ -66,11 +66,13 @@ function baseDirectiveFields( if (meta.queries.length > 0) { // e.g. `contentQueries: (rf, ctx, dirIndex) => { ... } - definitionMap.set('contentQueries', createContentQueriesFunction(meta, constantPool)); + definitionMap.set( + 'contentQueries', createContentQueriesFunction(meta.queries, constantPool, meta.name)); } if (meta.viewQueries.length) { - definitionMap.set('viewQuery', createViewQueriesFunction(meta, constantPool)); + definitionMap.set( + 'viewQuery', createViewQueriesFunction(meta.viewQueries, constantPool, meta.name)); } // Initialize hostVarsCount to number of bound host properties (interpolations illegal), @@ -163,13 +165,15 @@ export function compileDirectiveFromMetadata( export interface R3BaseRefMetaData { inputs?: {[key: string]: string | [string, string]}; outputs?: {[key: string]: string}; + viewQueries?: R3QueryMetadata[]; + queries?: R3QueryMetadata[]; } /** * Compile a base definition for the render3 runtime as defined by {@link R3BaseRefMetadata} * @param meta the metadata used for compilation. */ -export function compileBaseDefFromMetadata(meta: R3BaseRefMetaData) { +export function compileBaseDefFromMetadata(meta: R3BaseRefMetaData, constantPool: ConstantPool) { const definitionMap = new DefinitionMap(); if (meta.inputs) { const inputs = meta.inputs; @@ -188,9 +192,14 @@ export function compileBaseDefFromMetadata(meta: R3BaseRefMetaData) { }); definitionMap.set('outputs', o.literalMap(outputsMap)); } + if (meta.viewQueries && meta.viewQueries.length > 0) { + definitionMap.set('viewQuery', createViewQueriesFunction(meta.viewQueries, constantPool)); + } + if (meta.queries && meta.queries.length > 0) { + definitionMap.set('contentQueries', createContentQueriesFunction(meta.queries, constantPool)); + } const expression = o.importExpr(R3.defineBase).callFn([definitionMap.toLiteralMap()]); - const type = new o.ExpressionType(o.importExpr(R3.BaseDef)); return {expression, type}; @@ -475,12 +484,12 @@ function convertAttributesToExpressions(attributes: {[name: string]: o.Expressio // Define and update any content queries function createContentQueriesFunction( - meta: R3DirectiveMetadata, constantPool: ConstantPool): o.Expression { + queries: R3QueryMetadata[], constantPool: ConstantPool, name?: string): o.Expression { const createStatements: o.Statement[] = []; const updateStatements: o.Statement[] = []; const tempAllocator = temporaryAllocator(updateStatements, TEMPORARY_NAME); - for (const query of meta.queries) { + for (const query of queries) { // creation, e.g. r3.contentQuery(dirIndex, somePredicate, true, null); const args = [o.variable('dirIndex'), ...prepareQueryParams(query, constantPool) as any]; @@ -498,7 +507,7 @@ function createContentQueriesFunction( updateStatements.push(refresh.and(updateDirective).toStmt()); } - const contentQueriesFnName = meta.name ? `${meta.name}_ContentQueries` : null; + const contentQueriesFnName = name ? `${name}_ContentQueries` : null; return o.fn( [ new o.FnParam(RENDER_FLAGS, o.NUMBER_TYPE), new o.FnParam(CONTEXT_NAME, null), @@ -549,12 +558,12 @@ function createTypeForDef(meta: R3DirectiveMetadata, typeBase: o.ExternalReferen // Define and update any view queries function createViewQueriesFunction( - meta: R3DirectiveMetadata, constantPool: ConstantPool): o.Expression { + viewQueries: R3QueryMetadata[], constantPool: ConstantPool, name?: string): o.Expression { const createStatements: o.Statement[] = []; const updateStatements: o.Statement[] = []; const tempAllocator = temporaryAllocator(updateStatements, TEMPORARY_NAME); - meta.viewQueries.forEach((query: R3QueryMetadata) => { + viewQueries.forEach((query: R3QueryMetadata) => { const queryInstruction = query.static ? R3.staticViewQuery : R3.viewQuery; // creation, e.g. r3.viewQuery(somePredicate, true); @@ -572,7 +581,7 @@ function createViewQueriesFunction( updateStatements.push(refresh.and(updateDirective).toStmt()); }); - const viewQueryFnName = meta.name ? `${meta.name}_Query` : null; + const viewQueryFnName = name ? `${name}_Query` : null; return o.fn( [new o.FnParam(RENDER_FLAGS, o.NUMBER_TYPE), new o.FnParam(CONTEXT_NAME, null)], [ diff --git a/packages/core/src/compiler/compiler_facade_interface.ts b/packages/core/src/compiler/compiler_facade_interface.ts index a010cff02e2a6..97e8c07d5447b 100644 --- a/packages/core/src/compiler/compiler_facade_interface.ts +++ b/packages/core/src/compiler/compiler_facade_interface.ts @@ -37,6 +37,8 @@ export interface CompilerFacade { angularCoreEnv: CoreEnvironment, sourceMapUrl: string, meta: R3DirectiveMetadataFacade): any; compileComponent( angularCoreEnv: CoreEnvironment, sourceMapUrl: string, meta: R3ComponentMetadataFacade): any; + compileBase(angularCoreEnv: CoreEnvironment, sourceMapUrl: string, meta: R3BaseMetadataFacade): + any; createParseSourceSpan(kind: string, typeName: string, sourceUrl: string): ParseSourceSpan; @@ -146,6 +148,13 @@ export interface R3ComponentMetadataFacade extends R3DirectiveMetadataFacade { changeDetection?: ChangeDetectionStrategy; } +export interface R3BaseMetadataFacade { + inputs?: {[key: string]: string | [string, string]}; + outputs?: {[key: string]: string}; + queries?: R3QueryMetadataFacade[]; + viewQueries?: R3QueryMetadataFacade[]; +} + export type ViewEncapsulation = number; export type ChangeDetectionStrategy = number; diff --git a/packages/core/src/metadata/directives.ts b/packages/core/src/metadata/directives.ts index 601fb2fb655dc..a10dba99911a2 100644 --- a/packages/core/src/metadata/directives.ts +++ b/packages/core/src/metadata/directives.ts @@ -9,12 +9,10 @@ import {ChangeDetectionStrategy} from '../change_detection/constants'; import {Provider} from '../di'; import {Type} from '../interface/type'; -import {NG_BASE_DEF} from '../render3/fields'; import {compileComponent as render3CompileComponent, compileDirective as render3CompileDirective} from '../render3/jit/directive'; import {compilePipe as render3CompilePipe} from '../render3/jit/pipe'; import {TypeDecorator, makeDecorator, makePropDecorator} from '../util/decorators'; import {noop} from '../util/noop'; -import {fillProperties} from '../util/property'; import {ViewEncapsulation} from './view'; @@ -695,47 +693,12 @@ export interface Input { bindingPropertyName?: string; } -const initializeBaseDef = (target: any): void => { - const constructor = target.constructor; - const inheritedBaseDef = constructor.ngBaseDef; - - const baseDef = constructor.ngBaseDef = { - inputs: {}, - outputs: {}, - declaredInputs: {}, - }; - - if (inheritedBaseDef) { - fillProperties(baseDef.inputs, inheritedBaseDef.inputs); - fillProperties(baseDef.outputs, inheritedBaseDef.outputs); - fillProperties(baseDef.declaredInputs, inheritedBaseDef.declaredInputs); - } -}; - -/** - * Does the work of creating the `ngBaseDef` property for the `Input` and `Output` decorators. - * @param key "inputs" or "outputs" - */ -const updateBaseDefFromIOProp = (getProp: (baseDef: {inputs?: any, outputs?: any}) => any) => - (target: any, name: string, ...args: any[]) => { - const constructor = target.constructor; - - if (!constructor.hasOwnProperty(NG_BASE_DEF)) { - initializeBaseDef(target); - } - - const baseDef = constructor.ngBaseDef; - const defProp = getProp(baseDef); - defProp[name] = args[0] || name; - }; - /** * @Annotation * @publicApi */ -export const Input: InputDecorator = makePropDecorator( - 'Input', (bindingPropertyName?: string) => ({bindingPropertyName}), undefined, - updateBaseDefFromIOProp(baseDef => baseDef.inputs || {})); +export const Input: InputDecorator = + makePropDecorator('Input', (bindingPropertyName?: string) => ({bindingPropertyName})); /** * Type of the Output decorator / constructor function. @@ -777,9 +740,8 @@ export interface Output { * @Annotation * @publicApi */ -export const Output: OutputDecorator = makePropDecorator( - 'Output', (bindingPropertyName?: string) => ({bindingPropertyName}), undefined, - updateBaseDefFromIOProp(baseDef => baseDef.outputs || {})); +export const Output: OutputDecorator = + makePropDecorator('Output', (bindingPropertyName?: string) => ({bindingPropertyName})); diff --git a/packages/core/src/render3/definition.ts b/packages/core/src/render3/definition.ts index 1293409123861..4715e117fcd6d 100644 --- a/packages/core/src/render3/definition.ts +++ b/packages/core/src/render3/definition.ts @@ -18,7 +18,7 @@ import {noSideEffects} from '../util/closure'; import {stringify} from '../util/stringify'; import {EMPTY_ARRAY, EMPTY_OBJ} from './empty'; -import {NG_COMPONENT_DEF, NG_DIRECTIVE_DEF, NG_MODULE_DEF, NG_PIPE_DEF} from './fields'; +import {NG_BASE_DEF, NG_COMPONENT_DEF, NG_DIRECTIVE_DEF, NG_MODULE_DEF, NG_PIPE_DEF} from './fields'; import {ComponentDef, ComponentDefFeature, ComponentTemplate, ComponentType, ContentQueriesFunction, DirectiveDef, DirectiveDefFeature, DirectiveType, DirectiveTypesOrFactory, FactoryFn, HostBindingsFunction, PipeDef, PipeType, PipeTypesOrFactory, ViewQueriesFunction, ɵɵBaseDef} from './interfaces/definition'; // while SelectorFlags is unused here, it's required so that types don't get resolved lazily // see: https://github.com/Microsoft/web-build-tools/issues/1050 @@ -560,12 +560,25 @@ export function ɵɵdefineBase(baseDefinition: { * of properties. */ outputs?: {[P in keyof T]?: string}; + + /** + * Function to create instances of content queries associated with a given directive. + */ + contentQueries?: ContentQueriesFunction| null; + + /** + * 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| null; }): ɵɵBaseDef { const declaredInputs: {[P in keyof T]: string} = {} as any; return { inputs: invertObject(baseDefinition.inputs as any, declaredInputs), declaredInputs: declaredInputs, outputs: invertObject(baseDefinition.outputs as any), + viewQuery: baseDefinition.viewQuery || null, + contentQueries: baseDefinition.contentQueries || null, }; } @@ -742,6 +755,10 @@ export function getPipeDef(type: any): PipeDef|null { return (type as any)[NG_PIPE_DEF] || null; } +export function getBaseDef(type: any): ɵɵBaseDef|null { + return (type as any)[NG_BASE_DEF] || null; +} + export function getNgModuleDef(type: any, throwNotFound: true): NgModuleDef; export function getNgModuleDef(type: any): NgModuleDef|null; export function getNgModuleDef(type: any, throwNotFound?: boolean): NgModuleDef|null { diff --git a/packages/core/src/render3/features/inherit_definition_feature.ts b/packages/core/src/render3/features/inherit_definition_feature.ts index db6686dda1b92..76529d43908ca 100644 --- a/packages/core/src/render3/features/inherit_definition_feature.ts +++ b/packages/core/src/render3/features/inherit_definition_feature.ts @@ -9,7 +9,7 @@ import {Type} from '../../interface/type'; import {fillProperties} from '../../util/property'; import {EMPTY_ARRAY, EMPTY_OBJ} from '../empty'; -import {ComponentDef, DirectiveDef, DirectiveDefFeature, RenderFlags} from '../interfaces/definition'; +import {ComponentDef, ContentQueriesFunction, DirectiveDef, DirectiveDefFeature, RenderFlags, ViewQueriesFunction} from '../interfaces/definition'; import {adjustActiveDirectiveSuperClassDepthPosition} from '../state'; import {isComponentDef} from '../util/view_utils'; @@ -54,7 +54,10 @@ export function ɵɵInheritDefinitionFeature(definition: DirectiveDef| Comp } if (baseDef) { - // Merge inputs and outputs + const baseViewQuery = baseDef.viewQuery; + const baseContentQueries = baseDef.contentQueries; + baseViewQuery && inheritViewQuery(definition, baseViewQuery); + baseContentQueries && inheritContentQueries(definition, baseContentQueries); fillProperties(definition.inputs, baseDef.inputs); fillProperties(definition.declaredInputs, baseDef.declaredInputs); fillProperties(definition.outputs, baseDef.outputs); @@ -91,34 +94,11 @@ export function ɵɵInheritDefinitionFeature(definition: DirectiveDef| Comp } } - // Merge View Queries - const prevViewQuery = definition.viewQuery; + // Merge queries const superViewQuery = superDef.viewQuery; - - if (superViewQuery) { - if (prevViewQuery) { - definition.viewQuery = (rf: RenderFlags, ctx: T): void => { - superViewQuery(rf, ctx); - prevViewQuery(rf, ctx); - }; - } else { - definition.viewQuery = superViewQuery; - } - } - - // Merge Content Queries - const prevContentQueries = definition.contentQueries; const superContentQueries = superDef.contentQueries; - if (superContentQueries) { - if (prevContentQueries) { - definition.contentQueries = (rf: RenderFlags, ctx: T, directiveIndex: number) => { - superContentQueries(rf, ctx, directiveIndex); - prevContentQueries(rf, ctx, directiveIndex); - }; - } else { - definition.contentQueries = superContentQueries; - } - } + superViewQuery && inheritViewQuery(definition, superViewQuery); + superContentQueries && inheritContentQueries(definition, superContentQueries); // Merge inputs and outputs fillProperties(definition.inputs, superDef.inputs); @@ -181,3 +161,32 @@ function maybeUnwrapEmpty(value: any): any { return value; } } + +function inheritViewQuery( + definition: DirectiveDef| ComponentDef, superViewQuery: ViewQueriesFunction) { + const prevViewQuery = definition.viewQuery; + + if (prevViewQuery) { + definition.viewQuery = (rf, ctx) => { + superViewQuery(rf, ctx); + prevViewQuery(rf, ctx); + }; + } else { + definition.viewQuery = superViewQuery; + } +} + +function inheritContentQueries( + definition: DirectiveDef| ComponentDef, + superContentQueries: ContentQueriesFunction) { + const prevContentQueries = definition.contentQueries; + + if (prevContentQueries) { + definition.contentQueries = (rf, ctx, directiveIndex) => { + superContentQueries(rf, ctx, directiveIndex); + prevContentQueries(rf, ctx, directiveIndex); + }; + } else { + definition.contentQueries = superContentQueries; + } +} diff --git a/packages/core/src/render3/interfaces/definition.ts b/packages/core/src/render3/interfaces/definition.ts index a674901be6909..f0476d39894f0 100644 --- a/packages/core/src/render3/interfaces/definition.ts +++ b/packages/core/src/render3/interfaces/definition.ts @@ -96,7 +96,7 @@ export type ɵɵDirectiveDefWithMeta< * Runtime information for classes that are inherited by components or directives * that aren't defined as components or directives. * - * This is an internal data structure used by the render to determine what inputs + * This is an internal data structure used by the renderer to determine what inputs * and outputs should be inherited. * * See: {@link defineBase} @@ -123,6 +123,18 @@ export interface ɵɵBaseDef { * (as in `@Output('alias') propertyName: any;`). */ readonly outputs: {[P in keyof T]: string}; + + /** + * Function to create and refresh content queries associated with a given directive. + */ + contentQueries: ContentQueriesFunction|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|null; } /** @@ -161,18 +173,6 @@ export interface DirectiveDef extends ɵɵBaseDef { */ factory: FactoryFn; - /** - * Function to create and refresh content queries associated with a given directive. - */ - contentQueries: ContentQueriesFunction|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|null; - /** * Refreshes host bindings on the associated directive. */ diff --git a/packages/core/src/render3/jit/directive.ts b/packages/core/src/render3/jit/directive.ts index 6306a5bc94a34..6f775253f5d33 100644 --- a/packages/core/src/render3/jit/directive.ts +++ b/packages/core/src/render3/jit/directive.ts @@ -7,7 +7,7 @@ */ import {R3DirectiveMetadataFacade, getCompilerFacade} from '../../compiler/compiler_facade'; -import {R3ComponentMetadataFacade, R3QueryMetadataFacade} from '../../compiler/compiler_facade_interface'; +import {R3BaseMetadataFacade, R3ComponentMetadataFacade, R3QueryMetadataFacade} from '../../compiler/compiler_facade_interface'; import {resolveForwardRef} from '../../di/forward_ref'; import {compileInjectable} from '../../di/jit/injectable'; import {getReflect, reflectDependencies} from '../../di/jit/util'; @@ -16,8 +16,9 @@ import {Query} from '../../metadata/di'; import {Component, Directive, Input} from '../../metadata/directives'; import {componentNeedsResolution, maybeQueueResolutionOfComponentResources} from '../../metadata/resource_loading'; import {ViewEncapsulation} from '../../metadata/view'; +import {getBaseDef, getComponentDef, getDirectiveDef} from '../definition'; import {EMPTY_ARRAY, EMPTY_OBJ} from '../empty'; -import {NG_COMPONENT_DEF, NG_DIRECTIVE_DEF} from '../fields'; +import {NG_BASE_DEF, NG_COMPONENT_DEF, NG_DIRECTIVE_DEF} from '../fields'; import {ComponentType} from '../interfaces/definition'; import {renderStringify} from '../util/misc_utils'; @@ -71,6 +72,9 @@ export function compileComponent(type: Type, metadata: Component): void { interpolation: metadata.interpolation, viewProviders: metadata.viewProviders || null, }; + if (meta.usesInheritance) { + addBaseDefToUndecoratedParents(type); + } ngComponentDef = compiler.compileComponent(angularCoreEnv, templateUrl, meta); // When NgModule decorator executed, we enqueued the module definition such that @@ -125,6 +129,9 @@ export function compileDirective(type: Type, directive: Directive): void { const facade = directiveMetadata(type as ComponentType, directive); facade.typeSourceSpan = compiler.createParseSourceSpan('Directive', renderStringify(type), sourceMapUrl); + if (facade.usesInheritance) { + addBaseDefToUndecoratedParents(type); + } ngDirectiveDef = compiler.compileDirective(angularCoreEnv, sourceMapUrl, facade); } return ngDirectiveDef; @@ -171,6 +178,71 @@ export function directiveMetadata(type: Type, metadata: Directive): R3Direc }; } +/** + * Adds an `ngBaseDef` to all parent classes of a type that don't have an Angular decorator. + */ +function addBaseDefToUndecoratedParents(type: Type) { + const objPrototype = Object.prototype; + let parent = Object.getPrototypeOf(type); + + // Go up the prototype until we hit `Object`. + while (parent && parent !== objPrototype) { + // Since inheritance works if the class was annotated already, we only need to add + // the base def if there are no annotations and the base def hasn't been created already. + if (!getDirectiveDef(parent) && !getComponentDef(parent) && !getBaseDef(parent)) { + const facade = extractBaseDefMetadata(parent); + facade && compileBase(parent, facade); + } + parent = Object.getPrototypeOf(parent); + } +} + +/** Compiles the base metadata into a base definition. */ +function compileBase(type: Type, facade: R3BaseMetadataFacade): void { + let ngBaseDef: any = null; + Object.defineProperty(type, NG_BASE_DEF, { + get: () => { + if (ngBaseDef === null) { + const name = type && type.name; + const sourceMapUrl = `ng://${name}/ngBaseDef.js`; + const compiler = getCompilerFacade(); + ngBaseDef = compiler.compileBase(angularCoreEnv, sourceMapUrl, facade); + } + return ngBaseDef; + }, + // Make the property configurable in dev mode to allow overriding in tests + configurable: !!ngDevMode, + }); +} + +/** Extracts the metadata necessary to construct an `ngBaseDef` from a class. */ +function extractBaseDefMetadata(type: Type): R3BaseMetadataFacade|null { + const propMetadata = getReflect().ownPropMetadata(type); + const viewQueries = extractQueriesMetadata(type, propMetadata, isViewQuery); + const queries = extractQueriesMetadata(type, propMetadata, isContentQuery); + let inputs: {[key: string]: string | [string, string]}|undefined; + let outputs: {[key: string]: string}|undefined; + + for (const field in propMetadata) { + propMetadata[field].forEach(ann => { + if (ann.ngMetadataName === 'Input') { + inputs = inputs || {}; + inputs[field] = ann.bindingPropertyName ? [ann.bindingPropertyName, field] : field; + } else if (ann.ngMetadataName === 'Output') { + outputs = outputs || {}; + outputs[field] = ann.bindingPropertyName || field; + } + }); + } + + // Only generate the base def if there's any info inside it. + if (inputs || outputs || viewQueries.length || queries.length) { + return {inputs, outputs, viewQueries, queries}; + } + + return null; +} + function convertToR3QueryPredicate(selector: any): any|string[] { return typeof selector === 'string' ? splitByComma(selector) : resolveForwardRef(selector); } diff --git a/packages/core/test/acceptance/integration_spec.ts b/packages/core/test/acceptance/integration_spec.ts index 041b7d6f63a36..0bae8e89a87dd 100644 --- a/packages/core/test/acceptance/integration_spec.ts +++ b/packages/core/test/acceptance/integration_spec.ts @@ -5,11 +5,10 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ -import {Component, ContentChild, Directive, HostBinding, HostListener, Input, QueryList, TemplateRef, ViewChildren} from '@angular/core'; +import {Component, ContentChild, Directive, EventEmitter, HostListener, Input, Output, QueryList, TemplateRef, ViewChildren} from '@angular/core'; import {TestBed} from '@angular/core/testing'; import {By} from '@angular/platform-browser'; import {expect} from '@angular/platform-browser/testing/src/matchers'; -import {ivyEnabled, onlyInIvy} from '@angular/private/testing'; describe('acceptance integration tests', () => { it('should only call inherited host listeners once', () => { @@ -117,4 +116,59 @@ describe('acceptance integration tests', () => { expect(fixture.componentInstance.tpl).not.toBeNull(); expect(fixture.debugElement.nativeElement.getAttribute('aria-disabled')).toBe('true'); }); + + it('should inherit inputs from undecorated superclasses', () => { + class ButtonSuperClass { + @Input() isDisabled !: boolean; + } + + @Component({selector: 'button[custom-button]', template: ''}) + class ButtonSubClass extends ButtonSuperClass { + } + + @Component({template: ''}) + class MyApp { + disableButton = false; + } + + TestBed.configureTestingModule({declarations: [MyApp, ButtonSubClass]}); + const fixture = TestBed.createComponent(MyApp); + const button = fixture.debugElement.query(By.directive(ButtonSubClass)).componentInstance; + fixture.detectChanges(); + + expect(button.isDisabled).toBe(false); + + fixture.componentInstance.disableButton = true; + fixture.detectChanges(); + + expect(button.isDisabled).toBe(true); + }); + + it('should inherit outputs from undecorated superclasses', () => { + let clicks = 0; + + class ButtonSuperClass { + @Output() clicked = new EventEmitter(); + emitClick() { this.clicked.emit(); } + } + + @Component({selector: 'button[custom-button]', template: ''}) + class ButtonSubClass extends ButtonSuperClass { + } + + @Component({template: ''}) + class MyApp { + handleClick() { clicks++; } + } + + TestBed.configureTestingModule({declarations: [MyApp, ButtonSubClass]}); + const fixture = TestBed.createComponent(MyApp); + const button = fixture.debugElement.query(By.directive(ButtonSubClass)).componentInstance; + + button.emitClick(); + fixture.detectChanges(); + + expect(clicks).toBe(1); + }); + }); diff --git a/packages/core/test/acceptance/query_spec.ts b/packages/core/test/acceptance/query_spec.ts index f49ba8edd688d..3a260bfce74a8 100644 --- a/packages/core/test/acceptance/query_spec.ts +++ b/packages/core/test/acceptance/query_spec.ts @@ -173,6 +173,96 @@ describe('query logic', () => { .toBe(true); }); + it('should support ViewChild query inherited from undecorated superclasses', () => { + class MyComp { + @ViewChild('foo') foo: any; + } + + @Component({selector: 'sub-comp', template: '
'}) + class SubComp extends MyComp { + } + + TestBed.configureTestingModule({declarations: [SubComp]}); + + const fixture = TestBed.createComponent(SubComp); + fixture.detectChanges(); + expect(fixture.componentInstance.foo).toBeAnInstanceOf(ElementRef); + }); + + it('should support ViewChild query inherited from undecorated grand superclasses', () => { + class MySuperComp { + @ViewChild('foo') foo: any; + } + + class MyComp extends MySuperComp {} + + @Component({selector: 'sub-comp', template: '
'}) + class SubComp extends MyComp { + } + + TestBed.configureTestingModule({declarations: [SubComp]}); + + const fixture = TestBed.createComponent(SubComp); + fixture.detectChanges(); + expect(fixture.componentInstance.foo).toBeAnInstanceOf(ElementRef); + }); + + it('should support ViewChildren query inherited from undecorated superclasses', () => { + @Directive({selector: '[some-dir]'}) + class SomeDir { + } + + class MyComp { + @ViewChildren(SomeDir) foo !: QueryList; + } + + @Component({ + selector: 'sub-comp', + template: ` +
+
+ ` + }) + class SubComp extends MyComp { + } + + TestBed.configureTestingModule({declarations: [SubComp, SomeDir]}); + + const fixture = TestBed.createComponent(SubComp); + fixture.detectChanges(); + expect(fixture.componentInstance.foo).toBeAnInstanceOf(QueryList); + expect(fixture.componentInstance.foo.length).toBe(2); + }); + + it('should support ViewChildren query inherited from undecorated grand superclasses', () => { + @Directive({selector: '[some-dir]'}) + class SomeDir { + } + + class MySuperComp { + @ViewChildren(SomeDir) foo !: QueryList; + } + + class MyComp extends MySuperComp {} + + @Component({ + selector: 'sub-comp', + template: ` +
+
+ ` + }) + class SubComp extends MyComp { + } + + TestBed.configureTestingModule({declarations: [SubComp, SomeDir]}); + + const fixture = TestBed.createComponent(SubComp); + fixture.detectChanges(); + expect(fixture.componentInstance.foo).toBeAnInstanceOf(QueryList); + expect(fixture.componentInstance.foo.length).toBe(2); + }); + }); describe('content queries', () => { @@ -399,6 +489,118 @@ describe('query logic', () => { expect(secondComponent.setEvents).toEqual(['textDir set', 'foo set']); }); + it('should support ContentChild query inherited from undecorated superclasses', () => { + class MyComp { + @ContentChild('foo') foo: any; + } + + @Component({selector: 'sub-comp', template: ''}) + class SubComp extends MyComp { + } + + @Component({template: '
'}) + class App { + @ViewChild(SubComp) subComp !: SubComp; + } + + TestBed.configureTestingModule({declarations: [App, SubComp]}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + + expect(fixture.componentInstance.subComp.foo).toBeAnInstanceOf(ElementRef); + }); + + it('should support ContentChild query inherited from undecorated grand superclasses', () => { + class MySuperComp { + @ContentChild('foo') foo: any; + } + + class MyComp extends MySuperComp {} + + @Component({selector: 'sub-comp', template: ''}) + class SubComp extends MyComp { + } + + @Component({template: '
'}) + class App { + @ViewChild(SubComp) subComp !: SubComp; + } + + TestBed.configureTestingModule({declarations: [App, SubComp]}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + + expect(fixture.componentInstance.subComp.foo).toBeAnInstanceOf(ElementRef); + }); + + it('should support ContentChildren query inherited from undecorated superclasses', () => { + @Directive({selector: '[some-dir]'}) + class SomeDir { + } + + class MyComp { + @ContentChildren(SomeDir) foo !: QueryList; + } + + @Component({selector: 'sub-comp', template: ''}) + class SubComp extends MyComp { + } + + @Component({ + template: ` + +
+
+ + ` + }) + class App { + @ViewChild(SubComp) subComp !: SubComp; + } + + TestBed.configureTestingModule({declarations: [App, SubComp, SomeDir]}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + + expect(fixture.componentInstance.subComp.foo).toBeAnInstanceOf(QueryList); + expect(fixture.componentInstance.subComp.foo.length).toBe(2); + }); + + it('should support ContentChildren query inherited from undecorated grand superclasses', () => { + @Directive({selector: '[some-dir]'}) + class SomeDir { + } + + class MySuperComp { + @ContentChildren(SomeDir) foo !: QueryList; + } + + class MyComp extends MySuperComp {} + + @Component({selector: 'sub-comp', template: ''}) + class SubComp extends MyComp { + } + + @Component({ + template: ` + +
+
+ + ` + }) + class App { + @ViewChild(SubComp) subComp !: SubComp; + } + + TestBed.configureTestingModule({declarations: [App, SubComp, SomeDir]}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + + expect(fixture.componentInstance.subComp.foo).toBeAnInstanceOf(QueryList); + expect(fixture.componentInstance.subComp.foo.length).toBe(2); + }); + }); // Some root components may have ContentChildren queries if they are also diff --git a/packages/core/test/render3/ivy/jit_spec.ts b/packages/core/test/render3/ivy/jit_spec.ts index 5cd0368a8ed49..6fb577103064a 100644 --- a/packages/core/test/render3/ivy/jit_spec.ts +++ b/packages/core/test/render3/ivy/jit_spec.ts @@ -283,32 +283,6 @@ ivyEnabled && describe('render3 jit', () => { expect(InputDirAny.ngDirectiveDef.declaredInputs).toEqual({publicName: 'privateName'}); }); - it('should add ngBaseDef to types with @Input properties', () => { - class C { - @Input('alias1') - prop1 = 'test'; - - @Input('alias2') - prop2 = 'test'; - } - - expect((C as any).ngBaseDef).toBeDefined(); - expect((C as any).ngBaseDef.inputs).toEqual({prop1: 'alias1', prop2: 'alias2'}); - }); - - it('should add ngBaseDef to types with @Output properties', () => { - class C { - @Output('alias1') - prop1 = 'test'; - - @Output('alias2') - prop2 = 'test'; - } - - expect((C as any).ngBaseDef).toBeDefined(); - expect((C as any).ngBaseDef.outputs).toEqual({prop1: 'alias1', prop2: 'alias2'}); - }); - it('should compile ContentChildren query with string predicate on a directive', () => { @Directive({selector: '[test]'}) class TestDirective { diff --git a/tools/public_api_guard/core/core.d.ts b/tools/public_api_guard/core/core.d.ts index 843a68282d4a5..6bf194273bac7 100644 --- a/tools/public_api_guard/core/core.d.ts +++ b/tools/public_api_guard/core/core.d.ts @@ -662,6 +662,7 @@ export interface OutputDecorator { export declare function ɵɵallocHostVars(count: number): void; export interface ɵɵBaseDef { + contentQueries: ContentQueriesFunction | null; /** @deprecated */ readonly declaredInputs: { [P in keyof T]: string; }; @@ -671,6 +672,7 @@ export interface ɵɵBaseDef { readonly outputs: { [P in keyof T]: string; }; + viewQuery: ViewQueriesFunction | null; } export declare function ɵɵbind(value: T): T | NO_CHANGE; @@ -702,6 +704,8 @@ export declare function ɵɵdefineBase(baseDefinition: { outputs?: { [P in keyof T]?: string; }; + contentQueries?: ContentQueriesFunction | null; + viewQuery?: ViewQueriesFunction | null; }): ɵɵBaseDef; export declare function ɵɵdefineComponent(componentDefinition: {