From 76ed13bffe47c325144ee0ece4ca876db8631b54 Mon Sep 17 00:00:00 2001 From: Marc Laval Date: Wed, 9 Jan 2019 17:55:23 +0100 Subject: [PATCH] fix(ivy): directives without selector should not be supported (#28021) PR Close #28021 --- .../src/ngtsc/annotations/src/directive.ts | 3 ++ .../compliance/r3_compiler_compliance_spec.ts | 54 +++++++++++++------ .../compiler/src/render3/view/compiler.ts | 6 +-- packages/core/test/linker/integration_spec.ts | 24 ++++----- 4 files changed, 55 insertions(+), 32 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts b/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts index a7a89500b2..27f569af19 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts @@ -165,6 +165,9 @@ export function extractDirectiveMetadata( } selector = resolved; } + if (!selector) { + throw new Error(`Directive ${clazz.name !.text} has no selector, please add it!`); + } const host = extractHostBindings(directive, decoratedElements, evaluator, coreModule); diff --git a/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts b/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts index 7ad7b35f18..7912501877 100644 --- a/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts +++ b/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts @@ -640,9 +640,6 @@ describe('compiler compliance', () => { 'spec.ts': ` import {Component, Directive, NgModule} from '@angular/core'; - @Directive({}) - export class EmptyOutletDirective {} - @Component({template: ''}) export class EmptyOutletComponent {} @@ -652,16 +649,6 @@ describe('compiler compliance', () => { } }; - // EmptyOutletDirective definition should be: - const EmptyOutletDirectiveDefinition = ` - … - EmptyOutletDirective.ngDirectiveDef = $r3$.ɵdefineDirective({ - type: EmptyOutletDirective, - selectors: [], - factory: function EmptyOutletDirective_Factory(t) { return new (t || EmptyOutletDirective)(); } - }); - `; - // EmptyOutletComponent definition should be: const EmptyOutletComponentDefinition = ` … @@ -683,13 +670,48 @@ describe('compiler compliance', () => { const result = compile(files, angularFiles); const source = result.source; - expectEmit( - source, EmptyOutletDirectiveDefinition, - 'Incorrect EmptyOutletDirective.ngDirectiveDefDef'); expectEmit( 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 = { diff --git a/packages/compiler/src/render3/view/compiler.ts b/packages/compiler/src/render3/view/compiler.ts index 93bf899c89..a326749a94 100644 --- a/packages/compiler/src/render3/view/compiler.ts +++ b/packages/compiler/src/render3/view/compiler.ts @@ -164,9 +164,9 @@ export function compileDirectiveFromMetadata( addFeatures(definitionMap, meta); const expression = o.importExpr(R3.defineDirective).callFn([definitionMap.toLiteralMap()]); - // 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, ''); + 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}; diff --git a/packages/core/test/linker/integration_spec.ts b/packages/core/test/linker/integration_spec.ts index 679794e9c3..ade34f8038 100644 --- a/packages/core/test/linker/integration_spec.ts +++ b/packages/core/test/linker/integration_spec.ts @@ -1423,21 +1423,19 @@ function declareTests(config?: {useJit: boolean}) { expect(getDOM().querySelectorAll(fixture.nativeElement, 'script').length).toEqual(0); }); - fixmeIvy('FW-662: Components without selector are not supported') - .it('should throw when using directives without selector', () => { - @Directive({}) - class SomeDirective { - } + it('should throw when using directives without selector', () => { + @Directive({}) + class SomeDirective { + } - @Component({selector: 'comp', template: ''}) - class SomeComponent { - } + @Component({selector: 'comp', template: ''}) + class SomeComponent { + } - TestBed.configureTestingModule({declarations: [MyComp, SomeDirective, SomeComponent]}); - expect(() => TestBed.createComponent(MyComp)) - .toThrowError( - `Directive ${stringify(SomeDirective)} has no selector, please add it!`); - }); + TestBed.configureTestingModule({declarations: [MyComp, SomeDirective, SomeComponent]}); + expect(() => TestBed.createComponent(MyComp)) + .toThrowError(`Directive ${stringify(SomeDirective)} has no selector, please add it!`); + }); it('should use a default element name for components without selectors', () => { let noSelectorComponentFactory: ComponentFactory = undefined !;