diff --git a/goldens/public-api/compiler-cli/error_code.md b/goldens/public-api/compiler-cli/error_code.md index 0204a606cf8..e8f2ee7635a 100644 --- a/goldens/public-api/compiler-cli/error_code.md +++ b/goldens/public-api/compiler-cli/error_code.md @@ -42,7 +42,6 @@ export enum ErrorCode { IMPORT_CYCLE_DETECTED = 3003, IMPORT_GENERATION_FAILURE = 3004, INJECTABLE_DUPLICATE_PROV = 9001, - INJECTABLE_INHERITS_INVALID_CONSTRUCTOR = 2013, INLINE_TCB_REQUIRED = 8900, INLINE_TYPE_CTOR_REQUIRED = 8901, INVALID_BANANA_IN_BOX = 8101, diff --git a/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts b/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts index a0b3dd76fde..160da5412ac 100644 --- a/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts +++ b/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts @@ -10,13 +10,12 @@ import ts from 'typescript'; import {ParsedConfiguration} from '../../..'; import {ComponentDecoratorHandler, DirectiveDecoratorHandler, InjectableDecoratorHandler, NgModuleDecoratorHandler, PipeDecoratorHandler, ReferencesRegistry, ResourceLoader} from '../../../src/ngtsc/annotations'; -import {InjectableClassRegistry} from '../../../src/ngtsc/annotations/common'; import {CycleAnalyzer, CycleHandlingStrategy, ImportGraph} from '../../../src/ngtsc/cycles'; import {isFatalDiagnosticError} from '../../../src/ngtsc/diagnostics'; import {absoluteFromSourceFile, LogicalFileSystem, ReadonlyFileSystem} from '../../../src/ngtsc/file_system'; import {AbsoluteModuleStrategy, LocalIdentifierStrategy, LogicalProjectStrategy, ModuleResolver, PrivateExportAliasingHost, Reexport, ReferenceEmitter} from '../../../src/ngtsc/imports'; import {SemanticSymbol} from '../../../src/ngtsc/incremental/semantic_graph'; -import {CompoundMetadataReader, CompoundMetadataRegistry, DtsMetadataReader, LocalMetadataRegistry, ResourceRegistry} from '../../../src/ngtsc/metadata'; +import {CompoundMetadataReader, CompoundMetadataRegistry, DtsMetadataReader, InjectableClassRegistry, LocalMetadataRegistry, ResourceRegistry} from '../../../src/ngtsc/metadata'; import {PartialEvaluator} from '../../../src/ngtsc/partial_evaluator'; import {NOOP_PERF_RECORDER} from '../../../src/ngtsc/perf'; import {LocalModuleScopeRegistry, MetadataDtsModuleScopeResolver, TypeCheckScopeRegistry} from '../../../src/ngtsc/scope'; @@ -98,15 +97,14 @@ export class DecorationAnalyzer { new PartialEvaluator(this.reflectionHost, this.typeChecker, /* dependencyTracker */ null); importGraph = new ImportGraph(this.typeChecker, NOOP_PERF_RECORDER); cycleAnalyzer = new CycleAnalyzer(this.importGraph); - injectableRegistry = new InjectableClassRegistry(this.reflectionHost, this.isCore); + injectableRegistry = new InjectableClassRegistry(this.reflectionHost); typeCheckScopeRegistry = new TypeCheckScopeRegistry(this.scopeRegistry, this.fullMetaReader); handlers: DecoratorHandler[] = [ new ComponentDecoratorHandler( this.reflectionHost, this.evaluator, this.fullRegistry, this.fullMetaReader, this.scopeRegistry, this.dtsModuleScopeResolver, this.scopeRegistry, - this.typeCheckScopeRegistry, new ResourceRegistry(), this.isCore, - /* strictCtorDeps */ false, this.resourceManager, this.rootDirs, - !!this.compilerOptions.preserveWhitespaces, + this.typeCheckScopeRegistry, new ResourceRegistry(), this.isCore, this.resourceManager, + this.rootDirs, !!this.compilerOptions.preserveWhitespaces, /* i18nUseExternalIds */ true, this.bundle.enableI18nLegacyMessageIdFormat, /* usePoisonedData */ false, /* i18nNormalizeLineEndingsInICUs */ false, this.moduleResolver, this.cycleAnalyzer, @@ -119,7 +117,7 @@ export class DecorationAnalyzer { // clang-format off new DirectiveDecoratorHandler( this.reflectionHost, this.evaluator, this.fullRegistry, this.scopeRegistry, - this.fullMetaReader, this.injectableRegistry, this.isCore, /* strictCtorDeps */ false, + this.fullMetaReader, this.injectableRegistry, this.isCore, /* semanticDepGraphUpdater */ null, !!this.compilerOptions.annotateForClosureCompiler, // In ngcc we want to compile undecorated classes with Angular features. As of @@ -136,7 +134,7 @@ export class DecorationAnalyzer { this.reflectionHost, this.evaluator, this.metaRegistry, this.scopeRegistry, this.injectableRegistry, this.isCore, NOOP_PERF_RECORDER), new InjectableDecoratorHandler( - this.reflectionHost, this.evaluator, this.isCore, + this.reflectionHost, this.isCore, /* strictCtorDeps */ false, this.injectableRegistry, NOOP_PERF_RECORDER, /* errorOnDuplicateProv */ false), new NgModuleDecoratorHandler( diff --git a/packages/compiler-cli/src/ngtsc/annotations/common/index.ts b/packages/compiler-cli/src/ngtsc/annotations/common/index.ts index 31c9461ece6..f6f6ba4deea 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/common/index.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/common/index.ts @@ -11,7 +11,6 @@ export * from './src/di'; export * from './src/diagnostics'; export * from './src/evaluation'; export * from './src/factory'; -export * from './src/injectable_registry'; export * from './src/metadata'; export * from './src/references_registry'; export * from './src/schema'; diff --git a/packages/compiler-cli/src/ngtsc/annotations/common/src/diagnostics.ts b/packages/compiler-cli/src/ngtsc/annotations/common/src/diagnostics.ts index 60af04687d0..4b30dfa4fc9 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/common/src/diagnostics.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/common/src/diagnostics.ts @@ -10,13 +10,13 @@ import ts from 'typescript'; import {ErrorCode, FatalDiagnosticError, makeDiagnostic, makeRelatedInformation} from '../../../diagnostics'; import {Reference} from '../../../imports'; +import {InjectableClassRegistry, MetadataReader} from '../../../metadata'; import {describeResolvedType, DynamicValue, PartialEvaluator, ResolvedValue, traceDynamicValue} from '../../../partial_evaluator'; import {ClassDeclaration, ReflectionHost} from '../../../reflection'; import {DeclarationData, LocalModuleScopeRegistry} from '../../../scope'; import {identifierOfNode} from '../../../util/src/typescript'; -import {InjectableClassRegistry} from './injectable_registry'; -import {isAbstractClassDeclaration, readBaseClass} from './util'; +import {readBaseClass} from './util'; /** @@ -102,10 +102,7 @@ export function getProviderDiagnostics( const diagnostics: ts.Diagnostic[] = []; for (const provider of providerClasses) { - const injectableMeta = registry.getInjectableMeta(provider.node); - if (injectableMeta !== null) { - // The provided type is recognized as injectable, so we don't report a diagnostic for this - // provider. + if (registry.isInjectable(provider.node)) { continue; } @@ -127,9 +124,9 @@ Either add the @Injectable() decorator to '${ } export function getDirectiveDiagnostics( - node: ClassDeclaration, injectableRegistry: InjectableClassRegistry, - evaluator: PartialEvaluator, reflector: ReflectionHost, scopeRegistry: LocalModuleScopeRegistry, - strictInjectionParameters: boolean, kind: 'Directive'|'Component'): ts.Diagnostic[]|null { + node: ClassDeclaration, reader: MetadataReader, evaluator: PartialEvaluator, + reflector: ReflectionHost, scopeRegistry: LocalModuleScopeRegistry, + kind: string): ts.Diagnostic[]|null { let diagnostics: ts.Diagnostic[]|null = []; const addDiagnostics = (more: ts.Diagnostic|ts.Diagnostic[]|null) => { @@ -150,8 +147,7 @@ export function getDirectiveDiagnostics( addDiagnostics(makeDuplicateDeclarationError(node, duplicateDeclarations, kind)); } - addDiagnostics(checkInheritanceOfInjectable( - node, injectableRegistry, reflector, evaluator, strictInjectionParameters, kind)); + addDiagnostics(checkInheritanceOfDirective(node, reader, reflector, evaluator)); return diagnostics; } @@ -163,42 +159,9 @@ export function getUndecoratedClassWithAngularFeaturesDiagnostic(node: ClassDecl `Angular decorator.`); } -export function checkInheritanceOfInjectable( - node: ClassDeclaration, injectableRegistry: InjectableClassRegistry, reflector: ReflectionHost, - evaluator: PartialEvaluator, strictInjectionParameters: boolean, - kind: 'Directive'|'Component'|'Pipe'|'Injectable'): ts.Diagnostic|null { - const classWithCtor = findInheritedCtor(node, injectableRegistry, reflector, evaluator); - if (classWithCtor === null || classWithCtor.isCtorValid) { - // The class does not inherit a constructor, or the inherited constructor is compatible - // with DI; no need to report a diagnostic. - return null; - } - - if (!classWithCtor.isDecorated) { - // The inherited constructor exists in a class that does not have an Angular decorator. - // This is an error, as there won't be a factory definition available for DI to invoke - // the constructor. - return getInheritedUndecoratedCtorDiagnostic(node, classWithCtor.ref, kind); - } - - if (!strictInjectionParameters || isAbstractClassDeclaration(node)) { - // An invalid constructor is only reported as error under `strictInjectionParameters` and - // only for concrete classes; follow the same exclusions for derived types. - return null; - } - - return getInheritedInvalidCtorDiagnostic(node, classWithCtor.ref, kind); -} - -interface ClassWithCtor { - ref: Reference; - isCtorValid: boolean; - isDecorated: boolean; -} - -export function findInheritedCtor( - node: ClassDeclaration, injectableRegistry: InjectableClassRegistry, reflector: ReflectionHost, - evaluator: PartialEvaluator): ClassWithCtor|null { +export function checkInheritanceOfDirective( + node: ClassDeclaration, reader: MetadataReader, reflector: ReflectionHost, + evaluator: PartialEvaluator): ts.Diagnostic|null { if (!reflector.isClass(node) || reflector.getConstructorParameters(node) !== null) { // We should skip nodes that aren't classes. If a constructor exists, then no base class // definition is required on the runtime side - it's legal to inherit from any class. @@ -215,64 +178,44 @@ export function findInheritedCtor( return null; } - const injectableMeta = injectableRegistry.getInjectableMeta(baseClass.node); - if (injectableMeta !== null) { - if (injectableMeta.ctorDeps !== null) { - // The class has an Angular decorator with a constructor. - return { - ref: baseClass, - isCtorValid: injectableMeta.ctorDeps !== 'invalid', - isDecorated: true, - }; - } - } else { - const baseClassConstructorParams = reflector.getConstructorParameters(baseClass.node); - if (baseClassConstructorParams !== null) { - // The class is not decorated, but it does have constructor. An undecorated class is only - // allowed to have a constructor without parameters, otherwise it is invalid. - return { - ref: baseClass, - isCtorValid: baseClassConstructorParams.length === 0, - isDecorated: false, - }; - } + // We can skip the base class if it has metadata. + const baseClassMeta = reader.getDirectiveMetadata(baseClass); + if (baseClassMeta !== null) { + return null; + } + + // If the base class has a blank constructor we can skip it since it can't be using DI. + const baseClassConstructorParams = reflector.getConstructorParameters(baseClass.node); + const newParentClass = readBaseClass(baseClass.node, reflector, evaluator); + + if (baseClassConstructorParams !== null && baseClassConstructorParams.length > 0) { + // This class has a non-trivial constructor, that's an error! + return getInheritedUndecoratedCtorDiagnostic(node, baseClass, reader); + } else if (baseClassConstructorParams !== null || newParentClass === null) { + // This class has a trivial constructor, or no constructor + is the + // top of the inheritance chain, so it's okay. + return null; } // Go up the chain and continue - baseClass = readBaseClass(baseClass.node, reflector, evaluator); + baseClass = newParentClass; } return null; } -function getInheritedInvalidCtorDiagnostic( - node: ClassDeclaration, baseClass: Reference, - kind: 'Directive'|'Component'|'Pipe'|'Injectable') { - const baseClassName = baseClass.debugName; - - return makeDiagnostic( - ErrorCode.INJECTABLE_INHERITS_INVALID_CONSTRUCTOR, node.name, - `The ${kind.toLowerCase()} ${node.name.text} inherits its constructor from ${ - baseClassName}, ` + - `but the latter has a constructor parameter that is not compatible with dependency injection. ` + - `Either add an explicit constructor to ${node.name.text} or change ${ - baseClassName}'s constructor to ` + - `use parameters that are valid for DI.`); -} - function getInheritedUndecoratedCtorDiagnostic( - node: ClassDeclaration, baseClass: Reference, - kind: 'Directive'|'Component'|'Pipe'|'Injectable') { + node: ClassDeclaration, baseClass: Reference, reader: MetadataReader) { + const subclassMeta = reader.getDirectiveMetadata(new Reference(node))!; + const dirOrComp = subclassMeta.isComponent ? 'Component' : 'Directive'; const baseClassName = baseClass.debugName; - const baseNeedsDecorator = - kind === 'Component' || kind === 'Directive' ? 'Directive' : 'Injectable'; return makeDiagnostic( ErrorCode.DIRECTIVE_INHERITS_UNDECORATED_CTOR, node.name, - `The ${kind.toLowerCase()} ${node.name.text} inherits its constructor from ${ + `The ${dirOrComp.toLowerCase()} ${node.name.text} inherits its constructor from ${ baseClassName}, ` + `but the latter does not have an Angular decorator of its own. Dependency injection will not be able to ` + - `resolve the parameters of ${baseClassName}'s constructor. Either add a @${ - baseNeedsDecorator} decorator ` + + `resolve the parameters of ${ + baseClassName}'s constructor. Either add a @Directive decorator ` + `to ${baseClassName}, or add an explicit constructor to ${node.name.text}.`); } diff --git a/packages/compiler-cli/src/ngtsc/annotations/common/src/injectable_registry.ts b/packages/compiler-cli/src/ngtsc/annotations/common/src/injectable_registry.ts deleted file mode 100644 index 90e8a59cd93..00000000000 --- a/packages/compiler-cli/src/ngtsc/annotations/common/src/injectable_registry.ts +++ /dev/null @@ -1,50 +0,0 @@ -/** - * @license - * Copyright Google LLC All Rights Reserved. - * - * Use of this source code is governed by an MIT-style license that can be - * found in the LICENSE file at https://angular.io/license - */ -import {R3DependencyMetadata} from '@angular/compiler'; - -import {hasInjectableFields} from '../../../metadata'; -import {ClassDeclaration, ReflectionHost} from '../../../reflection'; - -import {getConstructorDependencies, unwrapConstructorDependencies} from './di'; - - -export interface InjectableMeta { - ctorDeps: R3DependencyMetadata[]|'invalid'|null; -} - -/** - * Registry that keeps track of classes that can be constructed via dependency injection (e.g. - * injectables, directives, pipes). - */ -export class InjectableClassRegistry { - private classes = new Map(); - - constructor(private host: ReflectionHost, private isCore: boolean) {} - - registerInjectable(declaration: ClassDeclaration, meta: InjectableMeta): void { - this.classes.set(declaration, meta); - } - - getInjectableMeta(declaration: ClassDeclaration): InjectableMeta|null { - // Figure out whether the class is injectable based on the registered classes, otherwise - // fall back to looking at its members since we might not have been able to register the class - // if it was compiled in another compilation unit. - if (this.classes.has(declaration)) { - return this.classes.get(declaration)!; - } - - if (!hasInjectableFields(declaration, this.host)) { - return null; - } - - const ctorDeps = getConstructorDependencies(declaration, this.host, this.isCore); - return { - ctorDeps: unwrapConstructorDependencies(ctorDeps), - }; - } -} diff --git a/packages/compiler-cli/src/ngtsc/annotations/common/src/util.ts b/packages/compiler-cli/src/ngtsc/annotations/common/src/util.ts index 01009c7f207..7f215b12c5c 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/common/src/util.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/common/src/util.ts @@ -380,8 +380,3 @@ export function resolveImportedFile( // Figure out what file is being imported. return moduleResolver.resolveModule(expr.value.moduleName!, origin.fileName); } - -export function isAbstractClassDeclaration(clazz: ClassDeclaration): boolean { - return clazz.modifiers !== undefined && - clazz.modifiers.some(mod => mod.kind === ts.SyntaxKind.AbstractKeyword); -} diff --git a/packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts b/packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts index 3c5072882f4..b775f1949b3 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts @@ -16,7 +16,7 @@ import {assertSuccessfulReferenceEmit, ImportedFile, ModuleResolver, Reference, import {DependencyTracker} from '../../../incremental/api'; import {extractSemanticTypeParameters, SemanticDepGraphUpdater} from '../../../incremental/semantic_graph'; import {IndexingContext} from '../../../indexer'; -import {DirectiveMeta, extractDirectiveTypeCheckMeta, MetadataReader, MetadataRegistry, MetaKind, PipeMeta, ResourceRegistry} from '../../../metadata'; +import {DirectiveMeta, extractDirectiveTypeCheckMeta, InjectableClassRegistry, MetadataReader, MetadataRegistry, MetaKind, PipeMeta, ResourceRegistry} from '../../../metadata'; import {PartialEvaluator} from '../../../partial_evaluator'; import {PerfEvent, PerfRecorder} from '../../../perf'; import {ClassDeclaration, DeclarationNode, Decorator, ReflectionHost, reflectObjectLiteral} from '../../../reflection'; @@ -26,7 +26,7 @@ import {TypeCheckContext} from '../../../typecheck/api'; import {ExtendedTemplateChecker} from '../../../typecheck/extended/api'; import {getSourceFile} from '../../../util/src/typescript'; import {Xi18nContext} from '../../../xi18n'; -import {compileDeclareFactory, compileNgFactoryDefField, compileResults, extractClassMetadata, extractSchemas, findAngularDecorator, getDirectiveDiagnostics, getProviderDiagnostics, InjectableClassRegistry, isExpressionForwardReference, readBaseClass, resolveEnumValue, resolveImportedFile, resolveLiteral, resolveProvidersRequiringFactory, ResourceLoader, toFactoryMetadata, wrapFunctionExpressionsInParens} from '../../common'; +import {compileDeclareFactory, compileNgFactoryDefField, compileResults, extractClassMetadata, extractSchemas, findAngularDecorator, getDirectiveDiagnostics, getProviderDiagnostics, isExpressionForwardReference, readBaseClass, resolveEnumValue, resolveImportedFile, resolveLiteral, resolveProvidersRequiringFactory, ResourceLoader, toFactoryMetadata, wrapFunctionExpressionsInParens} from '../../common'; import {extractDirectiveMetadata, parseFieldArrayValue} from '../../directive'; import {NgModuleSymbol} from '../../ng_module'; @@ -51,13 +51,12 @@ export class ComponentDecoratorHandler implements private scopeRegistry: LocalModuleScopeRegistry, private typeCheckScopeRegistry: TypeCheckScopeRegistry, private resourceRegistry: ResourceRegistry, private isCore: boolean, - private strictCtorDeps: boolean, private resourceLoader: ResourceLoader, - private rootDirs: ReadonlyArray, private defaultPreserveWhitespaces: boolean, - private i18nUseExternalIds: boolean, private enableI18nLegacyMessageIdFormat: boolean, - private usePoisonedData: boolean, private i18nNormalizeLineEndingsInICUs: boolean, - private moduleResolver: ModuleResolver, private cycleAnalyzer: CycleAnalyzer, - private cycleHandlingStrategy: CycleHandlingStrategy, private refEmitter: ReferenceEmitter, - private depTracker: DependencyTracker|null, + private resourceLoader: ResourceLoader, private rootDirs: ReadonlyArray, + private defaultPreserveWhitespaces: boolean, private i18nUseExternalIds: boolean, + private enableI18nLegacyMessageIdFormat: boolean, private usePoisonedData: boolean, + private i18nNormalizeLineEndingsInICUs: boolean, private moduleResolver: ModuleResolver, + private cycleAnalyzer: CycleAnalyzer, private cycleHandlingStrategy: CycleHandlingStrategy, + private refEmitter: ReferenceEmitter, private depTracker: DependencyTracker|null, private injectableRegistry: InjectableClassRegistry, private semanticDepGraphUpdater: SemanticDepGraphUpdater|null, private annotateForClosureCompiler: boolean, private perf: PerfRecorder) {} @@ -483,9 +482,7 @@ export class ComponentDecoratorHandler implements }); this.resourceRegistry.registerResources(analysis.resources, node); - this.injectableRegistry.registerInjectable(node, { - ctorDeps: analysis.meta.deps, - }); + this.injectableRegistry.registerInjectable(node); } index( @@ -822,8 +819,7 @@ export class ComponentDecoratorHandler implements } const directiveDiagnostics = getDirectiveDiagnostics( - node, this.injectableRegistry, this.evaluator, this.reflector, this.scopeRegistry, - this.strictCtorDeps, 'Component'); + node, this.metaReader, this.evaluator, this.reflector, this.scopeRegistry, 'Component'); if (directiveDiagnostics !== null) { diagnostics.push(...directiveDiagnostics); } diff --git a/packages/compiler-cli/src/ngtsc/annotations/component/test/component_spec.ts b/packages/compiler-cli/src/ngtsc/annotations/component/test/component_spec.ts index b1cc9bcf06c..b5b065edbbd 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/component/test/component_spec.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/component/test/component_spec.ts @@ -14,13 +14,13 @@ import {ErrorCode, FatalDiagnosticError} from '../../../diagnostics'; import {absoluteFrom} from '../../../file_system'; import {runInEachFileSystem} from '../../../file_system/testing'; import {ModuleResolver, Reference, ReferenceEmitter} from '../../../imports'; -import {CompoundMetadataReader, DtsMetadataReader, LocalMetadataRegistry, ResourceRegistry} from '../../../metadata'; +import {CompoundMetadataReader, DtsMetadataReader, InjectableClassRegistry, LocalMetadataRegistry, ResourceRegistry} from '../../../metadata'; import {PartialEvaluator} from '../../../partial_evaluator'; import {NOOP_PERF_RECORDER} from '../../../perf'; import {isNamedClassDeclaration, TypeScriptReflectionHost} from '../../../reflection'; import {LocalModuleScopeRegistry, MetadataDtsModuleScopeResolver, TypeCheckScopeRegistry} from '../../../scope'; import {getDeclaration, makeProgram} from '../../../testing'; -import {InjectableClassRegistry, ResourceLoader, ResourceLoaderContext} from '../../common'; +import {ResourceLoader, ResourceLoaderContext} from '../../common'; import {ComponentDecoratorHandler} from '../src/handler'; export class StubResourceLoader implements ResourceLoader { @@ -55,7 +55,7 @@ function setup(program: ts.Program, options: ts.CompilerOptions, host: ts.Compil const scopeRegistry = new LocalModuleScopeRegistry( metaRegistry, metaReader, dtsResolver, new ReferenceEmitter([]), null); const refEmitter = new ReferenceEmitter([]); - const injectableRegistry = new InjectableClassRegistry(reflectionHost, /* isCore */ false); + const injectableRegistry = new InjectableClassRegistry(reflectionHost); const resourceRegistry = new ResourceRegistry(); const typeCheckScopeRegistry = new TypeCheckScopeRegistry(scopeRegistry, metaReader); const resourceLoader = new StubResourceLoader(); @@ -71,7 +71,6 @@ function setup(program: ts.Program, options: ts.CompilerOptions, host: ts.Compil typeCheckScopeRegistry, resourceRegistry, /* isCore */ false, - /* strictCtorDeps */ false, resourceLoader, /* rootDirs */['/'], /* defaultPreserveWhitespaces */ false, diff --git a/packages/compiler-cli/src/ngtsc/annotations/directive/src/handler.ts b/packages/compiler-cli/src/ngtsc/annotations/directive/src/handler.ts index 183edfe24cb..048084a1408 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/directive/src/handler.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/directive/src/handler.ts @@ -11,13 +11,13 @@ import ts from 'typescript'; import {Reference} from '../../../imports'; import {extractSemanticTypeParameters, SemanticDepGraphUpdater} from '../../../incremental/semantic_graph'; -import {ClassPropertyMapping, DirectiveTypeCheckMeta, extractDirectiveTypeCheckMeta, MetadataReader, MetadataRegistry, MetaKind} from '../../../metadata'; +import {ClassPropertyMapping, DirectiveTypeCheckMeta, extractDirectiveTypeCheckMeta, InjectableClassRegistry, MetadataReader, MetadataRegistry, MetaKind} from '../../../metadata'; import {PartialEvaluator} from '../../../partial_evaluator'; import {PerfEvent, PerfRecorder} from '../../../perf'; import {ClassDeclaration, ClassMember, ClassMemberKind, Decorator, ReflectionHost} from '../../../reflection'; import {LocalModuleScopeRegistry} from '../../../scope'; import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerFlags, HandlerPrecedence, ResolveResult} from '../../../transform'; -import {compileDeclareFactory, compileNgFactoryDefField, compileResults, extractClassMetadata, findAngularDecorator, getDirectiveDiagnostics, getProviderDiagnostics, getUndecoratedClassWithAngularFeaturesDiagnostic, InjectableClassRegistry, isAngularDecorator, readBaseClass, resolveProvidersRequiringFactory, toFactoryMetadata,} from '../../common'; +import {compileDeclareFactory, compileNgFactoryDefField, compileResults, extractClassMetadata, findAngularDecorator, getDirectiveDiagnostics, getProviderDiagnostics, getUndecoratedClassWithAngularFeaturesDiagnostic, isAngularDecorator, readBaseClass, resolveProvidersRequiringFactory, toFactoryMetadata} from '../../common'; import {extractDirectiveMetadata} from './shared'; import {DirectiveSymbol} from './symbol'; @@ -49,8 +49,7 @@ export class DirectiveDecoratorHandler implements private reflector: ReflectionHost, private evaluator: PartialEvaluator, private metaRegistry: MetadataRegistry, private scopeRegistry: LocalModuleScopeRegistry, private metaReader: MetadataReader, private injectableRegistry: InjectableClassRegistry, - private isCore: boolean, private strictCtorDeps: boolean, - private semanticDepGraphUpdater: SemanticDepGraphUpdater|null, + private isCore: boolean, private semanticDepGraphUpdater: SemanticDepGraphUpdater|null, private annotateForClosureCompiler: boolean, private compileUndecoratedClassesWithAngularFeatures: boolean, private perf: PerfRecorder) {} @@ -152,9 +151,7 @@ export class DirectiveDecoratorHandler implements schemas: null, }); - this.injectableRegistry.registerInjectable(node, { - ctorDeps: analysis.meta.deps, - }); + this.injectableRegistry.registerInjectable(node); } resolve(node: ClassDeclaration, analysis: DirectiveHandlerData, symbol: DirectiveSymbol): @@ -173,8 +170,7 @@ export class DirectiveDecoratorHandler implements } const directiveDiagnostics = getDirectiveDiagnostics( - node, this.injectableRegistry, this.evaluator, this.reflector, this.scopeRegistry, - this.strictCtorDeps, 'Directive'); + node, this.metaReader, this.evaluator, this.reflector, this.scopeRegistry, 'Directive'); if (directiveDiagnostics !== null) { diagnostics.push(...directiveDiagnostics); } diff --git a/packages/compiler-cli/src/ngtsc/annotations/directive/test/BUILD.bazel b/packages/compiler-cli/src/ngtsc/annotations/directive/test/BUILD.bazel index a1a27f243d1..c4bff20194e 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/directive/test/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/annotations/directive/test/BUILD.bazel @@ -11,7 +11,6 @@ ts_library( deps = [ "//packages:types", "//packages/compiler", - "//packages/compiler-cli/src/ngtsc/annotations/common", "//packages/compiler-cli/src/ngtsc/annotations/directive", "//packages/compiler-cli/src/ngtsc/file_system", "//packages/compiler-cli/src/ngtsc/file_system/testing", diff --git a/packages/compiler-cli/src/ngtsc/annotations/directive/test/directive_spec.ts b/packages/compiler-cli/src/ngtsc/annotations/directive/test/directive_spec.ts index f17c1868a0c..8f5e420b818 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/directive/test/directive_spec.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/directive/test/directive_spec.ts @@ -11,13 +11,12 @@ import ts from 'typescript'; import {absoluteFrom} from '../../../file_system'; import {runInEachFileSystem} from '../../../file_system/testing'; import {ReferenceEmitter} from '../../../imports'; -import {CompoundMetadataReader, DtsMetadataReader, LocalMetadataRegistry} from '../../../metadata'; +import {CompoundMetadataReader, DtsMetadataReader, InjectableClassRegistry, LocalMetadataRegistry} from '../../../metadata'; import {PartialEvaluator} from '../../../partial_evaluator'; import {NOOP_PERF_RECORDER} from '../../../perf'; import {ClassDeclaration, isNamedClassDeclaration, TypeScriptReflectionHost} from '../../../reflection'; import {LocalModuleScopeRegistry, MetadataDtsModuleScopeResolver} from '../../../scope'; import {getDeclaration, makeProgram} from '../../../testing'; -import {InjectableClassRegistry} from '../../common'; import {DirectiveDecoratorHandler} from '../index'; runInEachFileSystem(() => { @@ -168,11 +167,10 @@ runInEachFileSystem(() => { const scopeRegistry = new LocalModuleScopeRegistry( metaReader, new CompoundMetadataReader([metaReader, dtsReader]), new MetadataDtsModuleScopeResolver(dtsReader, null), new ReferenceEmitter([]), null); - const injectableRegistry = new InjectableClassRegistry(reflectionHost, /* isCore */ false); + const injectableRegistry = new InjectableClassRegistry(reflectionHost); const handler = new DirectiveDecoratorHandler( reflectionHost, evaluator, scopeRegistry, scopeRegistry, metaReader, injectableRegistry, /*isCore*/ false, - /*strictCtorDeps*/ false, /*semanticDepGraphUpdater*/ null, /*annotateForClosureCompiler*/ false, /*detectUndecoratedClassesWithAngularFeatures*/ false, NOOP_PERF_RECORDER); diff --git a/packages/compiler-cli/src/ngtsc/annotations/ng_module/src/handler.ts b/packages/compiler-cli/src/ngtsc/annotations/ng_module/src/handler.ts index a7dd4b9d383..9bdeadc4f80 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/ng_module/src/handler.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/ng_module/src/handler.ts @@ -12,7 +12,7 @@ import ts from 'typescript'; import {ErrorCode, FatalDiagnosticError, makeDiagnostic, makeRelatedInformation} from '../../../diagnostics'; import {assertSuccessfulReferenceEmit, Reference, ReferenceEmitter} from '../../../imports'; import {isArrayEqual, isReferenceEqual, isSymbolEqual, SemanticReference, SemanticSymbol} from '../../../incremental/semantic_graph'; -import {MetadataReader, MetadataRegistry, MetaKind} from '../../../metadata'; +import {InjectableClassRegistry, MetadataReader, MetadataRegistry, MetaKind} from '../../../metadata'; import {DynamicValue, PartialEvaluator, ResolvedValue, SyntheticValue} from '../../../partial_evaluator'; import {PerfEvent, PerfRecorder} from '../../../perf'; import {ClassDeclaration, Decorator, isNamedClassDeclaration, ReflectionHost, reflectObjectLiteral, typeNodeToValueExpr} from '../../../reflection'; @@ -21,7 +21,7 @@ import {getDiagnosticNode} from '../../../scope/src/util'; import {FactoryTracker} from '../../../shims/api'; import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence, ResolveResult} from '../../../transform'; import {getSourceFile} from '../../../util/src/typescript'; -import {combineResolvers, compileDeclareFactory, compileNgFactoryDefField, createValueHasWrongTypeError, extractClassMetadata, extractSchemas, findAngularDecorator, forwardRefResolver, getProviderDiagnostics, getValidConstructorDependencies, InjectableClassRegistry, isExpressionForwardReference, ReferencesRegistry, resolveProvidersRequiringFactory, toR3Reference, unwrapExpression, wrapFunctionExpressionsInParens, wrapTypeReference} from '../../common'; +import {combineResolvers, compileDeclareFactory, compileNgFactoryDefField, createValueHasWrongTypeError, extractClassMetadata, extractSchemas, findAngularDecorator, forwardRefResolver, getProviderDiagnostics, getValidConstructorDependencies, isExpressionForwardReference, ReferencesRegistry, resolveProvidersRequiringFactory, toR3Reference, unwrapExpression, wrapFunctionExpressionsInParens, wrapTypeReference} from '../../common'; export interface NgModuleAnalysis { mod: R3NgModuleMetadata; @@ -424,9 +424,7 @@ export class NgModuleDecoratorHandler implements }); } - this.injectableRegistry.registerInjectable(node, { - ctorDeps: analysis.fac.deps, - }); + this.injectableRegistry.registerInjectable(node); } resolve(node: ClassDeclaration, analysis: Readonly): diff --git a/packages/compiler-cli/src/ngtsc/annotations/ng_module/test/ng_module_spec.ts b/packages/compiler-cli/src/ngtsc/annotations/ng_module/test/ng_module_spec.ts index 5ebe2df59e3..f884bd47cd6 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/ng_module/test/ng_module_spec.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/ng_module/test/ng_module_spec.ts @@ -12,13 +12,13 @@ import ts from 'typescript'; import {absoluteFrom} from '../../../file_system'; import {runInEachFileSystem} from '../../../file_system/testing'; import {LocalIdentifierStrategy, ReferenceEmitter} from '../../../imports'; -import {CompoundMetadataReader, DtsMetadataReader, LocalMetadataRegistry} from '../../../metadata'; +import {CompoundMetadataReader, DtsMetadataReader, InjectableClassRegistry, LocalMetadataRegistry} from '../../../metadata'; import {PartialEvaluator} from '../../../partial_evaluator'; import {NOOP_PERF_RECORDER} from '../../../perf'; import {isNamedClassDeclaration, TypeScriptReflectionHost} from '../../../reflection'; import {LocalModuleScopeRegistry, MetadataDtsModuleScopeResolver} from '../../../scope'; import {getDeclaration, makeProgram} from '../../../testing'; -import {InjectableClassRegistry, NoopReferencesRegistry} from '../../common'; +import {NoopReferencesRegistry} from '../../common'; import {NgModuleDecoratorHandler} from '../src/handler'; runInEachFileSystem(() => { @@ -67,7 +67,7 @@ runInEachFileSystem(() => { metaRegistry, metaReader, new MetadataDtsModuleScopeResolver(dtsReader, null), new ReferenceEmitter([]), null); const refEmitter = new ReferenceEmitter([new LocalIdentifierStrategy()]); - const injectableRegistry = new InjectableClassRegistry(reflectionHost, /* isCore */ false); + const injectableRegistry = new InjectableClassRegistry(reflectionHost); const handler = new NgModuleDecoratorHandler( reflectionHost, evaluator, metaReader, metaRegistry, scopeRegistry, referencesRegistry, diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/injectable.ts b/packages/compiler-cli/src/ngtsc/annotations/src/injectable.ts index 722ddde3768..0d63dcbdb37 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/injectable.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/injectable.ts @@ -9,13 +9,12 @@ import {compileClassMetadata, CompileClassMetadataFn, compileDeclareClassMetadata, compileDeclareInjectableFromMetadata, compileInjectable, createMayBeForwardRefExpression, FactoryTarget, ForwardRefHandling, LiteralExpr, MaybeForwardRefExpression, R3ClassMetadata, R3CompiledExpression, R3DependencyMetadata, R3InjectableMetadata, WrappedNodeExpr} from '@angular/compiler'; import ts from 'typescript'; -import {InjectableClassRegistry, isAbstractClassDeclaration} from '../../annotations/common'; import {ErrorCode, FatalDiagnosticError} from '../../diagnostics'; -import {PartialEvaluator} from '../../partial_evaluator'; +import {InjectableClassRegistry} from '../../metadata'; import {PerfEvent, PerfRecorder} from '../../perf'; import {ClassDeclaration, Decorator, ReflectionHost, reflectObjectLiteral} from '../../reflection'; -import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence, ResolveResult} from '../../transform'; -import {checkInheritanceOfInjectable, compileDeclareFactory, CompileFactoryFn, compileNgFactoryDefField, extractClassMetadata, findAngularDecorator, getConstructorDependencies, getValidConstructorDependencies, isAngularCore, toFactoryMetadata, tryUnwrapForwardRef, unwrapConstructorDependencies, validateConstructorDependencies, wrapTypeReference,} from '../common'; +import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence} from '../../transform'; +import {compileDeclareFactory, CompileFactoryFn, compileNgFactoryDefField, extractClassMetadata, findAngularDecorator, getConstructorDependencies, getValidConstructorDependencies, isAngularCore, toFactoryMetadata, tryUnwrapForwardRef, unwrapConstructorDependencies, validateConstructorDependencies, wrapTypeReference} from '../common'; export interface InjectableHandlerData { meta: R3InjectableMetadata; @@ -30,8 +29,7 @@ export interface InjectableHandlerData { export class InjectableDecoratorHandler implements DecoratorHandler { constructor( - private reflector: ReflectionHost, private evaluator: PartialEvaluator, - private isCore: boolean, private strictCtorDeps: boolean, + private reflector: ReflectionHost, private isCore: boolean, private strictCtorDeps: boolean, private injectableRegistry: InjectableClassRegistry, private perf: PerfRecorder, /** * What to do if the injectable already contains a ɵprov property. @@ -85,26 +83,8 @@ export class InjectableDecoratorHandler implements return null; } - register(node: ClassDeclaration, analysis: InjectableHandlerData): void { - this.injectableRegistry.registerInjectable(node, { - ctorDeps: analysis.ctorDeps, - }); - } - - resolve(node: ClassDeclaration, analysis: Readonly, symbol: null): - ResolveResult { - if (requiresValidCtor(analysis.meta)) { - const diagnostic = checkInheritanceOfInjectable( - node, this.injectableRegistry, this.reflector, this.evaluator, this.strictCtorDeps, - 'Injectable'); - if (diagnostic !== null) { - return { - diagnostics: [diagnostic], - }; - } - } - - return {}; + register(node: ClassDeclaration): void { + this.injectableRegistry.registerInjectable(node); } compileFull(node: ClassDeclaration, analysis: Readonly): CompileResult[] { @@ -264,7 +244,7 @@ function extractInjectableCtorDeps( // To deal with this, @Injectable() without an argument is more lenient, and if the // constructor signature does not work for DI then a factory definition (ɵfac) that throws is // generated. - if (strictCtorDeps && !isAbstractClassDeclaration(clazz)) { + if (strictCtorDeps) { ctorDeps = getValidConstructorDependencies(clazz, reflector, isCore); } else { ctorDeps = @@ -275,9 +255,9 @@ function extractInjectableCtorDeps( } else if (decorator.args.length === 1) { const rawCtorDeps = getConstructorDependencies(clazz, reflector, isCore); - if (strictCtorDeps && !isAbstractClassDeclaration(clazz) && requiresValidCtor(meta)) { - // Since use* was not provided for a concrete class, validate the deps according to - // strictCtorDeps. + if (strictCtorDeps && meta.useValue === undefined && meta.useExisting === undefined && + meta.useClass === undefined && meta.useFactory === undefined) { + // Since use* was not provided, validate the deps according to strictCtorDeps. ctorDeps = validateConstructorDependencies(clazz, rawCtorDeps); } else { ctorDeps = unwrapConstructorDependencies(rawCtorDeps); @@ -287,11 +267,6 @@ function extractInjectableCtorDeps( return ctorDeps; } -function requiresValidCtor(meta: R3InjectableMetadata): boolean { - return meta.useValue === undefined && meta.useExisting === undefined && - meta.useClass === undefined && meta.useFactory === undefined; -} - function getDep(dep: ts.Expression, reflector: ReflectionHost): R3DependencyMetadata { const meta: R3DependencyMetadata = { token: new WrappedNodeExpr(dep), diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/pipe.ts b/packages/compiler-cli/src/ngtsc/annotations/src/pipe.ts index 6c333c6e27c..97fcd3d8ee2 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/pipe.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/pipe.ts @@ -12,13 +12,13 @@ import ts from 'typescript'; import {ErrorCode, FatalDiagnosticError} from '../../diagnostics'; import {Reference} from '../../imports'; import {SemanticSymbol} from '../../incremental/semantic_graph'; -import {MetadataRegistry, MetaKind} from '../../metadata'; +import {InjectableClassRegistry, MetadataRegistry, MetaKind} from '../../metadata'; import {PartialEvaluator} from '../../partial_evaluator'; import {PerfEvent, PerfRecorder} from '../../perf'; import {ClassDeclaration, Decorator, ReflectionHost, reflectObjectLiteral} from '../../reflection'; import {LocalModuleScopeRegistry} from '../../scope'; import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence, ResolveResult} from '../../transform'; -import {compileDeclareFactory, compileNgFactoryDefField, compileResults, createValueHasWrongTypeError, extractClassMetadata, findAngularDecorator, getValidConstructorDependencies, InjectableClassRegistry, makeDuplicateDeclarationError, toFactoryMetadata, unwrapExpression, wrapTypeReference,} from '../common'; +import {compileDeclareFactory, compileNgFactoryDefField, compileResults, createValueHasWrongTypeError, extractClassMetadata, findAngularDecorator, getValidConstructorDependencies, makeDuplicateDeclarationError, toFactoryMetadata, unwrapExpression, wrapTypeReference} from '../common'; export interface PipeHandlerData { meta: R3PipeMetadata; @@ -161,9 +161,7 @@ export class PipeDecoratorHandler implements isStandalone: analysis.meta.isStandalone, }); - this.injectableRegistry.registerInjectable(node, { - ctorDeps: analysis.meta.deps, - }); + this.injectableRegistry.registerInjectable(node); } resolve(node: ClassDeclaration): ResolveResult { diff --git a/packages/compiler-cli/src/ngtsc/annotations/test/BUILD.bazel b/packages/compiler-cli/src/ngtsc/annotations/test/BUILD.bazel index 2ee2f98ddc4..bfe1897493b 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/test/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/annotations/test/BUILD.bazel @@ -12,7 +12,6 @@ ts_library( "//packages:types", "//packages/compiler", "//packages/compiler-cli/src/ngtsc/annotations", - "//packages/compiler-cli/src/ngtsc/annotations/common", "//packages/compiler-cli/src/ngtsc/cycles", "//packages/compiler-cli/src/ngtsc/diagnostics", "//packages/compiler-cli/src/ngtsc/file_system", diff --git a/packages/compiler-cli/src/ngtsc/annotations/test/injectable_spec.ts b/packages/compiler-cli/src/ngtsc/annotations/test/injectable_spec.ts index 6ecd3b24ce7..901463b7e4a 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/test/injectable_spec.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/test/injectable_spec.ts @@ -5,11 +5,10 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ -import {InjectableClassRegistry} from '../../annotations/common'; import {ErrorCode, FatalDiagnosticError, ngErrorCode} from '../../diagnostics'; import {absoluteFrom} from '../../file_system'; import {runInEachFileSystem} from '../../file_system/testing'; -import {PartialEvaluator} from '../../partial_evaluator'; +import {InjectableClassRegistry} from '../../metadata'; import {NOOP_PERF_RECORDER} from '../../perf'; import {isNamedClassDeclaration, TypeScriptReflectionHost} from '../../reflection'; import {getDeclaration, makeProgram} from '../../testing'; @@ -68,10 +67,9 @@ function setupHandler(errorOnDuplicateProv: boolean) { ]); const checker = program.getTypeChecker(); const reflectionHost = new TypeScriptReflectionHost(checker); - const injectableRegistry = new InjectableClassRegistry(reflectionHost, /* isCore */ false); - const evaluator = new PartialEvaluator(reflectionHost, checker, null); + const injectableRegistry = new InjectableClassRegistry(reflectionHost); const handler = new InjectableDecoratorHandler( - reflectionHost, evaluator, /* isCore */ false, + reflectionHost, /* isCore */ false, /* strictCtorDeps */ false, injectableRegistry, NOOP_PERF_RECORDER, errorOnDuplicateProv); const TestClass = getDeclaration(program, ENTRY_FILE, 'TestClass', isNamedClassDeclaration); const ɵprov = reflectionHost.getMembersOfClass(TestClass).find(member => member.name === 'ɵprov'); diff --git a/packages/compiler-cli/src/ngtsc/core/BUILD.bazel b/packages/compiler-cli/src/ngtsc/core/BUILD.bazel index e0894f405f4..0dc379f3a5d 100644 --- a/packages/compiler-cli/src/ngtsc/core/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/core/BUILD.bazel @@ -13,7 +13,6 @@ ts_library( "//packages:types", "//packages/compiler", "//packages/compiler-cli/src/ngtsc/annotations", - "//packages/compiler-cli/src/ngtsc/annotations/common", "//packages/compiler-cli/src/ngtsc/cycles", "//packages/compiler-cli/src/ngtsc/diagnostics", "//packages/compiler-cli/src/ngtsc/entry_point", diff --git a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts index 0d992ec5d56..819f8256e28 100644 --- a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts +++ b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts @@ -9,7 +9,6 @@ import ts from 'typescript'; import {ComponentDecoratorHandler, DirectiveDecoratorHandler, InjectableDecoratorHandler, NgModuleDecoratorHandler, NoopReferencesRegistry, PipeDecoratorHandler, ReferencesRegistry} from '../../annotations'; -import {InjectableClassRegistry} from '../../annotations/common'; import {CycleAnalyzer, CycleHandlingStrategy, ImportGraph} from '../../cycles'; import {COMPILER_ERRORS_WITH_GUIDES, ERROR_DETAILS_PAGE_BASE_URL, ErrorCode, ngErrorCode} from '../../diagnostics'; import {checkForPrivateExports, ReferenceGraph} from '../../entry_point'; @@ -18,7 +17,7 @@ import {AbsoluteModuleStrategy, AliasingHost, AliasStrategy, DefaultImportTracke import {IncrementalBuildStrategy, IncrementalCompilation, IncrementalState} from '../../incremental'; import {SemanticSymbol} from '../../incremental/semantic_graph'; import {generateAnalysis, IndexedComponent, IndexingContext} from '../../indexer'; -import {ComponentResources, CompoundMetadataReader, CompoundMetadataRegistry, DirectiveMeta, DtsMetadataReader, LocalMetadataRegistry, MetadataReader, PipeMeta, ResourceRegistry} from '../../metadata'; +import {ComponentResources, CompoundMetadataReader, CompoundMetadataRegistry, DirectiveMeta, DtsMetadataReader, InjectableClassRegistry, LocalMetadataRegistry, MetadataReader, PipeMeta, ResourceRegistry} from '../../metadata'; import {PartialEvaluator} from '../../partial_evaluator'; import {ActivePerfRecorder, DelegatingPerfRecorder, PerfCheckpoint, PerfEvent, PerfPhase} from '../../perf'; import {FileUpdate, ProgramDriver, UpdateMode} from '../../program_driver'; @@ -959,8 +958,6 @@ export class NgCompiler { aliasingHost = new UnifiedModulesAliasingHost(this.adapter.unifiedModulesHost); } - const isCore = isAngularCorePackage(this.inputProgram); - const evaluator = new PartialEvaluator(reflector, checker, this.incrementalCompilation.depGraph); const dtsReader = new DtsMetadataReader(checker, reflector); @@ -976,7 +973,7 @@ export class NgCompiler { new CompoundComponentScopeReader([ngModuleScopeRegistry, standaloneScopeReader]); const semanticDepGraphUpdater = this.incrementalCompilation.semanticDepGraphUpdater; const metaRegistry = new CompoundMetadataRegistry([localMetaRegistry, ngModuleScopeRegistry]); - const injectableRegistry = new InjectableClassRegistry(reflector, isCore); + const injectableRegistry = new InjectableClassRegistry(reflector); const typeCheckScopeRegistry = new TypeCheckScopeRegistry(scopeReader, metaReader); @@ -995,6 +992,8 @@ export class NgCompiler { const dtsTransforms = new DtsTransformRegistry(); + const isCore = isAngularCorePackage(this.inputProgram); + const resourceRegistry = new ResourceRegistry(); // Note: If this compilation builds `@angular/core`, we always build in full compilation @@ -1011,13 +1010,11 @@ export class NgCompiler { CycleHandlingStrategy.UseRemoteScoping : CycleHandlingStrategy.Error; - const strictCtorDeps = this.options.strictInjectionParameters || false; - // Set up the IvyCompilation, which manages state for the Ivy transformer. const handlers: DecoratorHandler[] = [ new ComponentDecoratorHandler( reflector, evaluator, metaRegistry, metaReader, scopeReader, depScopeReader, - ngModuleScopeRegistry, typeCheckScopeRegistry, resourceRegistry, isCore, strictCtorDeps, + ngModuleScopeRegistry, typeCheckScopeRegistry, resourceRegistry, isCore, this.resourceManager, this.adapter.rootDirs, this.options.preserveWhitespaces || false, this.options.i18nUseExternalIds !== false, this.options.enableI18nLegacyMessageIdFormat !== false, this.usePoisonedData, @@ -1031,7 +1028,7 @@ export class NgCompiler { // clang-format off new DirectiveDecoratorHandler( reflector, evaluator, metaRegistry, ngModuleScopeRegistry, metaReader, - injectableRegistry, isCore, strictCtorDeps, semanticDepGraphUpdater, + injectableRegistry, isCore, semanticDepGraphUpdater, this.closureCompilerEnabled, /** compileUndecoratedClassesWithAngularFeatures */ false, this.delegatingPerfRecorder, ) as Readonly>, @@ -1042,7 +1039,7 @@ export class NgCompiler { reflector, evaluator, metaRegistry, ngModuleScopeRegistry, injectableRegistry, isCore, this.delegatingPerfRecorder), new InjectableDecoratorHandler( - reflector, evaluator, isCore, strictCtorDeps, injectableRegistry, + reflector, isCore, this.options.strictInjectionParameters || false, injectableRegistry, this.delegatingPerfRecorder), new NgModuleDecoratorHandler( reflector, evaluator, metaReader, metaRegistry, ngModuleScopeRegistry, referencesRegistry, 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 923e775f8bd..cf6a07ef4d9 100644 --- a/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts +++ b/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts @@ -72,12 +72,6 @@ export enum ErrorCode { */ COMPONENT_UNKNOWN_IMPORT = 2012, - /** - * Raised when a type with Angular decorator inherits its constructor from a base class - * which has a constructor that is incompatible with Angular DI. - */ - INJECTABLE_INHERITS_INVALID_CONSTRUCTOR = 2013, - SYMBOL_NOT_EXPORTED = 3001, SYMBOL_EXPORTED_UNDER_DIFFERENT_NAME = 3002, /** diff --git a/packages/compiler-cli/src/ngtsc/metadata/index.ts b/packages/compiler-cli/src/ngtsc/metadata/index.ts index 1f659fe98e6..30a12ab42a7 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/index.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/index.ts @@ -9,7 +9,7 @@ export * from './src/api'; export {DtsMetadataReader} from './src/dts'; export {flattenInheritedDirectiveMetadata} from './src/inheritance'; -export {CompoundMetadataRegistry, LocalMetadataRegistry} from './src/registry'; +export {CompoundMetadataRegistry, LocalMetadataRegistry, InjectableClassRegistry} from './src/registry'; export {ResourceRegistry, Resource, ComponentResources, isExternalResource, ExternalResource} from './src/resource_registry'; -export {extractDirectiveTypeCheckMeta, hasInjectableFields, CompoundMetadataReader} from './src/util'; +export {extractDirectiveTypeCheckMeta, CompoundMetadataReader} from './src/util'; export {BindingPropertyName, ClassPropertyMapping, ClassPropertyName, InputOrOutput} from './src/property_mapping'; diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/registry.ts b/packages/compiler-cli/src/ngtsc/metadata/src/registry.ts index 5206d718f40..c36efbfbb6c 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/registry.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/registry.ts @@ -7,9 +7,10 @@ */ import {Reference} from '../../imports'; -import {ClassDeclaration} from '../../reflection'; +import {ClassDeclaration, ReflectionHost} from '../../reflection'; import {DirectiveMeta, MetadataReader, MetadataRegistry, NgModuleMeta, PipeMeta} from './api'; +import {hasInjectableFields} from './util'; /** * A registry of directive, pipe, and module metadata for types defined in the current compilation @@ -65,3 +66,24 @@ export class CompoundMetadataRegistry implements MetadataRegistry { } } } + +/** + * Registry that keeps track of classes that can be constructed via dependency injection (e.g. + * injectables, directives, pipes). + */ +export class InjectableClassRegistry { + private classes = new Set(); + + constructor(private host: ReflectionHost) {} + + registerInjectable(declaration: ClassDeclaration): void { + this.classes.add(declaration); + } + + isInjectable(declaration: ClassDeclaration): boolean { + // Figure out whether the class is injectable based on the registered classes, otherwise + // fall back to looking at its members since we might not have been able register the class + // if it was compiled already. + return this.classes.has(declaration) || hasInjectableFields(declaration, this.host); + } +} diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index 4e2c49fa3fc..1f42439a266 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -2672,234 +2672,6 @@ function allTests(os: string) { expect(jsContents) .toMatch(/function Test_Factory\(t\) { i0\.ɵɵinvalidFactory\(\)/ms); }); - - it('should not give a compile-time error if an invalid @Injectable without providedIn is an abstract class', - () => { - env.tsconfig({strictInjectionParameters: true}); - env.write('test.ts', ` - import {Injectable} from '@angular/core'; - - @Injectable() - export abstract class Test { - constructor(private notInjectable: string) {} - } - `); - - env.driveMain(); - const jsContents = env.getContents('test.js'); - expect(jsContents) - .toMatch(/function Test_Factory\(t\) { i0\.ɵɵinvalidFactory\(\)/ms); - }); - - it('should not give a compile-time error if an invalid @Injectable with providedIn is an abstract class', - () => { - env.tsconfig({strictInjectionParameters: true}); - env.write('test.ts', ` - import {Injectable} from '@angular/core'; - - @Injectable({ - providedIn: 'root', - }) - export abstract class Test { - constructor(private notInjectable: string) {} - } - `); - - env.driveMain(); - const jsContents = env.getContents('test.js'); - expect(jsContents) - .toMatch(/function Test_Factory\(t\) { i0\.ɵɵinvalidFactory\(\)/ms); - }); - - it('should give a compile-time error when a derived Directive inherits an invalid constructor', () => { - env.tsconfig({strictInjectionParameters: true}); - env.write('test.ts', ` - import {Directive} from '@angular/core'; - - @Directive() - export abstract class ParentDir { - constructor(private notInjectable: string) {} - } - - @Directive() - export abstract class AbstractMiddleDir extends ParentDir {} - - @Directive() - export class ConcreteMiddleDir extends AbstractMiddleDir {} - - @Directive() - export class ConcreteDirWithoutCtor extends ConcreteMiddleDir {} - - @Directive() - export class ConcreteDirWithCtor extends ConcreteMiddleDir { - constructor() { - super('correct'); - } - } - - @Directive() - export class ConcreteDerivedDirWithoutCtor extends ConcreteDirWithCtor {} - `); - - const diags = env.driveDiagnostics(); - expect(diags.length).toBe(2); - expect(diags[0].code) - .toBe(ngErrorCode(ErrorCode.INJECTABLE_INHERITS_INVALID_CONSTRUCTOR)); - expect(diags[0].messageText) - .toEqual( - `The directive ConcreteMiddleDir inherits its constructor from ParentDir, but the latter has ` + - `a constructor parameter that is not compatible with dependency injection. Either add an explicit ` + - `constructor to ConcreteMiddleDir or change ParentDir's constructor to use parameters that are ` + - `valid for DI.`); - expect(getDiagnosticSourceCode(diags[0])).toBe('ConcreteMiddleDir'); - - expect(diags[1].code) - .toBe(ngErrorCode(ErrorCode.INJECTABLE_INHERITS_INVALID_CONSTRUCTOR)); - expect(diags[1].messageText) - .toEqual( - `The directive ConcreteDirWithoutCtor inherits its constructor from ParentDir, but the latter ` + - `has a constructor parameter that is not compatible with dependency injection. Either add an ` + - `explicit constructor to ConcreteDirWithoutCtor or change ParentDir's constructor to use ` + - `parameters that are valid for DI.`); - expect(getDiagnosticSourceCode(diags[1])).toBe('ConcreteDirWithoutCtor'); - }); - - it('should give a compile-time error when a derived Injectable inherits an invalid constructor', () => { - env.tsconfig({strictInjectionParameters: true}); - env.write('test.ts', ` - import {Injectable} from '@angular/core'; - - @Injectable() - export abstract class ParentService { - constructor(private notInjectable: string) {} - } - - @Injectable() - export abstract class AbstractMiddleService extends ParentService {} - - @Injectable() - export class ConcreteMiddleService extends AbstractMiddleService {} - - @Injectable() - export class ConcreteServiceWithoutCtor extends ConcreteMiddleService {} - - @Injectable() - export class ConcreteServiceWithCtor extends ConcreteMiddleService { - constructor() { - super('correct'); - } - } - - @Injectable() - export class ConcreteDerivedServiceWithoutCtor extends ConcreteServiceWithCtor {} - - @Injectable({ providedIn: 'root' }) - export class ProvidedInRootService extends ParentService {} - - @Injectable({ providedIn: 'root', useFactory: () => null }) - export class UseFactoryService extends ParentService {} - - @Injectable({ providedIn: 'root', useValue: null }) - export class UseValueService extends ParentService {} - - @Injectable({ providedIn: 'root', useClass: ConcreteServiceWithCtor }) - export class UseClassService extends ParentService {} - - @Injectable({ providedIn: 'root', useExisting: ConcreteServiceWithCtor }) - export class UseExistingService extends ParentService {} - `); - - const diags = env.driveDiagnostics(); - expect(diags.length).toBe(3); - expect(diags[0].code) - .toBe(ngErrorCode(ErrorCode.INJECTABLE_INHERITS_INVALID_CONSTRUCTOR)); - expect(diags[0].messageText) - .toEqual( - `The injectable ConcreteMiddleService inherits its constructor from ParentService, but the ` + - `latter has a constructor parameter that is not compatible with dependency injection. Either add ` + - `an explicit constructor to ConcreteMiddleService or change ParentService's constructor to use ` + - `parameters that are valid for DI.`); - expect(getDiagnosticSourceCode(diags[0])).toBe('ConcreteMiddleService'); - - expect(diags[1].code) - .toBe(ngErrorCode(ErrorCode.INJECTABLE_INHERITS_INVALID_CONSTRUCTOR)); - expect(diags[1].messageText) - .toEqual( - `The injectable ConcreteServiceWithoutCtor inherits its constructor from ParentService, but the ` + - `latter has a constructor parameter that is not compatible with dependency injection. Either add ` + - `an explicit constructor to ConcreteServiceWithoutCtor or change ParentService's constructor to ` + - `use parameters that are valid for DI.`); - expect(getDiagnosticSourceCode(diags[1])).toBe('ConcreteServiceWithoutCtor'); - - expect(diags[2].code) - .toBe(ngErrorCode(ErrorCode.INJECTABLE_INHERITS_INVALID_CONSTRUCTOR)); - expect(diags[2].messageText) - .toEqual( - `The injectable ProvidedInRootService inherits its constructor from ParentService, but the ` + - `latter has a constructor parameter that is not compatible with dependency injection. Either add ` + - `an explicit constructor to ProvidedInRootService or change ParentService's constructor to use ` + - `parameters that are valid for DI.`); - expect(getDiagnosticSourceCode(diags[2])).toBe('ProvidedInRootService'); - }); - - it('should give a compile-time error when a derived Directive inherits from a non-decorated class', () => { - env.tsconfig({strictInjectionParameters: true}); - env.write('test.ts', ` - import {Directive} from '@angular/core'; - - export abstract class ParentClass { - constructor(private notInjectable: string) {} - } - - @Directive() - export abstract class AbstractMiddleDir extends ParentClass {} - - @Directive() - export class ConcreteMiddleDir extends AbstractMiddleDir {} - - @Directive() - export class ConcreteDirWithoutCtor extends ConcreteMiddleDir {} - - @Directive() - export class ConcreteDirWithCtor extends ConcreteMiddleDir { - constructor() { - super('correct'); - } - } - - @Directive() - export class ConcreteDerivedDirWithoutCtor extends ConcreteDirWithCtor {} - `); - - const diags = env.driveDiagnostics(); - expect(diags.length).toBe(3); - expect(diags[0].code).toBe(ngErrorCode(ErrorCode.DIRECTIVE_INHERITS_UNDECORATED_CTOR)); - expect(diags[0].messageText) - .toEqual( - `The directive AbstractMiddleDir inherits its constructor from ParentClass, but the latter ` + - `does not have an Angular decorator of its own. Dependency injection will not be able to resolve ` + - `the parameters of ParentClass's constructor. Either add a @Directive decorator to ParentClass, ` + - `or add an explicit constructor to AbstractMiddleDir.`); - expect(getDiagnosticSourceCode(diags[0])).toBe('AbstractMiddleDir'); - - expect(diags[1].code).toBe(ngErrorCode(ErrorCode.DIRECTIVE_INHERITS_UNDECORATED_CTOR)); - expect(diags[1].messageText) - .toEqual( - `The directive ConcreteMiddleDir inherits its constructor from ParentClass, but the latter ` + - `does not have an Angular decorator of its own. Dependency injection will not be able to resolve ` + - `the parameters of ParentClass's constructor. Either add a @Directive decorator to ParentClass, or ` + - `add an explicit constructor to ConcreteMiddleDir.`); - expect(getDiagnosticSourceCode(diags[1])).toBe('ConcreteMiddleDir'); - - expect(diags[2].code).toBe(ngErrorCode(ErrorCode.DIRECTIVE_INHERITS_UNDECORATED_CTOR)); - expect(diags[2].messageText) - .toEqual( - `The directive ConcreteDirWithoutCtor inherits its constructor from ParentClass, but the latter ` + - `does not have an Angular decorator of its own. Dependency injection will not be able to resolve ` + - `the parameters of ParentClass's constructor. Either add a @Directive decorator to ParentClass, ` + - `or add an explicit constructor to ConcreteDirWithoutCtor.`); - expect(getDiagnosticSourceCode(diags[2])).toBe('ConcreteDirWithoutCtor'); - }); }); describe('with strictInjectionParameters = false', () => { @@ -2936,148 +2708,6 @@ function allTests(os: string) { expect(jsContents) .toContain('Test.ɵfac = function Test_Factory(t) { i0.ɵɵinvalidFactory()'); }); - - it('should compile when a derived Directive inherits an invalid constructor', () => { - env.tsconfig({strictInjectionParameters: false}); - env.write('test.ts', ` - import {Directive} from '@angular/core'; - - @Directive() - export abstract class ParentDir { - constructor(private notInjectable: string) {} - } - - @Directive() - export abstract class AbstractMiddleDir extends ParentDir {} - - @Directive() - export class ConcreteMiddleDir extends AbstractMiddleDir {} - - @Directive() - export class ConcreteDirWithoutCtor extends ConcreteMiddleDir {} - - @Directive() - export class ConcreteDirWithCtor extends ConcreteMiddleDir { - constructor() { - super('correct'); - } - } - - @Directive() - export class ConcreteDerivedDirWithoutCtor extends ConcreteDirWithCtor {} - `); - - env.driveMain(); - }); - - it('should compile when a derived Injectable inherits an invalid constructor', () => { - env.tsconfig({strictInjectionParameters: false}); - env.write('test.ts', ` - import {Injectable} from '@angular/core'; - - @Injectable() - export abstract class ParentService { - constructor(private notInjectable: string) {} - } - - @Injectable() - export abstract class AbstractMiddleService extends ParentService {} - - @Injectable() - export class ConcreteMiddleService extends AbstractMiddleService {} - - @Injectable() - export class ConcreteServiceWithoutCtor extends ConcreteMiddleService {} - - @Injectable() - export class ConcreteServiceWithCtor extends ConcreteMiddleService { - constructor() { - super('correct'); - } - } - - @Injectable() - export class ConcreteDerivedServiceWithoutCtor extends ConcreteServiceWithCtor {} - - @Injectable({ providedIn: 'root' }) - export class ProvidedInRootService extends ParentService {} - - @Injectable({ providedIn: 'root', useFactory: () => null }) - export class UseFactoryService extends ParentService {} - - @Injectable({ providedIn: 'root', useValue: null }) - export class UseValueService extends ParentService {} - - @Injectable({ providedIn: 'root', useClass: ConcreteServiceWithCtor }) - export class UseClassService extends ParentService {} - - @Injectable({ providedIn: 'root', useExisting: ConcreteServiceWithCtor }) - export class UseExistingService extends ParentService {} - `); - - env.driveMain(); - }); - - it('should give a compile-time error when a derived Directive inherits from a non-decorated class', () => { - // Errors for undecorated base classes should always be reported, even under - // `strictInjectionParameters`. - env.tsconfig({strictInjectionParameters: false}); - env.write('test.ts', ` - import {Directive} from '@angular/core'; - - export abstract class ParentClass { - constructor(private notInjectable: string) {} - } - - @Directive() - export abstract class AbstractMiddleDir extends ParentClass {} - - @Directive() - export class ConcreteMiddleDir extends AbstractMiddleDir {} - - @Directive() - export class ConcreteDirWithoutCtor extends ConcreteMiddleDir {} - - @Directive() - export class ConcreteDirWithCtor extends ConcreteMiddleDir { - constructor() { - super('correct'); - } - } - - @Directive() - export class ConcreteDerivedDirWithoutCtor extends ConcreteDirWithCtor {} - `); - - const diags = env.driveDiagnostics(); - expect(diags.length).toBe(3); - expect(diags[0].code).toBe(ngErrorCode(ErrorCode.DIRECTIVE_INHERITS_UNDECORATED_CTOR)); - expect(diags[0].messageText) - .toEqual( - `The directive AbstractMiddleDir inherits its constructor from ParentClass, but the latter ` + - `does not have an Angular decorator of its own. Dependency injection will not be able to resolve ` + - `the parameters of ParentClass's constructor. Either add a @Directive decorator to ParentClass, ` + - `or add an explicit constructor to AbstractMiddleDir.`); - expect(getDiagnosticSourceCode(diags[0])).toBe('AbstractMiddleDir'); - - expect(diags[1].code).toBe(ngErrorCode(ErrorCode.DIRECTIVE_INHERITS_UNDECORATED_CTOR)); - expect(diags[1].messageText) - .toEqual( - `The directive ConcreteMiddleDir inherits its constructor from ParentClass, but the latter ` + - `does not have an Angular decorator of its own. Dependency injection will not be able to resolve ` + - `the parameters of ParentClass's constructor. Either add a @Directive decorator to ParentClass, ` + - `or add an explicit constructor to ConcreteMiddleDir.`); - expect(getDiagnosticSourceCode(diags[1])).toBe('ConcreteMiddleDir'); - - expect(diags[2].code).toBe(ngErrorCode(ErrorCode.DIRECTIVE_INHERITS_UNDECORATED_CTOR)); - expect(diags[2].messageText) - .toEqual( - `The directive ConcreteDirWithoutCtor inherits its constructor from ParentClass, but the latter ` + - `does not have an Angular decorator of its own. Dependency injection will not be able to resolve ` + - `the parameters of ParentClass's constructor. Either add a @Directive decorator to ParentClass, ` + - `or add an explicit constructor to ConcreteDirWithoutCtor.`); - expect(getDiagnosticSourceCode(diags[2])).toBe('ConcreteDirWithoutCtor'); - }); }); });