Skip to content

Commit

Permalink
fix(ivy): queries not being inherited from undecorated classes (#30015)
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@crisbeto @AndrewKushnir
crisbeto authored and AndrewKushnir committed Apr 24, 2019
1 parent 8ca208f commit c7f1b0a
Show file tree
Hide file tree
Showing 15 changed files with 688 additions and 140 deletions.
54 changes: 40 additions & 14 deletions packages/compiler-cli/src/ngtsc/annotations/src/base_def.ts
View file
Expand Up @@ -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 {
Expand Down Expand Up @@ -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});
}
}
});
Expand Down Expand Up @@ -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',
Expand All @@ -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[]}[];
}
187 changes: 187 additions & 0 deletions packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts
View file
Expand Up @@ -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: {
Expand Down Expand Up @@ -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<SomeDirective>;
}
@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<SomeDirective>;
}
@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: {
Expand Down
9 changes: 9 additions & 0 deletions packages/compiler/src/compiler_facade_interface.ts
View file
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down
18 changes: 16 additions & 2 deletions packages/compiler/src/jit_compiler_facade.ts
View file
Expand Up @@ -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';
Expand All @@ -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';
Expand Down Expand Up @@ -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);
}
Expand Down

0 comments on commit c7f1b0a

Please sign in to comment.