honeymoose
/
angular-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(ivy): support abstract directives in template type checking (#33131) 

Recently it was made possible to have a directive without selector,
which are referred to as abstract directives. Such directives should not
be registered in an NgModule, but can still contain decorators for
inputs, outputs, queries, etc. The information from these decorators and
the `@Directive()` decorator itself needs to be registered with the
central `MetadataRegistry` so that other areas of the compiler can
request information about a given directive, an example of which is the
template type checker that needs to know about the inputs and outputs of
directives.

Prior to this change, however, abstract directives would only register
themselves with the `MetadataRegistry` as being an abstract directive,
without all of its other metadata like inputs and outputs. This meant
that the template type checker was unable to resolve the inputs and
outputs of these abstract directives, therefore failing to check them
correctly. The typical error would be that some property does not exist
on a DOM element, whereas said property should have been bound to the
abstract directive's input.

This commit fixes the problem by always registering the metadata of a
directive or component with the `MetadataRegistry`. Tests have been
added to ensure abstract directives are handled correctly in the
template type checker, together with tests to verify the form of
abstract directives in declaration files.

Fixes #30080

PR Close #33131
This commit is contained in:
JoostK 2019-10-13 17:00:13 +02:00 committed by Andrew Kushnir
parent 9a5e08f2a7
commit a42057d0f8
14 changed files with 166 additions and 110 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

50
packages/compiler-cli/src/ngtsc/annotations/src/component.ts
View File

@ -216,24 +216,20 @@ export class ComponentDecoratorHandler implements
`Errors parsing template: ${template.errors.map(e => e.toString()).join(', ')}`);
}
// If the component has a selector, it should be registered with the
// `LocalModuleScopeRegistry`
// so that when this component appears in an `@NgModule` scope, its selector can be
// determined.
if (metadata.selector !== null) {
const ref = new Reference(node);
this.metaRegistry.registerDirectiveMetadata({
ref,
name: node.name.text,
selector: metadata.selector,
exportAs: metadata.exportAs,
inputs: metadata.inputs,
outputs: metadata.outputs,
queries: metadata.queries.map(query => query.propertyName),
isComponent: true, ...extractDirectiveGuards(node, this.reflector),
baseClass: readBaseClass(node, this.reflector, this.evaluator),
});
}
// Register this component's information with the `MetadataRegistry`. This ensures that
// the information about the component is available during the compile() phase.
const ref = new Reference(node);
this.metaRegistry.registerDirectiveMetadata({
ref,
name: node.name.text,
selector: metadata.selector,
exportAs: metadata.exportAs,
inputs: metadata.inputs,
outputs: metadata.outputs,
queries: metadata.queries.map(query => query.propertyName),
isComponent: true, ...extractDirectiveGuards(node, this.reflector),
baseClass: readBaseClass(node, this.reflector, this.evaluator),
});
// 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
@ -325,7 +321,9 @@ export class ComponentDecoratorHandler implements
const matcher = new SelectorMatcher<DirectiveMeta>();
if (scope !== null) {
for (const directive of scope.compilation.directives) {
matcher.addSelectables(CssSelector.parse(directive.selector), directive);
if (directive.selector !== null) {
matcher.addSelectables(CssSelector.parse(directive.selector), directive);
}
}
}
const binder = new R3TargetBinder(matcher);
@ -368,8 +366,10 @@ export class ComponentDecoratorHandler implements
const scope = this.scopeReader.getScopeForComponent(node);
if (scope !== null) {
for (const meta of scope.compilation.directives) {
const extMeta = flattenInheritedDirectiveMetadata(this.metaReader, meta.ref);
matcher.addSelectables(CssSelector.parse(meta.selector), extMeta);
if (meta.selector !== null) {
const extMeta = flattenInheritedDirectiveMetadata(this.metaReader, meta.ref);
matcher.addSelectables(CssSelector.parse(meta.selector), extMeta);
}
}
for (const {name, ref} of scope.compilation.pipes) {
if (!ts.isClassDeclaration(ref.node)) {
@ -422,9 +422,11 @@ export class ComponentDecoratorHandler implements
for (const dir of scope.compilation.directives) {
const {ref, selector} = dir;
const expression = this.refEmitter.emit(ref, context);
directives.push({selector, expression});
matcher.addSelectables(CssSelector.parse(selector), {...dir, expression});
if (selector !== null) {
const expression = this.refEmitter.emit(ref, context);
directives.push({selector, expression});
matcher.addSelectables(CssSelector.parse(selector), {...dir, expression});
}
}
const pipes = new Map<string, Expression>();
for (const pipe of scope.compilation.pipes) {

37
packages/compiler-cli/src/ngtsc/annotations/src/directive.ts
View File

@ -55,32 +55,25 @@ export class DirectiveDecoratorHandler implements
const directiveResult = extractDirectiveMetadata(
node, decorator, this.reflector, this.evaluator, this.defaultImportRecorder, this.isCore);
const analysis = directiveResult && directiveResult.metadata;
// If the directive has a selector, it should be registered with the `SelectorScopeRegistry` so
// when this directive appears in an `@NgModule` scope, its selector can be determined.
if (analysis && analysis.selector !== null) {
const ref = new Reference(node);
this.metaRegistry.registerDirectiveMetadata({
ref,
name: node.name.text,
selector: analysis.selector,
exportAs: analysis.exportAs,
inputs: analysis.inputs,
outputs: analysis.outputs,
queries: analysis.queries.map(query => query.propertyName),
isComponent: false, ...extractDirectiveGuards(node, this.reflector),
baseClass: readBaseClass(node, this.reflector, this.evaluator),
});
}
if (analysis && !analysis.selector) {
this.metaRegistry.registerAbstractDirective(node);
}
if (analysis === undefined) {
return {};
}
// Register this directive's information with the `MetadataRegistry`. This ensures that
// the information about the directive is available during the compile() phase.
const ref = new Reference(node);
this.metaRegistry.registerDirectiveMetadata({
ref,
name: node.name.text,
selector: analysis.selector,
exportAs: analysis.exportAs,
inputs: analysis.inputs,
outputs: analysis.outputs,
queries: analysis.queries.map(query => query.propertyName),
isComponent: false, ...extractDirectiveGuards(node, this.reflector),
baseClass: readBaseClass(node, this.reflector, this.evaluator),
});
return {
analysis: {
meta: analysis,

4
packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts
View File

@ -262,7 +262,9 @@ export class NgModuleDecoratorHandler implements DecoratorHandler<NgModuleAnalys
}
for (const decl of analysis.declarations) {
if (this.metaReader.isAbstractDirective(decl)) {
const metadata = this.metaReader.getDirectiveMetadata(decl);
if (metadata !== null && metadata.selector === null) {
throw new FatalDiagnosticError(
ErrorCode.DIRECTIVE_MISSING_SELECTOR, decl.node,
`Directive ${decl.node.name.text} has no selector, please add it!`);

14
packages/compiler-cli/src/ngtsc/incremental/src/state.ts
View File

@ -93,19 +93,6 @@ export class IncrementalState implements DependencyTracker, MetadataReader, Meta
metadata.ngModuleMeta.set(meta.ref.node, meta);
}
isAbstractDirective(ref: Reference<ClassDeclaration>): boolean {
if (!this.metadata.has(ref.node.getSourceFile())) {
return false;
}
const metadata = this.metadata.get(ref.node.getSourceFile()) !;
return metadata.abstractDirectives.has(ref.node);
}
registerAbstractDirective(clazz: ClassDeclaration): void {
const metadata = this.ensureMetadata(clazz.getSourceFile());
metadata.abstractDirectives.add(clazz);
}
getDirectiveMetadata(ref: Reference<ClassDeclaration>): DirectiveMeta|null {
if (!this.metadata.has(ref.node.getSourceFile())) {
return null;
@ -200,7 +187,6 @@ class FileMetadata {
/** A set of source files that this file depends upon. */
fileDependencies = new Set<ts.SourceFile>();
resourcePaths = new Set<string>();
abstractDirectives = new Set<ClassDeclaration>();
directiveMeta = new Map<ClassDeclaration, DirectiveMeta>();
ngModuleMeta = new Map<ClassDeclaration, NgModuleMeta>();
pipeMeta = new Map<ClassDeclaration, PipeMeta>();

4
packages/compiler-cli/src/ngtsc/indexer/src/context.ts
View File

@ -13,9 +13,9 @@ import {ClassDeclaration} from '../../reflection';
export interface ComponentMeta extends DirectiveMeta {
ref: Reference<ClassDeclaration>;
/**
* Unparsed selector of the directive.
* Unparsed selector of the directive, or null if the directive does not have a selector.
*/
selector: string;
selector: string|null;
}
/**

6
packages/compiler-cli/src/ngtsc/metadata/src/api.ts
View File

@ -28,9 +28,9 @@ export interface NgModuleMeta {
export interface DirectiveMeta extends T2DirectiveMeta {
ref: Reference<ClassDeclaration>;
/**
* Unparsed selector of the directive.
* Unparsed selector of the directive, or null if the directive does not have a selector.
*/
selector: string;
selector: string|null;
queries: string[];
ngTemplateGuards: TemplateGuardMeta[];
hasNgTemplateContextGuard: boolean;
@ -77,7 +77,6 @@ export interface PipeMeta {
* or a registry.
*/
export interface MetadataReader {
isAbstractDirective(node: Reference<ClassDeclaration>): boolean;
getDirectiveMetadata(node: Reference<ClassDeclaration>): DirectiveMeta|null;
getNgModuleMetadata(node: Reference<ClassDeclaration>): NgModuleMeta|null;
getPipeMetadata(node: Reference<ClassDeclaration>): PipeMeta|null;
@ -87,7 +86,6 @@ export interface MetadataReader {
* Registers new metadata for directives, pipes, and modules.
*/
export interface MetadataRegistry {
registerAbstractDirective(clazz: ClassDeclaration): void;
registerDirectiveMetadata(meta: DirectiveMeta): void;
registerNgModuleMetadata(meta: NgModuleMeta): void;
registerPipeMetadata(meta: PipeMeta): void;

12
packages/compiler-cli/src/ngtsc/metadata/src/dts.ts
View File

@ -21,11 +21,6 @@ import {extractDirectiveGuards, extractReferencesFromType, readStringArrayType,
export class DtsMetadataReader implements MetadataReader {
constructor(private checker: ts.TypeChecker, private reflector: ReflectionHost) {}
isAbstractDirective(ref: Reference<ClassDeclaration>): boolean {
const meta = this.getDirectiveMetadata(ref);
return meta !== null && meta.selector === null;
}
/**
* Read the metadata from a class that has already been compiled somehow (either it's in a .d.ts
* file, or in a .ts file with a handwritten definition).
@ -79,15 +74,12 @@ export class DtsMetadataReader implements MetadataReader {
// The type metadata was the wrong shape.
return null;
}
const selector = readStringType(def.type.typeArguments[1]);
if (selector === null) {
return null;
}
return {
ref,
name: clazz.name.text,
isComponent: def.name === 'ɵcmp', selector,
isComponent: def.name === 'ɵcmp',
selector: readStringType(def.type.typeArguments[1]),
exportAs: readStringArrayType(def.type.typeArguments[2]),
inputs: readStringMapType(def.type.typeArguments[3]),
outputs: readStringMapType(def.type.typeArguments[4]),

11
packages/compiler-cli/src/ngtsc/metadata/src/registry.ts
View File

@ -16,14 +16,10 @@ import {DirectiveMeta, MetadataReader, MetadataRegistry, NgModuleMeta, PipeMeta}
* unit, which supports both reading and registering.
*/
export class LocalMetadataRegistry implements MetadataRegistry, MetadataReader {
private abstractDirectives = new Set<ClassDeclaration>();
private directives = new Map<ClassDeclaration, DirectiveMeta>();
private ngModules = new Map<ClassDeclaration, NgModuleMeta>();
private pipes = new Map<ClassDeclaration, PipeMeta>();
isAbstractDirective(ref: Reference<ClassDeclaration>): boolean {
return this.abstractDirectives.has(ref.node);
}
getDirectiveMetadata(ref: Reference<ClassDeclaration>): DirectiveMeta|null {
return this.directives.has(ref.node) ? this.directives.get(ref.node) ! : null;
}
@ -34,7 +30,6 @@ export class LocalMetadataRegistry implements MetadataRegistry, MetadataReader {
return this.pipes.has(ref.node) ? this.pipes.get(ref.node) ! : null;
}
registerAbstractDirective(clazz: ClassDeclaration): void { this.abstractDirectives.add(clazz); }
registerDirectiveMetadata(meta: DirectiveMeta): void { this.directives.set(meta.ref.node, meta); }
registerNgModuleMetadata(meta: NgModuleMeta): void { this.ngModules.set(meta.ref.node, meta); }
registerPipeMetadata(meta: PipeMeta): void { this.pipes.set(meta.ref.node, meta); }
@ -46,12 +41,6 @@ export class LocalMetadataRegistry implements MetadataRegistry, MetadataReader {
export class CompoundMetadataRegistry implements MetadataRegistry {
constructor(private registries: MetadataRegistry[]) {}
registerAbstractDirective(clazz: ClassDeclaration) {
for (const registry of this.registries) {
registry.registerAbstractDirective(clazz);
}
}
registerDirectiveMetadata(meta: DirectiveMeta): void {
for (const registry of this.registries) {
registry.registerDirectiveMetadata(meta);

4
packages/compiler-cli/src/ngtsc/metadata/src/util.ts
View File

@ -137,10 +137,6 @@ function extractCoercedInput(member: ClassMember): string|null {
export class CompoundMetadataReader implements MetadataReader {
constructor(private readers: MetadataReader[]) {}
isAbstractDirective(node: Reference<ClassDeclaration>): boolean {
return this.readers.some(r => r.isAbstractDirective(node));
}
getDirectiveMetadata(node: Reference<ClassDeclaration<ts.Declaration>>): DirectiveMeta|null {
for (const reader of this.readers) {
const meta = reader.getDirectiveMetadata(node);

2
packages/compiler-cli/src/ngtsc/scope/src/local.ts
View File

@ -118,8 +118,6 @@ export class LocalModuleScopeRegistry implements MetadataRegistry, ComponentScop
}
}
registerAbstractDirective(clazz: ClassDeclaration): void {}
registerDirectiveMetadata(directive: DirectiveMeta): void {}
registerPipeMetadata(pipe: PipeMeta): void {}

44
packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
View File

@ -191,6 +191,50 @@ runInEachFileSystem(os => {
expect(jsContents).toContain('inject(Dep, 8)');
});
it('should compile Directives without errors', () => {
env.write('test.ts', `
import {Directive} from '@angular/core';
@Directive({selector: '[dir]'})
export class TestDir {}
`);
env.driveMain();
const jsContents = env.getContents('test.js');
expect(jsContents).toContain('TestDir.ɵdir = i0.ɵɵdefineDirective');
expect(jsContents).toContain('TestDir.ɵfac = function');
expect(jsContents).not.toContain('__decorate');
const dtsContents = env.getContents('test.d.ts');
expect(dtsContents)
.toContain(
'static ɵdir: i0.ɵɵDirectiveDefWithMeta<TestDir, "[dir]", never, {}, {}, never>');
expect(dtsContents).toContain('static ɵfac: i0.ɵɵFactoryDef<TestDir>');
});
it('should compile abstract Directives without errors', () => {
env.write('test.ts', `
import {Directive} from '@angular/core';
@Directive()
export class TestDir {}
`);
env.driveMain();
const jsContents = env.getContents('test.js');
expect(jsContents).toContain('TestDir.ɵdir = i0.ɵɵdefineDirective');
expect(jsContents).toContain('TestDir.ɵfac = function');
expect(jsContents).not.toContain('__decorate');
const dtsContents = env.getContents('test.d.ts');
expect(dtsContents)
.toContain(
'static ɵdir: i0.ɵɵDirectiveDefWithMeta<TestDir, never, never, {}, {}, never>');
expect(dtsContents).toContain('static ɵfac: i0.ɵɵFactoryDef<TestDir>');
});
it('should compile Components (inline template) without errors', () => {
env.write('test.ts', `
import {Component} from '@angular/core';

78
packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts
View File

@ -370,10 +370,15 @@ export declare class CommonModule {
env.write('test.ts', `
import {Component, Directive, Input, NgModule} from '@angular/core';
@Directive()
class AbstractDir {
@Input() fromAbstract!: number;
}
@Directive({
selector: '[base]',
})
class BaseDir {
class BaseDir extends AbstractDir {
@Input() fromBase!: string;
}
@ -386,7 +391,7 @@ export declare class CommonModule {
@Component({
selector: 'test',
template: '<div child [fromBase]="3" [fromChild]="4"></div>',
template: '<div child [fromAbstract]="true" [fromBase]="3" [fromChild]="4"></div>',
})
class TestCmp {}
@ -397,13 +402,68 @@ export declare class CommonModule {
`);
const diags = env.driveDiagnostics();
expect(diags.length).toBe(2);
expect(diags[0].messageText).toBe(`Type 'number' is not assignable to type 'string'.`);
expect(diags[0].start).toEqual(386);
expect(diags[0].length).toEqual(14);
expect(diags[1].messageText).toBe(`Type 'number' is not assignable to type 'boolean'.`);
expect(diags[1].start).toEqual(401);
expect(diags[1].length).toEqual(15);
expect(diags.length).toBe(3);
expect(diags[0].messageText).toBe(`Type 'true' is not assignable to type 'number'.`);
expect(getSourceCodeForDiagnostic(diags[0])).toEqual('[fromAbstract]="true"');
expect(diags[1].messageText).toBe(`Type 'number' is not assignable to type 'string'.`);
expect(getSourceCodeForDiagnostic(diags[1])).toEqual('[fromBase]="3"');
expect(diags[2].messageText).toBe(`Type 'number' is not assignable to type 'boolean'.`);
expect(getSourceCodeForDiagnostic(diags[2])).toEqual('[fromChild]="4"');
});
it('should properly type-check inherited directives from external libraries', () => {
env.write('node_modules/external/index.d.ts', `
import * as i0 from '@angular/core';
export declare class AbstractDir {
fromAbstract: number;
static ɵdir: i0.ɵɵDirectiveDefWithMeta<AbstractDir, never, never, {'fromAbstract': 'fromAbstract'}, never, never>;
}
export declare class BaseDir extends AbstractDir {
fromBase: string;
static ɵdir: i0.ɵɵDirectiveDefWithMeta<BaseDir, '[base]', never, {'fromBase': 'fromBase'}, never, never>;
}
export declare class ExternalModule {
static ɵmod: i0.ɵɵNgModuleDefWithMeta<ExternalModule, [typeof BaseDir], never, [typeof BaseDir]>;
}
`);
env.write('test.ts', `
import {Component, Directive, Input, NgModule} from '@angular/core';
import {BaseDir, ExternalModule} from 'external';
@Directive({
selector: '[child]',
})
class ChildDir extends BaseDir {
@Input() fromChild!: boolean;
}
@Component({
selector: 'test',
template: '<div child [fromAbstract]="true" [fromBase]="3" [fromChild]="4"></div>',
})
class TestCmp {}
@NgModule({
declarations: [TestCmp, ChildDir],
imports: [ExternalModule],
})
class Module {}
`);
const diags = env.driveDiagnostics();
expect(diags.length).toBe(3);
expect(diags[0].messageText).toBe(`Type 'true' is not assignable to type 'number'.`);
expect(getSourceCodeForDiagnostic(diags[0])).toEqual('[fromAbstract]="true"');
expect(diags[1].messageText).toBe(`Type 'number' is not assignable to type 'string'.`);
expect(getSourceCodeForDiagnostic(diags[1])).toEqual('[fromBase]="3"');
expect(diags[2].messageText).toBe(`Type 'number' is not assignable to type 'boolean'.`);
expect(getSourceCodeForDiagnostic(diags[2])).toEqual('[fromChild]="4"');
});
describe('input coercion', () => {

8
packages/compiler/src/render3/view/compiler.ts
View File

@ -304,10 +304,6 @@ export function compileComponentFromMetadata(
definitionMap.set('changeDetection', o.literal(changeDetection));
}
// On the type side, remove newlines from the selector as it will need to fit into a TypeScript
// string literal, which must be on one line.
const selectorForType = (meta.selector || '').replace(/\n/g, '');
const expression = o.importExpr(R3.defineComponent).callFn([definitionMap.toLiteralMap()]);
const type = createTypeForDef(meta, R3.ComponentDefWithMeta);
@ -538,11 +534,11 @@ function stringArrayAsType(arr: string[]): o.Type {
function createTypeForDef(meta: R3DirectiveMetadata, typeBase: o.ExternalReference): o.Type {
// On the type side, remove newlines from the selector as it will need to fit into a TypeScript
// string literal, which must be on one line.
const selectorForType = (meta.selector || '').replace(/\n/g, '');
const selectorForType = meta.selector !== null ? meta.selector.replace(/\n/g, '') : null;
return o.expressionType(o.importExpr(typeBase, [
typeWithParameters(meta.type, meta.typeArgumentCount),
stringAsType(selectorForType),
selectorForType !== null ? stringAsType(selectorForType) : o.NONE_TYPE,
meta.exportAs !== null ? stringArrayAsType(meta.exportAs) : o.NONE_TYPE,
stringMapAsType(meta.inputs),
stringMapAsType(meta.outputs),

2
packages/core/src/render3/jit/directive.ts
View File

@ -192,7 +192,7 @@ export function directiveMetadata(type: Type<any>, metadata: Directive): R3Direc
name: type.name,
type: type,
typeArgumentCount: 0,
selector: metadata.selector !,
selector: metadata.selector !== undefined ? metadata.selector : null,
deps: reflectDependencies(type),
host: metadata.host || EMPTY_OBJ,
propMetadata: propMetadata,