diff --git a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts index 8206fef07f851..d466df32697e0 100644 --- a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts +++ b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts @@ -1181,7 +1181,7 @@ export class NgCompiler { const traitCompiler = new TraitCompiler( handlers, reflector, this.delegatingPerfRecorder, this.incrementalCompilation, this.options.compileNonExportedClasses !== false, compilationMode, dtsTransforms, - semanticDepGraphUpdater, this.adapter, isCore); + semanticDepGraphUpdater, this.adapter, isCore, !!this.options.forbidOrphanComponents); // Template type-checking may use the `ProgramDriver` to produce new `ts.Program`(s). If this // happens, they need to be tracked by the `NgCompiler`. diff --git a/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts b/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts index 74d216632e3fb..eec7ae6917e4a 100644 --- a/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts +++ b/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts @@ -97,16 +97,13 @@ export class TraitCompiler implements ProgramTypeCheckAdapter { constructor( private handlers: DecoratorHandler[], - private reflector: ReflectionHost, - private perf: PerfRecorder, + private reflector: ReflectionHost, private perf: PerfRecorder, private incrementalBuild: IncrementalBuild, - private compileNonExportedClasses: boolean, - private compilationMode: CompilationMode, + private compileNonExportedClasses: boolean, private compilationMode: CompilationMode, private dtsTransforms: DtsTransformRegistry, private semanticDepGraphUpdater: SemanticDepGraphUpdater|null, - private sourceFileTypeIdentifier: SourceFileTypeIdentifier, - private readonly isCore: boolean, - ) { + private sourceFileTypeIdentifier: SourceFileTypeIdentifier, private readonly isCore: boolean, + private readonly forbidOrphanComponents: boolean) { for (const handler of handlers) { this.handlersByName.set(handler.name, handler); } @@ -260,11 +257,13 @@ export class TraitCompiler implements ProgramTypeCheckAdapter { let record: ClassRecord|null = this.recordFor(clazz); let foundTraits: PendingTrait[] = []; - // A set to track the detected decorators in local compilation mode. An error will be issued if + // A set to track the detected decorators in some cases. An error will be issued if // undetected decorators (= either non-Angular decorators or Angular duplicate decorators) are - // found. + // found in these cases. const detectedDecorators = - this.compilationMode === CompilationMode.LOCAL ? new Set() : null; + (this.compilationMode === CompilationMode.LOCAL || this.forbidOrphanComponents) ? + new Set() : + null; for (const handler of this.handlers) { const result = handler.detect(clazz, decorators); @@ -343,30 +342,38 @@ export class TraitCompiler implements ProgramTypeCheckAdapter { } } - // Local compilation uses the `DepsTracker` utility, which at the moment cannot track classes + // Local compilation and the logic for detecting orphan components both use the `DepsTracker` utility, which at the moment cannot track classes // mutated by custom/duplicate decorators. So we forbid custom/duplicate decorators on classes // used by deps tracker, i.e., Component, Directive, etc (basically everything except // Injectable) // TODO(b/320536434) Support custom/duplicate decorators for the DepsTracker utility. if (decorators !== null && detectedDecorators !== null && detectedDecorators.size < decorators.length && record !== null && - record.metaDiagnostics === null && - hasDepsTrackerAffectingScopeDecorator(detectedDecorators, this.isCore)) { - // Custom or duplicate decorators found in local compilation mode for a class which has - // Angular decorators other than `@Injectable`! This is not supported yet. But will eventually - // do (b/320536434). For now a temporary error is thrown. - record.metaDiagnostics = - decorators.filter(decorator => !detectedDecorators.has(decorator)) - .map( - decorator => ({ - category: ts.DiagnosticCategory.Error, - code: Number('-99' + ErrorCode.DECORATOR_UNEXPECTED), - file: getSourceFile(clazz), - start: decorator.node.getStart(), - length: decorator.node.getWidth(), - messageText: - 'In local compilation mode, Angular does not support custom decorators or duplicate Angular decorators (except for `@Injectable` classes). Ensure all class decorators are from Angular and each decorator is used at most once for each class.', - })); + record.metaDiagnostics === null && hasDepsTrackerAffectingScopeDecorator(detectedDecorators, this.isCore)) { + // Custom or duplicate decorators found for a class which has + // Angular decorators other than `@Injectable`! This is not supported yet in some cases. But + // will eventually do (b/320536434). For now a temporary error is thrown. + + let messageText: string; + if (this.compilationMode === CompilationMode.LOCAL) { + messageText = + 'In local compilation mode, Angular does not support custom decorators or duplicate Angular decorators (except for `@Injectable` classes). Ensure all class decorators are from Angular and each decorator is used at most once for each class.'; + } else if (this.forbidOrphanComponents) { + messageText = + 'When the Angular compiler option "forbidOrphanComponents" is set, Angular does not support custom decorators or duplicate Angular decorators (except for `@Injectable` classes). Ensure all class decorators are from Angular and each decorator is used at most once for each class.'; + } else { + throw new Error('Impossible state!'); + } + + record.metaDiagnostics = decorators.filter(decorator => !detectedDecorators.has(decorator)) + .map(decorator => ({ + category: ts.DiagnosticCategory.Error, + code: Number('-99' + ErrorCode.DECORATOR_UNEXPECTED), + file: getSourceFile(clazz), + start: decorator.node.getStart(), + length: decorator.node.getWidth(), + messageText, + })); record.traits = foundTraits = []; } diff --git a/packages/compiler-cli/src/ngtsc/transform/test/compilation_spec.ts b/packages/compiler-cli/src/ngtsc/transform/test/compilation_spec.ts index fd1038f3b5146..15631734f1fc9 100644 --- a/packages/compiler-cli/src/ngtsc/transform/test/compilation_spec.ts +++ b/packages/compiler-cli/src/ngtsc/transform/test/compilation_spec.ts @@ -41,7 +41,7 @@ runInEachFileSystem(() => { const compiler = new TraitCompiler( handlers, reflectionHost, NOOP_PERF_RECORDER, NOOP_INCREMENTAL_BUILD, true, compilationMode, new DtsTransformRegistry(), null, fakeSfTypeIdentifier, - /** isCore */ false); + /** isCore */ false, /** forbidOrphanComponents */ false); const sourceFile = program.getSourceFile(filename)!; return {compiler, sourceFile, program, filename: _('/' + filename)}; diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index 932cbf31a5c33..6e9523df72cda 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -552,6 +552,78 @@ function allTests(os: string) { })]); }); + it('should error for custom decorators when forbidOrphanComponents is true', () => { + env.tsconfig({ + forbidOrphanComponents: true, + }); + env.write('test.ts', ` + import {Component} from '@angular/core'; + + function customDecorator {}>(original: T) { + return class extends original { + someProp = 'default'; + }; + } + + @Component({template: ''}) + @customDecorator + class MyComp {} + `); + + const diagnostics = env.driveDiagnostics(); + + expect(diagnostics).toEqual([jasmine.objectContaining({ + messageText: jasmine.stringMatching( + /When the Angular compiler option "forbidOrphanComponents" is set, Angular does not support custom decorators or duplicate Angular decorators/), + })]); + }); + + it('should error for duplicate Component decorator when forbidOrphanComponents is true', () => { + env.tsconfig({ + forbidOrphanComponents: true, + }); + env.write('test.ts', ` + import {Component} from '@angular/core'; + + @Component({template: '1'}) + @Component({template: '2'}) + class MyComp {} + `); + + const diagnostics = env.driveDiagnostics(); + + expect(diagnostics).toEqual([jasmine.objectContaining({ + messageText: jasmine.stringMatching( + /When the Angular compiler option "forbidOrphanComponents" is set, Angular does not support custom decorators or duplicate Angular decorators/), + })]); + }); + + it('should not error for duplicate @Injectable decorators when forbidOrphanComponents is true', + () => { + env.tsconfig({ + forbidOrphanComponents: true, + }); + env.write('test.ts', ` + import {Injectable} from '@angular/core'; + + class SomeServiceImpl { + getMessage() { + return 'Hi!'; + } + } + + @Injectable({providedIn: 'root', useExisting: SomeServiceImpl}) + @Injectable({providedIn: 'root'}) + export abstract class SomeService { + abstract getMessage(): string; + } + `); + + const diagnostics = env.driveDiagnostics(); + + expect(diagnostics).toEqual([]); + }); + // This test triggers the Tsickle compiler which asserts that the file-paths // are valid for the real OS. When on non-Windows systems it doesn't like paths // that start with `C:`.