From 405515091040759c1e65e79b0dde3ab77e04975e Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Mon, 1 Jul 2019 16:04:58 -0700 Subject: [PATCH] feat(compiler): allow selector-less directives as base classes (#31379) In Angular today, the following pattern works: ```typescript export class BaseDir { constructor(@Inject(ViewContainerRef) protected vcr: ViewContainerRef) {} } @Directive({ selector: '[child]', }) export class ChildDir extends BaseDir { // constructor inherited from BaseDir } ``` A decorated child class can inherit a constructor from an undecorated base class, so long as the base class has metadata of its own (for JIT mode). This pattern works regardless of metadata in AOT. In Angular Ivy, this pattern does not work: without the @Directive annotation identifying the base class as a directive, information about its constructor parameters will not be captured by the Ivy compiler. This is a result of Ivy's locality principle, which is the basis behind a number of compilation optimizations. As a solution, @Directive() without a selector will be interpreted as a "directive base class" annotation. Such a directive cannot be declared in an NgModule, but can be inherited from. To implement this, a few changes are made to the ngc compiler: * the error for a selector-less directive is now generated when an NgModule declaring it is processed, not when the directive itself is processed. * selector-less directives are not tracked along with other directives in the compiler, preventing other errors (like their absence in an NgModule) from being generated from them. PR Close #31379 --- packages/compiler-cli/test/ngc_spec.ts | 24 +++++++++++++++++++ packages/compiler/src/aot/compiler.ts | 26 ++++++++++++++++++--- packages/compiler/src/metadata_resolver.ts | 27 ++++++++++++++++++---- packages/core/src/metadata/directives.ts | 4 ++-- tools/public_api_guard/core/core.d.ts | 4 ++-- 5 files changed, 73 insertions(+), 12 deletions(-) diff --git a/packages/compiler-cli/test/ngc_spec.ts b/packages/compiler-cli/test/ngc_spec.ts index 8a256af8f9..9d3a59974d 100644 --- a/packages/compiler-cli/test/ngc_spec.ts +++ b/packages/compiler-cli/test/ngc_spec.ts @@ -2283,4 +2283,28 @@ describe('ngc transformer command-line', () => { let exitCode = main(['-p', path.join(basePath, 'tsconfig.json')], errorSpy); expect(exitCode).toEqual(0); }); + + describe('base directives', () => { + it('should allow directives with no selector that are not in NgModules', () => { + // first only generate .d.ts / .js / .metadata.json files + writeConfig(`{ + "extends": "./tsconfig-base.json", + "files": ["main.ts"] + }`); + write('main.ts', ` + import {Directive} from '@angular/core'; + + @Directive({}) + export class BaseDir {} + + @Directive({}) + export abstract class AbstractBaseDir {} + + @Directive() + export abstract class EmptyDir {} + `); + let exitCode = main(['-p', path.join(basePath, 'tsconfig.json')], errorSpy); + expect(exitCode).toEqual(0); + }); + }); }); diff --git a/packages/compiler/src/aot/compiler.ts b/packages/compiler/src/aot/compiler.ts index f7b5c59e25..5a3f0c3278 100644 --- a/packages/compiler/src/aot/compiler.ts +++ b/packages/compiler/src/aot/compiler.ts @@ -797,6 +797,7 @@ export interface NgAnalyzedFileWithInjectables { export interface NgAnalyzedFile { fileName: string; directives: StaticSymbol[]; + abstractDirectives: StaticSymbol[]; pipes: StaticSymbol[]; ngModules: CompileNgModuleMetadata[]; injectables: CompileInjectableMetadata[]; @@ -857,18 +858,20 @@ function _analyzeFilesIncludingNonProgramFiles( export function analyzeFile( host: NgAnalyzeModulesHost, staticSymbolResolver: StaticSymbolResolver, metadataResolver: CompileMetadataResolver, fileName: string): NgAnalyzedFile { + const abstractDirectives: StaticSymbol[] = []; const directives: StaticSymbol[] = []; const pipes: StaticSymbol[] = []; const injectables: CompileInjectableMetadata[] = []; const ngModules: CompileNgModuleMetadata[] = []; const hasDecorators = staticSymbolResolver.hasDecorators(fileName); let exportsNonSourceFiles = false; + const isDeclarationFile = fileName.endsWith('.d.ts'); // Don't analyze .d.ts files that have no decorators as a shortcut // to speed up the analysis. This prevents us from // resolving the references in these files. // Note: exportsNonSourceFiles is only needed when compiling with summaries, // which is not the case when .d.ts files are treated as input files. - if (!fileName.endsWith('.d.ts') || hasDecorators) { + if (!isDeclarationFile || hasDecorators) { staticSymbolResolver.getSymbolsOf(fileName).forEach((symbol) => { const resolvedSymbol = staticSymbolResolver.resolveSymbol(symbol); const symbolMeta = resolvedSymbol.metadata; @@ -879,7 +882,23 @@ export function analyzeFile( if (symbolMeta.__symbolic === 'class') { if (metadataResolver.isDirective(symbol)) { isNgSymbol = true; - directives.push(symbol); + if (!isDeclarationFile) { + // This directive either has a selector or doesn't. Selector-less directives get tracked + // in abstractDirectives, not directives. The compiler doesn't deal with selector-less + // directives at all, really, other than to persist their metadata. This is done so that + // apps will have an easier time migrating to Ivy, which requires the selector-less + // annotations to be applied. + if (!metadataResolver.isAbstractDirective(symbol)) { + // The directive is an ordinary directive. + directives.push(symbol); + } else { + // The directive has no selector and is an "abstract" directive, so track it + // accordingly. + abstractDirectives.push(symbol); + } + } else { + directives.push(symbol); + } } else if (metadataResolver.isPipe(symbol)) { isNgSymbol = true; pipes.push(symbol); @@ -904,7 +923,8 @@ export function analyzeFile( }); } return { - fileName, directives, pipes, ngModules, injectables, exportsNonSourceFiles, + fileName, directives, abstractDirectives, pipes, + ngModules, injectables, exportsNonSourceFiles, }; } diff --git a/packages/compiler/src/metadata_resolver.ts b/packages/compiler/src/metadata_resolver.ts index 6c2f2d6fff..1cee4dbb2e 100644 --- a/packages/compiler/src/metadata_resolver.ts +++ b/packages/compiler/src/metadata_resolver.ts @@ -348,11 +348,7 @@ export class CompileMetadataResolver { } else { // Directive if (!selector) { - this._reportError( - syntaxError( - `Directive ${stringifyType(directiveType)} has no selector, please add it!`), - directiveType); - selector = 'error'; + selector = null !; } } @@ -432,6 +428,21 @@ export class CompileMetadataResolver { this._directiveResolver.isDirective(type); } + isAbstractDirective(type: any): boolean { + const summary = + this._loadSummary(type, cpl.CompileSummaryKind.Directive) as cpl.CompileDirectiveSummary; + if (summary && !summary.isComponent) { + return !summary.selector; + } + + const meta = this._directiveResolver.resolve(type, false); + if (meta && !createComponent.isTypeOf(meta)) { + return !meta.selector; + } + + return false; + } + isPipe(type: any) { return !!this._loadSummary(type, cpl.CompileSummaryKind.Pipe) || this._pipeResolver.isPipe(type); @@ -607,6 +618,12 @@ export class CompileMetadataResolver { } const declaredIdentifier = this._getIdentifierMetadata(declaredType); if (this.isDirective(declaredType)) { + if (this.isAbstractDirective(declaredType)) { + this._reportError( + syntaxError( + `Directive ${stringifyType(declaredType)} has no selector, please add it!`), + declaredType); + } transitiveModule.addDirective(declaredIdentifier); declaredDirectives.push(declaredIdentifier); this._addTypeToModule(declaredType, moduleType); diff --git a/packages/core/src/metadata/directives.ts b/packages/core/src/metadata/directives.ts index aed0867528..e95cbbddcf 100644 --- a/packages/core/src/metadata/directives.ts +++ b/packages/core/src/metadata/directives.ts @@ -68,12 +68,12 @@ export interface DirectiveDecorator { * * @Annotation */ - (obj: Directive): TypeDecorator; + (obj?: Directive): TypeDecorator; /** * See the `Directive` decorator. */ - new (obj: Directive): Directive; + new (obj?: Directive): Directive; } /** diff --git a/tools/public_api_guard/core/core.d.ts b/tools/public_api_guard/core/core.d.ts index 5ae9323bf3..e6eba1a481 100644 --- a/tools/public_api_guard/core/core.d.ts +++ b/tools/public_api_guard/core/core.d.ts @@ -297,8 +297,8 @@ export interface Directive { export declare const Directive: DirectiveDecorator; export interface DirectiveDecorator { - (obj: Directive): TypeDecorator; - new (obj: Directive): Directive; + (obj?: Directive): TypeDecorator; + new (obj?: Directive): Directive; } export interface DoBootstrap {