diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts index 62b3571031f1f..4578da8b48e02 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts @@ -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(); 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(); for (const pipe of scope.compilation.pipes) { diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts b/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts index 1704e2aeb4db0..d4cd46cabdffb 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts @@ -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, diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts b/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts index 7a814e7bec225..c40d354581d81 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts @@ -262,7 +262,9 @@ export class NgModuleDecoratorHandler implements DecoratorHandler): 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): 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(); resourcePaths = new Set(); - abstractDirectives = new Set(); directiveMeta = new Map(); ngModuleMeta = new Map(); pipeMeta = new Map(); diff --git a/packages/compiler-cli/src/ngtsc/indexer/src/context.ts b/packages/compiler-cli/src/ngtsc/indexer/src/context.ts index 32991eab073b8..1881ec01f0ce4 100644 --- a/packages/compiler-cli/src/ngtsc/indexer/src/context.ts +++ b/packages/compiler-cli/src/ngtsc/indexer/src/context.ts @@ -13,9 +13,9 @@ import {ClassDeclaration} from '../../reflection'; export interface ComponentMeta extends DirectiveMeta { ref: Reference; /** - * 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; } /** diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/api.ts b/packages/compiler-cli/src/ngtsc/metadata/src/api.ts index 6739226478141..bde0ee4c71794 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/api.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/api.ts @@ -28,9 +28,9 @@ export interface NgModuleMeta { export interface DirectiveMeta extends T2DirectiveMeta { ref: Reference; /** - * 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): boolean; getDirectiveMetadata(node: Reference): DirectiveMeta|null; getNgModuleMetadata(node: Reference): NgModuleMeta|null; getPipeMetadata(node: Reference): 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; diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts b/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts index bb4a12c6344a3..2559fd6a35357 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts @@ -21,11 +21,6 @@ import {extractDirectiveGuards, extractReferencesFromType, readStringArrayType, export class DtsMetadataReader implements MetadataReader { constructor(private checker: ts.TypeChecker, private reflector: ReflectionHost) {} - isAbstractDirective(ref: Reference): 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]), diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/registry.ts b/packages/compiler-cli/src/ngtsc/metadata/src/registry.ts index c4d7ec05e6712..f834218efd2e2 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/registry.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/registry.ts @@ -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(); private directives = new Map(); private ngModules = new Map(); private pipes = new Map(); - isAbstractDirective(ref: Reference): boolean { - return this.abstractDirectives.has(ref.node); - } getDirectiveMetadata(ref: Reference): 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); diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/util.ts b/packages/compiler-cli/src/ngtsc/metadata/src/util.ts index d49a9540130fc..a3982fd37c247 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/util.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/util.ts @@ -137,10 +137,6 @@ function extractCoercedInput(member: ClassMember): string|null { export class CompoundMetadataReader implements MetadataReader { constructor(private readers: MetadataReader[]) {} - isAbstractDirective(node: Reference): boolean { - return this.readers.some(r => r.isAbstractDirective(node)); - } - getDirectiveMetadata(node: Reference>): DirectiveMeta|null { for (const reader of this.readers) { const meta = reader.getDirectiveMetadata(node); diff --git a/packages/compiler-cli/src/ngtsc/scope/src/local.ts b/packages/compiler-cli/src/ngtsc/scope/src/local.ts index 3d7cb2a90f7f0..94f9f7ad1cdc5 100644 --- a/packages/compiler-cli/src/ngtsc/scope/src/local.ts +++ b/packages/compiler-cli/src/ngtsc/scope/src/local.ts @@ -118,8 +118,6 @@ export class LocalModuleScopeRegistry implements MetadataRegistry, ComponentScop } } - registerAbstractDirective(clazz: ClassDeclaration): void {} - registerDirectiveMetadata(directive: DirectiveMeta): void {} registerPipeMetadata(pipe: PipeMeta): void {} diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index e5fc130ffebfb..a38c2c3e71d93 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -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'); + expect(dtsContents).toContain('static ɵfac: i0.ɵɵFactoryDef'); + }); + + 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'); + expect(dtsContents).toContain('static ɵfac: i0.ɵɵFactoryDef'); + }); + it('should compile Components (inline template) without errors', () => { env.write('test.ts', ` import {Component} from '@angular/core'; diff --git a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts index 85fe42130406b..6b0293592b3ab 100644 --- a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts +++ b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts @@ -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: '
', + template: '
', }) 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; + } + + export declare class BaseDir extends AbstractDir { + fromBase: string; + + static ɵdir: i0.ɵɵDirectiveDefWithMeta; + } + + export declare class ExternalModule { + static ɵmod: i0.ɵɵNgModuleDefWithMeta; + } + `); + + 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: '
', + }) + 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', () => { diff --git a/packages/compiler/src/render3/view/compiler.ts b/packages/compiler/src/render3/view/compiler.ts index 145d5734fe173..9769f31a5a1ab 100644 --- a/packages/compiler/src/render3/view/compiler.ts +++ b/packages/compiler/src/render3/view/compiler.ts @@ -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), diff --git a/packages/core/src/render3/jit/directive.ts b/packages/core/src/render3/jit/directive.ts index b1b7d75647370..a2c53e8186969 100644 --- a/packages/core/src/render3/jit/directive.ts +++ b/packages/core/src/render3/jit/directive.ts @@ -192,7 +192,7 @@ export function directiveMetadata(type: Type, 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,