From 4c92cf43cf26d2df356093c67c4b8ea1d5c5def6 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Mon, 4 May 2020 21:20:00 +0200 Subject: [PATCH] feat(compiler-cli): report error if undecorated class with Angular features is discovered (#36921) Previously in v9, we deprecated the pattern of undecorated base classes that rely on Angular features. We ran a migration for this in version 9 and will run the same on in version 10 again. To ensure that projects do not regress and start using the unsupported pattern again, we report an error in ngtsc if such undecorated classes are discovered. We keep the compatibility code enabled in ngcc so that libraries can be still be consumed, even if they have not been migrated yet. Resolves FW-2130. PR Close #36921 --- .../public-api/compiler-cli/error_code.d.ts | 1 + package.json | 1 + .../ngcc/src/analysis/decoration_analyzer.ts | 8 +- .../src/ngtsc/annotations/src/diagnostics.ts | 8 + .../src/ngtsc/annotations/src/directive.ts | 53 +- .../ngtsc/annotations/test/directive_spec.ts | 3 +- .../src/ngtsc/core/src/compiler.ts | 6 +- .../src/ngtsc/diagnostics/src/error_code.ts | 6 + .../compliance/r3_compiler_compliance_spec.ts | 570 ------------------ .../compiler-cli/test/ngtsc/ngtsc_spec.ts | 76 ++- yarn.lock | 5 + 11 files changed, 146 insertions(+), 591 deletions(-) diff --git a/goldens/public-api/compiler-cli/error_code.d.ts b/goldens/public-api/compiler-cli/error_code.d.ts index 14c75dc2a1..62a4be71ca 100644 --- a/goldens/public-api/compiler-cli/error_code.d.ts +++ b/goldens/public-api/compiler-cli/error_code.d.ts @@ -13,6 +13,7 @@ export declare enum ErrorCode { DIRECTIVE_MISSING_SELECTOR = 2004, UNDECORATED_PROVIDER = 2005, DIRECTIVE_INHERITS_UNDECORATED_CTOR = 2006, + UNDECORATED_CLASS_USING_ANGULAR_FEATURES = 2007, SYMBOL_NOT_EXPORTED = 3001, SYMBOL_EXPORTED_UNDER_DIFFERENT_NAME = 3002, CONFIG_FLAT_MODULE_NO_INDEX = 4001, diff --git a/package.json b/package.json index dd83d259bc..18ea1c88fd 100644 --- a/package.json +++ b/package.json @@ -108,6 +108,7 @@ "jasmine": "^3.5.0", "jasmine-core": "^3.5.0", "jquery": "3.0.0", + "js-levenshtein": "^1.1.6", "karma": "~4.1.0", "karma-chrome-launcher": "^2.2.0", "karma-firefox-launcher": "^1.2.0", diff --git a/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts b/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts index 32c8af2a09..b4ec66fde8 100644 --- a/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts +++ b/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts @@ -104,7 +104,13 @@ export class DecorationAnalyzer { new DirectiveDecoratorHandler( this.reflectionHost, this.evaluator, this.fullRegistry, this.scopeRegistry, this.fullMetaReader, NOOP_DEFAULT_IMPORT_RECORDER, this.injectableRegistry, this.isCore, - /* annotateForClosureCompiler */ false) as DecoratorHandler, + /* annotateForClosureCompiler */ false, + // In ngcc we want to compile undecorated classes with Angular features. As of + // version 10, undecorated classes that use Angular features are no longer handled + // in ngtsc, but we want to ensure compatibility in ngcc for outdated libraries that + // have not migrated to explicit decorators. See: https://hackmd.io/@alx/ryfYYuvzH. + /* compileUndecoratedClassesWithAngularFeatures */ true + ) as DecoratorHandler, // clang-format on // Pipe handler must be before injectable handler in list so pipe factories are printed // before injectable factories (so injectable factories can delegate to them) diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/diagnostics.ts b/packages/compiler-cli/src/ngtsc/annotations/src/diagnostics.ts index 0b4c1da3c5..6ddcacd939 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/diagnostics.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/diagnostics.ts @@ -78,6 +78,14 @@ export function getDirectiveDiagnostics( return diagnostics; } +export function getUndecoratedClassWithAngularFeaturesDiagnostic(node: ClassDeclaration): + ts.Diagnostic { + return makeDiagnostic( + ErrorCode.UNDECORATED_CLASS_USING_ANGULAR_FEATURES, node.name, + `Class is using Angular features but is not decorated. Please add an explicit ` + + `Angular decorator.`); +} + export function checkInheritanceOfDirective( node: ClassDeclaration, reader: MetadataReader, reflector: ReflectionHost, evaluator: PartialEvaluator): ts.Diagnostic|null { diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts b/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts index 1c86f3d0b3..b4ea78b33e 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts @@ -18,7 +18,7 @@ import {ClassDeclaration, ClassMember, ClassMemberKind, Decorator, filterToMembe import {LocalModuleScopeRegistry} from '../../scope'; import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerFlags, HandlerPrecedence, ResolveResult} from '../../transform'; -import {getDirectiveDiagnostics, getProviderDiagnostics} from './diagnostics'; +import {getDirectiveDiagnostics, getProviderDiagnostics, getUndecoratedClassWithAngularFeaturesDiagnostic} from './diagnostics'; import {compileNgFactoryDefField} from './factory'; import {generateSetClassMetadataCall} from './metadata'; import {createSourceSpan, findAngularDecorator, getConstructorDependencies, isAngularDecorator, readBaseClass, resolveProvidersRequiringFactory, unwrapConstructorDependencies, unwrapExpression, unwrapForwardRef, validateConstructorDependencies, wrapFunctionExpressionsInParens, wrapTypeReference} from './util'; @@ -48,28 +48,20 @@ export class DirectiveDecoratorHandler implements private metaRegistry: MetadataRegistry, private scopeRegistry: LocalModuleScopeRegistry, private metaReader: MetadataReader, private defaultImportRecorder: DefaultImportRecorder, private injectableRegistry: InjectableClassRegistry, private isCore: boolean, - private annotateForClosureCompiler: boolean) {} + private annotateForClosureCompiler: boolean, + private compileUndecoratedClassesWithAngularFeatures: boolean) {} readonly precedence = HandlerPrecedence.PRIMARY; readonly name = DirectiveDecoratorHandler.name; detect(node: ClassDeclaration, decorators: Decorator[]|null): DetectResult|undefined { - // If the class is undecorated, check if any of the fields have Angular decorators or lifecycle - // hooks, and if they do, label the class as an abstract directive. + // If a class is undecorated but uses Angular features, we detect it as an + // abstract directive. This is an unsupported pattern as of v10, but we want + // to still detect these patterns so that we can report diagnostics, or compile + // them for backwards compatibility in ngcc. if (!decorators) { - const angularField = this.reflector.getMembersOfClass(node).find(member => { - if (!member.isStatic && member.kind === ClassMemberKind.Method && - LIFECYCLE_HOOKS.has(member.name)) { - return true; - } - if (member.decorators) { - return member.decorators.some( - decorator => FIELD_DECORATORS.some( - decoratorName => isAngularDecorator(decorator, decoratorName, this.isCore))); - } - return false; - }); + const angularField = this.findClassFieldWithAngularFeatures(node); return angularField ? {trigger: angularField.node, decorator: null, metadata: null} : undefined; } else { @@ -80,6 +72,14 @@ export class DirectiveDecoratorHandler implements analyze(node: ClassDeclaration, decorator: Readonly, flags = HandlerFlags.NONE): AnalysisOutput { + // Skip processing of the class declaration if compilation of undecorated classes + // with Angular features is disabled. Previously in ngtsc, such classes have always + // been processed, but we want to enforce a consistent decorator mental model. + // See: https://v9.angular.io/guide/migration-undecorated-classes. + if (this.compileUndecoratedClassesWithAngularFeatures === false && decorator === null) { + return {diagnostics: [getUndecoratedClassWithAngularFeaturesDiagnostic(node)]}; + } + const directiveResult = extractDirectiveMetadata( node, decorator, this.reflector, this.evaluator, this.defaultImportRecorder, this.isCore, flags, this.annotateForClosureCompiler); @@ -167,6 +167,27 @@ export class DirectiveDecoratorHandler implements } ]; } + + /** + * Checks if a given class uses Angular features and returns the TypeScript node + * that indicated the usage. Classes are considered using Angular features if they + * contain class members that are either decorated with a known Angular decorator, + * or if they correspond to a known Angular lifecycle hook. + */ + private findClassFieldWithAngularFeatures(node: ClassDeclaration): ClassMember|undefined { + return this.reflector.getMembersOfClass(node).find(member => { + if (!member.isStatic && member.kind === ClassMemberKind.Method && + LIFECYCLE_HOOKS.has(member.name)) { + return true; + } + if (member.decorators) { + return member.decorators.some( + decorator => FIELD_DECORATORS.some( + decoratorName => isAngularDecorator(decorator, decoratorName, this.isCore))); + } + return false; + }); + } } /** diff --git a/packages/compiler-cli/src/ngtsc/annotations/test/directive_spec.ts b/packages/compiler-cli/src/ngtsc/annotations/test/directive_spec.ts index 0cd61336f1..d8e350ffd5 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/test/directive_spec.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/test/directive_spec.ts @@ -99,7 +99,8 @@ runInEachFileSystem(() => { const handler = new DirectiveDecoratorHandler( reflectionHost, evaluator, scopeRegistry, scopeRegistry, metaReader, NOOP_DEFAULT_IMPORT_RECORDER, injectableRegistry, /*isCore*/ false, - /*annotateForClosureCompiler*/ false); + /*annotateForClosureCompiler*/ false, + /*detectUndecoratedClassesWithAngularFeatures*/ false); const DirNode = getDeclaration(program, _('/entry.ts'), dirName, isNamedClassDeclaration); diff --git a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts index 7b411a30a0..868b51ab67 100644 --- a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts +++ b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts @@ -704,7 +704,11 @@ export class NgCompiler { // clang-format off new DirectiveDecoratorHandler( reflector, evaluator, metaRegistry, scopeRegistry, metaReader, - defaultImportTracker, injectableRegistry, isCore, this.closureCompilerEnabled + defaultImportTracker, injectableRegistry, isCore, this.closureCompilerEnabled, + // In ngtsc we no longer want to compile undecorated classes with Angular features. + // Migrations for these patterns ran as part of `ng update` and we want to ensure + // that projects do not regress. See https://hackmd.io/@alx/ryfYYuvzH for more details. + /* compileUndecoratedClassesWithAngularFeatures */ false ) as Readonly>, // clang-format on // Pipe handler must be before injectable handler in list so pipe factories are printed diff --git a/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts b/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts index 23c80c91e5..0ee2a9c900 100644 --- a/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts +++ b/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts @@ -38,6 +38,12 @@ export enum ErrorCode { */ DIRECTIVE_INHERITS_UNDECORATED_CTOR = 2006, + /** + * Raised when an undecorated class that is using Angular features + * has been discovered. + */ + UNDECORATED_CLASS_USING_ANGULAR_FEATURES = 2007, + SYMBOL_NOT_EXPORTED = 3001, SYMBOL_EXPORTED_UNDER_DIFFERENT_NAME = 3002, 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 00f1155b58..e3fbbdc6af 100644 --- a/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts +++ b/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts @@ -3276,574 +3276,4 @@ describe('compiler compliance', () => { expectEmit(result.source, MyAppDeclaration, 'Invalid component definition'); }); }); - - describe('inherited base classes', () => { - const directive = { - 'some.directive.ts': ` - import {Directive} from '@angular/core'; - - @Directive({ - selector: '[someDir]', - }) - export class SomeDirective { } - ` - }; - - it('should add an abstract directive if one or more @Input is present', () => { - const files = { - app: { - 'spec.ts': ` - import {Component, NgModule, Input} from '@angular/core'; - export class BaseClass { - @Input() - input1 = 'test'; - - @Input('alias2') - input2 = 'whatever'; - } - - @Component({ - selector: 'my-component', - template: \`
{{input1}} {{input2}}
\` - }) - export class MyComponent extends BaseClass { - } - - @NgModule({ - declarations: [MyComponent] - }) - export class MyModule {} - ` - } - }; - const expectedOutput = ` - // ... - BaseClass.ɵdir = $r3$.ɵɵdefineDirective({ - type: BaseClass, - inputs: { - input1: "input1", - input2: ["alias2", "input2"] - } - }); - // ... - `; - const result = compile(files, angularFiles); - expectEmit(result.source, expectedOutput, 'Invalid directive definition'); - }); - - it('should add an abstract directive if one or more @Output is present', () => { - const files = { - app: { - 'spec.ts': ` - import {Component, NgModule, Output, EventEmitter} from '@angular/core'; - export class BaseClass { - @Output() - output1 = new EventEmitter(); - - @Output() - output2 = new EventEmitter(); - - clicked() { - this.output1.emit('test'); - this.output2.emit('test'); - } - } - - @Component({ - selector: 'my-component', - template: \`\` - }) - export class MyComponent extends BaseClass { - } - - @NgModule({ - declarations: [MyComponent] - }) - export class MyModule {} - ` - } - }; - const expectedOutput = ` - // ... - BaseClass.ɵdir = $r3$.ɵɵdefineDirective({ - type: BaseClass, - outputs: { - output1: "output1", - output2: "output2" - } - }); - // ... - `; - const result = compile(files, angularFiles); - expectEmit(result.source, expectedOutput, 'Invalid directive definition'); - }); - - it('should add an abstract directive if a mixture of @Input and @Output props are present', - () => { - const files = { - app: { - 'spec.ts': ` - import {Component, NgModule, Input, Output, EventEmitter} from '@angular/core'; - export class BaseClass { - @Output() - output1 = new EventEmitter(); - - @Output() - output2 = new EventEmitter(); - - @Input() - input1 = 'test'; - - @Input('whatever') - input2 = 'blah'; - - clicked() { - this.output1.emit('test'); - this.output2.emit('test'); - } - } - - @Component({ - selector: 'my-component', - template: \`\` - }) - export class MyComponent extends BaseClass { - } - - @NgModule({ - declarations: [MyComponent] - }) - export class MyModule {} - ` - } - }; - const expectedOutput = ` - // ... - BaseClass.ɵdir = $r3$.ɵɵdefineDirective({ - type: BaseClass, - inputs: { - input1: "input1", - input2: ["whatever", "input2"] - }, - outputs: { - output1: "output1", - output2: "output2" - } - }); - // ... - `; - const result = compile(files, angularFiles); - expectEmit(result.source, expectedOutput, 'Invalid directive definition'); - }); - - it('should add an abstract directive if a ViewChild query is present', () => { - const files = { - app: { - 'spec.ts': ` - import {Component, NgModule, ViewChild} from '@angular/core'; - export class BaseClass { - @ViewChild('something') something: any; - } - - @Component({ - selector: 'my-component', - template: '' - }) - export class MyComponent extends BaseClass { - } - - @NgModule({ - declarations: [MyComponent] - }) - export class MyModule {} - ` - } - }; - const expectedOutput = ` - const $e0_attrs$ = ["something"]; - // ... - BaseClass.ɵdir = $r3$.ɵɵdefineDirective({ - type: BaseClass, - viewQuery: function BaseClass_Query(rf, ctx) { - if (rf & 1) { - $r3$.ɵɵviewQuery($e0_attrs$, true); - } - if (rf & 2) { - var $tmp$; - $r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.something = $tmp$.first); - } - } - }); - // ... - `; - const result = compile(files, angularFiles); - expectEmit(result.source, expectedOutput, 'Invalid directive definition'); - }); - - it('should add an abstract directive if a ViewChildren query is present', () => { - const files = { - app: { - ...directive, - 'spec.ts': ` - import {Component, NgModule, ViewChildren} from '@angular/core'; - import {SomeDirective} from './some.directive'; - - export class BaseClass { - @ViewChildren(SomeDirective) something: QueryList; - } - - @Component({ - selector: 'my-component', - template: '' - }) - export class MyComponent extends BaseClass { - } - - @NgModule({ - declarations: [MyComponent, SomeDirective] - }) - export class MyModule {} - ` - } - }; - const expectedOutput = ` - // ... - BaseClass.ɵdir = $r3$.ɵɵdefineDirective({ - type: BaseClass, - viewQuery: function BaseClass_Query(rf, ctx) { - if (rf & 1) { - $r3$.ɵɵviewQuery(SomeDirective, true); - } - if (rf & 2) { - var $tmp$; - $r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.something = $tmp$); - } - } - }); - // ... - `; - const result = compile(files, angularFiles); - expectEmit(result.source, expectedOutput, 'Invalid directive definition'); - }); - - it('should add an abstract directive if a ContentChild query is present', () => { - const files = { - app: { - 'spec.ts': ` - import {Component, NgModule, ContentChild} from '@angular/core'; - export class BaseClass { - @ContentChild('something') something: any; - } - - @Component({ - selector: 'my-component', - template: '' - }) - export class MyComponent extends BaseClass { - } - - @NgModule({ - declarations: [MyComponent] - }) - export class MyModule {} - ` - } - }; - const expectedOutput = ` - const $e0_attrs$ = ["something"]; - // ... - BaseClass.ɵdir = $r3$.ɵɵdefineDirective({ - type: BaseClass, - contentQueries: function BaseClass_ContentQueries(rf, ctx, dirIndex) { - if (rf & 1) { - $r3$.ɵɵcontentQuery(dirIndex, $e0_attrs$, true); - } - if (rf & 2) { - var $tmp$; - $r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.something = $tmp$.first); - } - } - }); - // ... - `; - const result = compile(files, angularFiles); - expectEmit(result.source, expectedOutput, 'Invalid directive definition'); - }); - - it('should add an abstract directive if a ContentChildren query is present', () => { - const files = { - app: { - ...directive, - 'spec.ts': ` - import {Component, NgModule, ContentChildren} from '@angular/core'; - import {SomeDirective} from './some.directive'; - - export class BaseClass { - @ContentChildren(SomeDirective) something: QueryList; - } - - @Component({ - selector: 'my-component', - template: '' - }) - export class MyComponent extends BaseClass { - } - - @NgModule({ - declarations: [MyComponent, SomeDirective] - }) - export class MyModule {} - ` - } - }; - const expectedOutput = ` - // ... - BaseClass.ɵdir = $r3$.ɵɵdefineDirective({ - type: BaseClass, - contentQueries: function BaseClass_ContentQueries(rf, ctx, dirIndex) { - if (rf & 1) { - $r3$.ɵɵcontentQuery(dirIndex, SomeDirective, false); - } - if (rf & 2) { - var $tmp$; - $r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.something = $tmp$); - } - } - }); - // ... - `; - const result = compile(files, angularFiles); - expectEmit(result.source, expectedOutput, 'Invalid directive definition'); - }); - - it('should add an abstract directive if a host binding is present', () => { - const files = { - app: { - 'spec.ts': ` - import {Component, NgModule, HostBinding} from '@angular/core'; - export class BaseClass { - @HostBinding('attr.tabindex') - tabindex = -1; - } - - @Component({ - selector: 'my-component', - template: '' - }) - export class MyComponent extends BaseClass { - } - - @NgModule({ - declarations: [MyComponent] - }) - export class MyModule {} - ` - } - }; - const expectedOutput = ` - // ... - BaseClass.ɵdir = $r3$.ɵɵdefineDirective({ - type: BaseClass, - hostVars: 1, - hostBindings: function BaseClass_HostBindings(rf, ctx) { - if (rf & 2) { - $r3$.ɵɵattribute("tabindex", ctx.tabindex); - } - } - }); - // ... - `; - const result = compile(files, angularFiles); - expectEmit(result.source, expectedOutput, 'Invalid directive definition'); - }); - - it('should add an abstract directive if a host listener is present', () => { - const files = { - app: { - 'spec.ts': ` - import {Component, NgModule, HostListener} from '@angular/core'; - export class BaseClass { - @HostListener('mousedown', ['$event']) - handleMousedown(event: any) {} - } - - @Component({ - selector: 'my-component', - template: '' - }) - export class MyComponent extends BaseClass { - } - - @NgModule({ - declarations: [MyComponent] - }) - export class MyModule {} - ` - } - }; - const expectedOutput = ` - // ... - BaseClass.ɵdir = $r3$.ɵɵdefineDirective({ - type: BaseClass, - hostBindings: function BaseClass_HostBindings(rf, ctx) { - if (rf & 1) { - $r3$.ɵɵlistener("mousedown", function BaseClass_mousedown_HostBindingHandler($event) { - return ctx.handleMousedown($event); - }); - } - } - }); - // ... - `; - const result = compile(files, angularFiles); - expectEmit(result.source, expectedOutput, 'Invalid directive definition'); - }); - - it('should add an abstract directive when using any lifecycle hook', () => { - const files = { - app: { - 'spec.ts': ` - import {Component, NgModule, Input} from '@angular/core'; - export class BaseClass { - ngAfterContentChecked() {} - } - - @Component({ - selector: 'my-component', - template: \`
{{input1}} {{input2}}
\` - }) - export class MyComponent extends BaseClass { - } - - @NgModule({ - declarations: [MyComponent] - }) - export class MyModule {} - ` - } - }; - const expectedOutput = ` - // ... - BaseClass.ɵdir = $r3$.ɵɵdefineDirective({ - type: BaseClass - }); - // ... - `; - const result = compile(files, angularFiles); - expectEmit(result.source, expectedOutput, 'Invalid directive definition'); - }); - - - it('should add an abstract directive when using ngOnChanges', () => { - const files = { - app: { - 'spec.ts': ` - import {Component, NgModule, Input} from '@angular/core'; - export class BaseClass { - ngOnChanges() {} - } - - @Component({ - selector: 'my-component', - template: \`
{{input1}} {{input2}}
\` - }) - export class MyComponent extends BaseClass { - } - - @NgModule({ - declarations: [MyComponent] - }) - export class MyModule {} - ` - } - }; - const expectedOutput = ` - // ... - BaseClass.ɵdir = $r3$.ɵɵdefineDirective({ - type: BaseClass, - features: [$r3$.ɵɵNgOnChangesFeature] - }); - // ... - `; - const result = compile(files, angularFiles); - expectEmit(result.source, expectedOutput, 'Invalid directive definition'); - }); - - it('should NOT add an abstract directive if @Component is present', () => { - const files = { - app: { - 'spec.ts': ` - import {Component, NgModule, Output, EventEmitter} from '@angular/core'; - @Component({ - selector: 'whatever', - template: '' - }) - export class BaseClass { - @Output() - output1 = new EventEmitter(); - - @Input() - input1 = 'whatever'; - - clicked() { - this.output1.emit('test'); - } - } - - @Component({ - selector: 'my-component', - template: \`
What is this developer doing?
\` - }) - export class MyComponent extends BaseClass { - } - - @NgModule({ - declarations: [MyComponent] - }) - export class MyModule {} - ` - } - }; - const result = compile(files, angularFiles); - expect(result.source).not.toContain('ɵdir'); - }); - - it('should NOT add an abstract directive if @Directive is present', () => { - const files = { - app: { - 'spec.ts': ` - import {Component, Directive, NgModule, Output, EventEmitter} from '@angular/core'; - @Directive({ - selector: 'whatever', - }) - export class BaseClass { - @Output() - output1 = new EventEmitter(); - - @Input() - input1 = 'whatever'; - - clicked() { - this.output1.emit('test'); - } - } - - @Component({ - selector: 'my-component', - template: '' - }) - export class MyComponent extends BaseClass { - } - - @NgModule({ - declarations: [MyComponent] - }) - export class MyModule {} - ` - } - }; - const result = compile(files, angularFiles); - expect(result.source.match(/ɵdir/g)!.length).toBe(1); - }); - }); }); diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index bc92643b9c..a3f83888aa 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -545,6 +545,7 @@ runInEachFileSystem(os => { Output as AngularOutput } from '@angular/core'; + @AngularDirective() export class TestBase { @AngularInput() input: any; @AngularOutput() output: any; @@ -884,10 +885,11 @@ runInEachFileSystem(os => { expect(jsContents).toContain('background-color: blue'); }); - it('should include generic type for undecorated class declarations', () => { + it('should include generic type in directive definition', () => { env.write('test.ts', ` - import {Component, Input, NgModule} from '@angular/core'; + import {Directive, Input, NgModule} from '@angular/core'; + @Directive() export class TestBase { @Input() input: any; } @@ -905,6 +907,76 @@ runInEachFileSystem(os => { `static ɵdir: i0.ɵɵDirectiveDefWithMeta;`); }); + describe('undecorated classes using Angular features', () => { + it('should error if @Input has been discovered', + () => assertErrorUndecoratedClassWithField('Input')); + it('should error if @Output has been discovered', + () => assertErrorUndecoratedClassWithField('Output')); + it('should error if @ViewChild has been discovered', + () => assertErrorUndecoratedClassWithField('ViewChild')); + it('should error if @ViewChildren has been discovered', + () => assertErrorUndecoratedClassWithField('ViewChildren')); + it('should error if @ContentChild has been discovered', + () => assertErrorUndecoratedClassWithField('ContentChildren')); + it('should error if @HostBinding has been discovered', + () => assertErrorUndecoratedClassWithField('HostBinding')); + it('should error if @HostListener has been discovered', + () => assertErrorUndecoratedClassWithField('HostListener')); + + it(`should error if ngOnChanges lifecycle hook has been discovered`, + () => assertErrorUndecoratedClassWithLifecycleHook('ngOnChanges')); + it(`should error if ngOnInit lifecycle hook has been discovered`, + () => assertErrorUndecoratedClassWithLifecycleHook('ngOnInit')); + it(`should error if ngOnDestroy lifecycle hook has been discovered`, + () => assertErrorUndecoratedClassWithLifecycleHook('ngOnDestroy')); + it(`should error if ngDoCheck lifecycle hook has been discovered`, + () => assertErrorUndecoratedClassWithLifecycleHook('ngDoCheck')); + it(`should error if ngAfterViewInit lifecycle hook has been discovered`, + () => assertErrorUndecoratedClassWithLifecycleHook('ngAfterViewInit')); + it(`should error if ngAfterViewChecked lifecycle hook has been discovered`, + () => assertErrorUndecoratedClassWithLifecycleHook('ngAfterViewChecked')); + it(`should error if ngAfterContentInit lifecycle hook has been discovered`, + () => assertErrorUndecoratedClassWithLifecycleHook('ngAfterContentInit')); + it(`should error if ngAfterContentChecked lifecycle hook has been discovered`, + () => assertErrorUndecoratedClassWithLifecycleHook('ngAfterContentChecked')); + + function assertErrorUndecoratedClassWithField(fieldDecoratorName: string) { + env.write('test.ts', ` + import {Component, ${fieldDecoratorName}, NgModule} from '@angular/core'; + + export class SomeBaseClass { + @${fieldDecoratorName}() someMember: any; + } + `); + + const errors = env.driveDiagnostics(); + expect(errors.length).toBe(1); + expect(trim(errors[0].messageText as string)) + .toContain( + 'Class is using Angular features but is not decorated. Please add an explicit ' + + 'Angular decorator.'); + } + + function assertErrorUndecoratedClassWithLifecycleHook(lifecycleName: string) { + env.write('test.ts', ` + import {Component, NgModule} from '@angular/core'; + + export class SomeBaseClass { + ${lifecycleName}() { + // empty + } + } + `); + + const errors = env.driveDiagnostics(); + expect(errors.length).toBe(1); + expect(trim(errors[0].messageText as string)) + .toContain( + 'Class is using Angular features but is not decorated. Please add an explicit ' + + 'Angular decorator.'); + } + }); + it('should compile NgModules without errors', () => { env.write('test.ts', ` import {Component, NgModule} from '@angular/core'; diff --git a/yarn.lock b/yarn.lock index 329f19beba..abadcbe521 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8096,6 +8096,11 @@ jquery@3.0.0: resolved "https://registry.yarnpkg.com/jquery/-/jquery-3.0.0.tgz#95a2a9541291a9f819e016f85ba247116d03e4ab" integrity sha1-laKpVBKRqfgZ4Bb4W6JHEW0D5Ks= +js-levenshtein@^1.1.6: + version "1.1.6" + resolved "https://registry.yarnpkg.com/js-levenshtein/-/js-levenshtein-1.1.6.tgz#c6cee58eb3550372df8deb85fad5ce66ce01d59d" + integrity sha512-X2BB11YZtrRqY4EnQcLX5Rh373zbK4alC1FW7D7MBhL2gtcC17cTnr6DmfHZeS0s2rTHjUTMMHfG7gO8SSdw+g== + "js-tokens@^3.0.0 || ^4.0.0", js-tokens@^4.0.0: version "4.0.0" resolved "https://registry.yarnpkg.com/js-tokens/-/js-tokens-4.0.0.tgz#19203fb59991df98e3a287050d4647cdeaf32499"