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

fix(ivy): Support selector-less directive as base classes (#32125) 

Following #31379, this adds support for directives without a selector to
Ivy.

PR Close #32125
This commit is contained in:
atscott 2019-08-12 14:56:30 -07:00 committed by Andrew Kushnir
parent bb3c684b98
commit cfed0c0cf1
19 changed files with 169 additions and 79 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts
View File

@ -86,9 +86,9 @@ export class DecorationAnalyzer {
this.reflectionHost, NOOP_DEFAULT_IMPORT_RECORDER, this.isCore,
/* strictCtorDeps */ false),
new NgModuleDecoratorHandler(
this.reflectionHost, this.evaluator, this.fullRegistry, this.scopeRegistry,
this.referencesRegistry, this.isCore, /* routeAnalyzer */ null, this.refEmitter,
NOOP_DEFAULT_IMPORT_RECORDER),
this.reflectionHost, this.evaluator, this.fullMetaReader, this.fullRegistry,
this.scopeRegistry, this.referencesRegistry, this.isCore, /* routeAnalyzer */ null,
this.refEmitter, NOOP_DEFAULT_IMPORT_RECORDER),
new PipeDecoratorHandler(
this.reflectionHost, this.evaluator, this.metaRegistry, NOOP_DEFAULT_IMPORT_RECORDER,
this.isCore),

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

