From fe76494759aa7bdad9d5bd6ccc7ac1d351f7da52 Mon Sep 17 00:00:00 2001 From: Andrew Kushnir Date: Mon, 11 Mar 2019 17:58:37 -0700 Subject: [PATCH] fix(ivy): use default selector for Components if selector is empty (#29239) Prior to this change default selector for Components was not applied in case selector is missing or defined as an empty string. This update aligns this behavior between Ivy and VE: now default selector is used for Components when it's needed. Directives with empty selector are not allowed and trigger a compile-time error in both Ivy and VE. PR Close #29239 --- .../src/ngtsc/annotations/src/directive.ts | 3 +- .../compiler-cli/test/ngtsc/ngtsc_spec.ts | 69 +++++++++++++++++++ packages/core/test/linker/integration_spec.ts | 14 ++++ 3 files changed, 85 insertions(+), 1 deletion(-) diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts b/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts index 71099dfba7..c53a39e492 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts @@ -172,7 +172,8 @@ export function extractDirectiveMetadata( throw new FatalDiagnosticError( ErrorCode.VALUE_HAS_WRONG_TYPE, expr, `selector must be a string`); } - selector = resolved; + // 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!`); diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index 9e8be5578e..35924de0ab 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -738,6 +738,75 @@ describe('ngtsc behavioral tests', () => { 'i0.ɵNgModuleDefWithMeta'); }); + describe('empty and missing selectors', () => { + it('should use default selector for Components when no selector present', () => { + env.tsconfig({}); + env.write('test.ts', ` + import {Component} from '@angular/core'; + + @Component({ + template: '...', + }) + export class TestCmp {} + `); + + env.driveMain(); + + const jsContents = env.getContents('test.js'); + expect(jsContents).toContain('selectors: [["ng-component"]]'); + }); + + it('should use default selector for Components with empty string selector', () => { + env.tsconfig({}); + env.write('test.ts', ` + import {Component} from '@angular/core'; + + @Component({ + selector: '', + template: '...', + }) + export class TestCmp {} + `); + + env.driveMain(); + + const jsContents = env.getContents('test.js'); + expect(jsContents).toContain('selectors: [["ng-component"]]'); + }); + + it('should throw if selector is missing in Directive decorator params', () => { + env.tsconfig({}); + env.write('test.ts', ` + import {Directive} from '@angular/core'; + + @Directive({ + inputs: ['a', 'b'] + }) + export class TestDir {} + `); + + const errors = env.driveDiagnostics(); + expect(trim(errors[0].messageText as string)) + .toContain('Directive TestDir has no selector, please add it!'); + }); + + it('should throw if Directive selector is an empty string', () => { + env.tsconfig({}); + env.write('test.ts', ` + import {Directive} from '@angular/core'; + + @Directive({ + selector: '' + }) + export class TestDir {} + `); + + const errors = env.driveDiagnostics(); + expect(trim(errors[0].messageText as string)) + .toContain('Directive TestDir has no selector, please add it!'); + }); + }); + describe('multiple decorators on classes', () => { it('should compile @Injectable on Components, Directives, Pipes, and Modules', () => { env.tsconfig(); diff --git a/packages/core/test/linker/integration_spec.ts b/packages/core/test/linker/integration_spec.ts index b705cd2af8..992a013568 100644 --- a/packages/core/test/linker/integration_spec.ts +++ b/packages/core/test/linker/integration_spec.ts @@ -1426,6 +1426,20 @@ function declareTests(config?: {useJit: boolean}) { .toThrowError(`Directive ${stringify(SomeDirective)} has no selector, please add it!`); }); + it('should throw when using directives with empty string selector', () => { + @Directive({selector: ''}) + class SomeDirective { + } + + @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!`); + }); + it('should use a default element name for components without selectors', () => { let noSelectorComponentFactory: ComponentFactory = undefined !;