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
This commit is contained in:
Alex Rickabaugh 2019-07-01 16:04:58 -07:00 committed by Andrew Kushnir
parent 4d436800de
commit 4055150910
5 changed files with 73 additions and 12 deletions

View File

@ -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);
});
});
});

View File

@ -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,
};
}

View File

@ -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);

View File

@ -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;
}
/**

View File

@ -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 {