@ -72,6 +72,10 @@ export class DirectiveDecoratorHandler implements
});
}
if (analysis && !analysis.selector) {
this.metaRegistry.registerAbstractDirective(node);
}
if (analysis === undefined) {
return {};
}
@ -102,7 +106,10 @@ export class DirectiveDecoratorHandler implements
}
/**
* Helper function to extract metadata from a `Directive` or `Component`.
* Helper function to extract metadata from a `Directive` or `Component`. `Directive`s without a
* selector are allowed to be used for abstract base classes. These abstract directives should not
* appear in the declarations of an `NgModule` and additional verification is done when processing
* the module.
*/
export function extractDirectiveMetadata(
clazz: ClassDeclaration, decorator: Decorator, reflector: ReflectionHost,
@ -112,17 +119,22 @@ export function extractDirectiveMetadata(
metadata: R3DirectiveMetadata,
decoratedElements: ClassMember[],
}|undefined {
if (decorator.args === null || decorator.args.length !== 1) {
let directive: Map<string, ts.Expression>;
if (decorator.args === null || decorator.args.length === 0) {
directive = new Map<string, ts.Expression>();
} else if (decorator.args.length !== 1) {
throw new FatalDiagnosticError(
ErrorCode.DECORATOR_ARITY_WRONG, decorator.node,
`Incorrect number of arguments to @${decorator.name} decorator`);
} else {
const meta = unwrapExpression(decorator.args[0]);
if (!ts.isObjectLiteralExpression(meta)) {
throw new FatalDiagnosticError(
ErrorCode.DECORATOR_ARG_NOT_LITERAL, meta,
`@${decorator.name} argument must be literal.`);
}
directive = reflectObjectLiteral(meta);
}
const meta = unwrapExpression(decorator.args[0]);
if (!ts.isObjectLiteralExpression(meta)) {
throw new FatalDiagnosticError(
ErrorCode.DECORATOR_ARG_NOT_LITERAL, meta, `@${decorator.name} argument must be literal.`);
}
const directive = reflectObjectLiteral(meta);
if (directive.has('jit')) {
// The only allowed value is true, so there's no need to expand further.
@ -188,9 +200,11 @@ export function extractDirectiveMetadata(
}
// use default selector in case selector is an empty string
selector = resolved === '' ? defaultSelector : resolved;
}
if (!selector) {
throw new Error(`Directive ${clazz.name.text} has no selector, please add it!`);
if (!selector) {
throw new FatalDiagnosticError(
ErrorCode.DIRECTIVE_MISSING_SELECTOR, expr,
`Directive ${clazz.name.text} has no selector, please add it!`);
}
}
const host = extractHostBindings(decoratedElements, evaluator, coreModule, directive);

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

@ -11,7 +11,7 @@ import * as ts from 'typescript';
import {ErrorCode, FatalDiagnosticError} from '../../diagnostics';
import {DefaultImportRecorder, Reference, ReferenceEmitter} from '../../imports';
import {MetadataRegistry} from '../../metadata';
import {MetadataReader, MetadataRegistry} from '../../metadata';
import {PartialEvaluator, ResolvedValue} from '../../partial_evaluator';
import {ClassDeclaration, Decorator, ReflectionHost, reflectObjectLiteral, typeNodeToValueExpr} from '../../reflection';
import {NgModuleRouteAnalyzer} from '../../routing';
@ -40,7 +40,8 @@ export interface NgModuleAnalysis {
export class NgModuleDecoratorHandler implements DecoratorHandler<NgModuleAnalysis, Decorator> {
constructor(
private reflector: ReflectionHost, private evaluator: PartialEvaluator,
private metaRegistry: MetadataRegistry, private scopeRegistry: LocalModuleScopeRegistry,
private metaReader: MetadataReader, private metaRegistry: MetadataRegistry,
private scopeRegistry: LocalModuleScopeRegistry,
private referencesRegistry: ReferencesRegistry, private isCore: boolean,
private routeAnalyzer: NgModuleRouteAnalyzer|null, private refEmitter: ReferenceEmitter,
private defaultImportRecorder: DefaultImportRecorder, private localeId?: string) {}
@ -210,15 +211,23 @@ export class NgModuleDecoratorHandler implements DecoratorHandler<NgModuleAnalys
const scope = this.scopeRegistry.getScopeOfModule(node);
const diagnostics = this.scopeRegistry.getDiagnosticsOfModule(node) || undefined;
// Using the scope information, extend the injector's imports using the modules that are
// specified as module exports.
if (scope !== null) {
// Using the scope information, extend the injector's imports using the modules that are
// specified as module exports.
const context = getSourceFile(node);
for (const exportRef of analysis.exports) {
if (isNgModule(exportRef.node, scope.compilation)) {
analysis.ngInjectorDef.imports.push(this.refEmitter.emit(exportRef, context));
}
}
for (const decl of analysis.declarations) {
if (this.metaReader.isAbstractDirective(decl)) {
throw new FatalDiagnosticError(
ErrorCode.DIRECTIVE_MISSING_SELECTOR, decl.node,
`Directive ${decl.node.name.text} has no selector, please add it!`);
}
}
}
if (scope === null || scope.reexports === null) {

8
packages/compiler-cli/src/ngtsc/annotations/test/ng_module_spec.ts
View File

@ -8,10 +8,11 @@
import {WrappedNodeExpr} from '@angular/compiler';
import {R3Reference} from '@angular/compiler/src/compiler';
import * as ts from 'typescript';
import {absoluteFrom} from '../../file_system';
import {runInEachFileSystem} from '../../file_system/testing';
import {LocalIdentifierStrategy, NOOP_DEFAULT_IMPORT_RECORDER, ReferenceEmitter} from '../../imports';
import {DtsMetadataReader, LocalMetadataRegistry} from '../../metadata';
import {CompoundMetadataReader, DtsMetadataReader, LocalMetadataRegistry} from '../../metadata';
import {PartialEvaluator} from '../../partial_evaluator';
import {TypeScriptReflectionHost, isNamedClassDeclaration} from '../../reflection';
import {LocalModuleScopeRegistry, MetadataDtsModuleScopeResolver} from '../../scope';
@ -59,6 +60,7 @@ runInEachFileSystem(() => {
const evaluator = new PartialEvaluator(reflectionHost, checker);
const referencesRegistry = new NoopReferencesRegistry();
const metaRegistry = new LocalMetadataRegistry();
const metaReader = new CompoundMetadataReader([metaRegistry]);
const dtsReader = new DtsMetadataReader(checker, reflectionHost);
const scopeRegistry = new LocalModuleScopeRegistry(
metaRegistry, new MetadataDtsModuleScopeResolver(dtsReader, null),
@ -66,8 +68,8 @@ runInEachFileSystem(() => {
const refEmitter = new ReferenceEmitter([new LocalIdentifierStrategy()]);
const handler = new NgModuleDecoratorHandler(
reflectionHost, evaluator, metaRegistry, scopeRegistry, referencesRegistry, false, null,
refEmitter, NOOP_DEFAULT_IMPORT_RECORDER);
reflectionHost, evaluator, metaReader, metaRegistry, scopeRegistry, referencesRegistry,
false, null, refEmitter, NOOP_DEFAULT_IMPORT_RECORDER);
const TestModule =
getDeclaration(program, _('/entry.ts'), 'TestModule', isNamedClassDeclaration);
const detected =

1
packages/compiler-cli/src/ngtsc/diagnostics/src/code.ts
View File

@ -24,6 +24,7 @@ export enum ErrorCode {
COMPONENT_MISSING_TEMPLATE = 2001,
PIPE_MISSING_NAME = 2002,
PARAM_MISSING_TOKEN = 2003,
DIRECTIVE_MISSING_SELECTOR = 2004,
SYMBOL_NOT_EXPORTED = 3001,
SYMBOL_EXPORTED_UNDER_DIFFERENT_NAME = 3002,

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

@ -93,6 +93,19 @@ 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;
@ -187,6 +200,7 @@ 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>();

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

@ -75,6 +75,7 @@ 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;
@ -84,6 +85,7 @@ 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;

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

@ -21,6 +21,11 @@ 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).

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

@ -16,10 +16,14 @@ 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;
}
@ -30,6 +34,7 @@ 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); }
@ -41,6 +46,12 @@ 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

@ -125,6 +125,10 @@ function extractTemplateGuard(member: ClassMember): TemplateGuardMeta|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);

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

@ -514,9 +514,9 @@ export class NgtscProgram implements api.Program {
this.reflector, this.defaultImportTracker, this.isCore,
this.options.strictInjectionParameters || false),
new NgModuleDecoratorHandler(
this.reflector, evaluator, metaRegistry, scopeRegistry, referencesRegistry, this.isCore,
this.routeAnalyzer, this.refEmitter, this.defaultImportTracker,
this.options.i18nInLocale),
this.reflector, evaluator, this.metaReader, metaRegistry, scopeRegistry,
referencesRegistry, this.isCore, this.routeAnalyzer, this.refEmitter,
this.defaultImportTracker, this.options.i18nInLocale),
new PipeDecoratorHandler(
this.reflector, evaluator, metaRegistry, this.defaultImportTracker, this.isCore),
];

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

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

38
packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts
View File

@ -669,44 +669,6 @@ describe('compiler compliance', () => {
source, EmptyOutletComponentDefinition, 'Incorrect EmptyOutletComponent.ngComponentDef');
});
it('should not support directives without selector', () => {
const files = {
app: {
'spec.ts': `
import {Component, Directive, NgModule} from '@angular/core';
@Directive({})
export class EmptyOutletDirective {}
@NgModule({declarations: [EmptyOutletDirective]})
export class MyModule{}
`
}
};
expect(() => compile(files, angularFiles))
.toThrowError('Directive EmptyOutletDirective has no selector, please add it!');
});
it('should not support directives with empty selector', () => {
const files = {
app: {
'spec.ts': `
import {Component, Directive, NgModule} from '@angular/core';
@Directive({selector: ''})
export class EmptyOutletDirective {}
@NgModule({declarations: [EmptyOutletDirective]})
export class MyModule{}
`
}
};
expect(() => compile(files, angularFiles))
.toThrowError('Directive EmptyOutletDirective has no selector, please add it!');
});
it('should not treat ElementRef, ViewContainerRef, or ChangeDetectorRef specially when injecting',
() => {
const files = {

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

@ -1008,19 +1008,43 @@ runInEachFileSystem(os => {
expect(jsContents).toContain('selectors: [["ng-component"]]');
});
it('should throw if selector is missing in Directive decorator params', () => {
env.write('test.ts', `
import {Directive} from '@angular/core';
it('should allow directives with no selector that are not in NgModules', () => {
env.write('main.ts', `
import {Directive} from '@angular/core';
@Directive({
inputs: ['a', 'b']
})
export class TestDir {}
`);
@Directive({})
export class BaseDir {}
@Directive({})
export abstract class AbstractBaseDir {}
@Directive()
export abstract class EmptyDir {}
@Directive({
inputs: ['a', 'b']
})
export class TestDirWithInputs {}
`);
const errors = env.driveDiagnostics();
expect(errors.length).toBe(0);
});
it('should not allow directives with no selector that are in NgModules', () => {
env.write('main.ts', `
import {Directive, NgModule} from '@angular/core';
@Directive({})
export class BaseDir {}
@NgModule({
declarations: [BaseDir],
})
export class MyModule {}
`);
const errors = env.driveDiagnostics();
expect(trim(errors[0].messageText as string))
.toContain('Directive TestDir has no selector, please add it!');
.toContain('Directive BaseDir has no selector, please add it!');
});
it('should throw if Directive selector is an empty string', () => {

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

@ -132,10 +132,6 @@ export function compileDirectiveFromMetadata(
addFeatures(definitionMap, meta);
const expression = o.importExpr(R3.defineDirective).callFn([definitionMap.toLiteralMap()]);
if (!meta.selector) {
throw new Error(`Directive ${meta.name} has no selector, please add it!`);
}
const type = createTypeForDef(meta, R3.DirectiveDefWithMeta);
return {expression, type, statements};
}

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

@ -117,7 +117,7 @@ function hasSelectorScope<T>(component: Type<T>): component is Type<T>&
* In the event that compilation is not immediate, `compileDirective` will return a `Promise` which
* will resolve when compilation completes and the directive becomes usable.
*/
export function compileDirective(type: Type<any>, directive: Directive): void {
export function compileDirective(type: Type<any>, directive: Directive | null): void {
let ngDirectiveDef: any = null;
Object.defineProperty(type, NG_DIRECTIVE_DEF, {
get: () => {
@ -125,7 +125,10 @@ export function compileDirective(type: Type<any>, directive: Directive): void {
const name = type && type.name;
const sourceMapUrl = `ng:///${name}/ngDirectiveDef.js`;
const compiler = getCompilerFacade();
const facade = directiveMetadata(type as ComponentType<any>, directive);
// `directive` can be null in the case of abstract directives as a base class
// that use `@Directive()` with no selector. In that case, pass empty object to the
// `directiveMetadata` function instead of null.
const facade = directiveMetadata(type as ComponentType<any>, directive || {});
facade.typeSourceSpan = compiler.createParseSourceSpan('Directive', name, sourceMapUrl);
if (facade.usesInheritance) {
addBaseDefToUndecoratedParents(type);

9
packages/core/src/render3/jit/module.ts
View File

@ -185,6 +185,7 @@ function verifySemanticsOfNgModuleDef(
});
const exports = maybeUnwrapFn(ngModuleDef.exports);
declarations.forEach(verifyDeclarationsHaveDefinitions);
declarations.forEach(verifyDirectivesHaveSelector);
const combinedDeclarations: Type<any>[] = [
...declarations.map(resolveForwardRef),
...flatten(imports.map(computeCombinedExports)).map(resolveForwardRef),
@ -220,6 +221,14 @@ function verifySemanticsOfNgModuleDef(
}
}
function verifyDirectivesHaveSelector(type: Type<any>): void {
type = resolveForwardRef(type);
const def = getDirectiveDef(type);
if (!getComponentDef(type) && def && def.selectors.length == 0) {
errors.push(`Directive ${stringifyForError(type)} has no selector, please add it!`);
}
}
function verifyExportsAreDeclaredOrReExported(type: Type<any>) {
type = resolveForwardRef(type);
const kind = getComponentDef(type) && 'component' || getDirectiveDef(type) && 'directive' ||

34
packages/core/test/linker/integration_spec.ts
View File

@ -1411,7 +1411,7 @@ function declareTests(config?: {useJit: boolean}) {
expect(getDOM().querySelectorAll(fixture.nativeElement, 'script').length).toEqual(0);
});
it('should throw when using directives without selector', () => {
it('should throw when using directives without selector in NgModule declarations', () => {
@Directive({})
class SomeDirective {
}
@ -1425,6 +1425,38 @@ function declareTests(config?: {useJit: boolean}) {
.toThrowError(`Directive ${stringify(SomeDirective)} has no selector, please add it!`);
});
it('should not throw when using directives without selector as base class not in declarations',
() => {
@Directive({})
abstract class Base {
constructor(readonly injector: Injector) {}
}
@Directive()
abstract class EmptyDir {
}
@Directive({inputs: ['a', 'b']})
class TestDirWithInputs {
}
@Component({selector: 'comp', template: ''})
class SomeComponent extends Base {
}
@Component({selector: 'comp2', template: ''})
class SomeComponent2 extends EmptyDir {
}
@Component({selector: 'comp3', template: ''})
class SomeComponent3 extends TestDirWithInputs {
}
TestBed.configureTestingModule(
{declarations: [MyComp, SomeComponent, SomeComponent2, SomeComponent3]});
expect(() => TestBed.createComponent(MyComp)).not.toThrowError();
});
it('should throw when using directives with empty string selector', () => {
@Directive({selector: ''})
class SomeDirective {

2
packages/core/testing/src/r3_test_bed_compiler.ts
View File

@ -299,7 +299,7 @@ export class R3TestBedCompiler {
this.pendingComponents.clear();
this.pendingDirectives.forEach(declaration => {
const metadata = this.resolvers.directive.resolve(declaration) !;
const metadata = this.resolvers.directive.resolve(declaration);
this.maybeStoreNgDef(NG_DIRECTIVE_DEF, declaration);
compileDirective(declaration, metadata);
});