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 a349166f0ea..1cfb85862b9 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 @@ -734,6 +734,7 @@ export class NgModuleDecoratorHandler rawExports: analysis.rawExports, decorator: analysis.decorator, mayDeclareProviders: analysis.providers !== null, + isPoisoned: false, }); this.injectableRegistry.registerInjectable(node, { diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/api.ts b/packages/compiler-cli/src/ngtsc/metadata/src/api.ts index 511872bc27a..93d998978a7 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/api.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/api.ts @@ -25,6 +25,12 @@ export interface NgModuleMeta { exports: Reference[]; schemas: SchemaMetadata[]; + /** + * Whether the module had some issue being analyzed. + * This means it likely does not have complete and reliable metadata. + */ + isPoisoned: boolean; + /** * The raw `ts.Expression` which gave rise to `declarations`, if one exists. * diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts b/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts index 28f0a502a07..33716418f64 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts @@ -76,16 +76,36 @@ export class DtsMetadataReader implements MetadataReader { // Read the ModuleData out of the type arguments. const [_, declarationMetadata, importMetadata, exportMetadata] = ngModuleDef.type.typeArguments; + + const declarations = extractReferencesFromType( + this.checker, + declarationMetadata, + ref.bestGuessOwningModule, + ); + const exports = extractReferencesFromType( + this.checker, + exportMetadata, + ref.bestGuessOwningModule, + ); + const imports = extractReferencesFromType( + this.checker, + importMetadata, + ref.bestGuessOwningModule, + ); + + // The module is considered poisoned if it's exports couldn't be + // resolved completely. This would make the module not necessarily + // usable for scope computation relying on this module; so we propagate + // this "incompleteness" information to the caller. + const isPoisoned = exports.isIncomplete; + return { kind: MetaKind.NgModule, ref, - declarations: extractReferencesFromType( - this.checker, - declarationMetadata, - ref.bestGuessOwningModule, - ), - exports: extractReferencesFromType(this.checker, exportMetadata, ref.bestGuessOwningModule), - imports: extractReferencesFromType(this.checker, importMetadata, ref.bestGuessOwningModule), + declarations: declarations.result, + isPoisoned, + exports: exports.result, + imports: imports.result, schemas: [], rawDeclarations: null, rawImports: null, @@ -156,6 +176,11 @@ export class DtsMetadataReader implements MetadataReader { const isSignal = def.type.typeArguments.length > 9 && (readBooleanType(def.type.typeArguments[9]) ?? false); + // At this point in time, the `.d.ts` may not be fully extractable when + // trying to resolve host directive types to their declarations. + // If this cannot be done completely, the metadata is incomplete and "poisoned". + const isPoisoned = hostDirectives !== null && hostDirectives?.isIncomplete; + return { kind: MetaKind.Directive, matchSource: MatchSource.Selector, @@ -166,11 +191,11 @@ export class DtsMetadataReader implements MetadataReader { exportAs: readStringArrayType(def.type.typeArguments[2]), inputs, outputs, - hostDirectives, + hostDirectives: hostDirectives?.result ?? null, queries: readStringArrayType(def.type.typeArguments[5]), ...extractDirectiveTypeCheckMeta(clazz, inputs, this.reflector), baseClass: readBaseClass(clazz, this.checker, this.reflector), - isPoisoned: false, + isPoisoned, isStructural, animationTriggerNames: null, ngContentSelectors, @@ -330,12 +355,13 @@ function readHostDirectivesType( checker: ts.TypeChecker, type: ts.TypeNode, bestGuessOwningModule: OwningModule | null, -): HostDirectiveMeta[] | null { +): {result: HostDirectiveMeta[]; isIncomplete: boolean} | null { if (!ts.isTupleTypeNode(type) || type.elements.length === 0) { return null; } const result: HostDirectiveMeta[] = []; + let isIncomplete = false; for (const hostDirectiveType of type.elements) { const {directive, inputs, outputs} = readMapType(hostDirectiveType, (type) => type); @@ -345,8 +371,14 @@ function readHostDirectivesType( throw new Error(`Expected TypeQueryNode: ${nodeDebugInfo(directive)}`); } + const ref = extraReferenceFromTypeQuery(checker, directive, type, bestGuessOwningModule); + if (ref === null) { + isIncomplete = true; + continue; + } + result.push({ - directive: extraReferenceFromTypeQuery(checker, directive, type, bestGuessOwningModule), + directive: ref, isForwardReference: false, inputs: readMapType(inputs, readStringType), outputs: readMapType(outputs, readStringType), @@ -354,5 +386,5 @@ function readHostDirectivesType( } } - return result.length > 0 ? result : null; + return result.length > 0 ? {result, isIncomplete} : null; } diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/util.ts b/packages/compiler-cli/src/ngtsc/metadata/src/util.ts index ce8aa79342f..e3668894ffb 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/util.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/util.ts @@ -31,23 +31,37 @@ import { TemplateGuardMeta, } from './api'; import {ClassPropertyMapping, ClassPropertyName} from './property_mapping'; +import {TypeEntityToDeclarationError} from '../../reflection/src/typescript'; export function extractReferencesFromType( checker: ts.TypeChecker, def: ts.TypeNode, bestGuessOwningModule: OwningModule | null, -): Reference[] { +): {result: Reference[]; isIncomplete: boolean} { if (!ts.isTupleTypeNode(def)) { - return []; + return {result: [], isIncomplete: false}; } - return def.elements.map((element) => { + const result: Reference[] = []; + let isIncomplete = false; + + for (const element of def.elements) { if (!ts.isTypeQueryNode(element)) { throw new Error(`Expected TypeQueryNode: ${nodeDebugInfo(element)}`); } - return extraReferenceFromTypeQuery(checker, element, def, bestGuessOwningModule); - }); + const ref = extraReferenceFromTypeQuery(checker, element, def, bestGuessOwningModule); + + // Note: Sometimes a reference inside the type tuple/array + // may not be resolvable/existent. We proceed with incomplete data. + if (ref === null) { + isIncomplete = true; + } else { + result.push(ref); + } + } + + return {result, isIncomplete}; } export function extraReferenceFromTypeQuery( @@ -55,12 +69,28 @@ export function extraReferenceFromTypeQuery( typeNode: ts.TypeQueryNode, origin: ts.TypeNode, bestGuessOwningModule: OwningModule | null, -) { +): Reference | null { const type = typeNode.exprName; - const {node, from} = reflectTypeEntityToDeclaration(type, checker); + let node: ts.Declaration; + let from: string | null; + + // Gracefully handle when the type entity could not be converted or + // resolved to its declaration node. + try { + const result = reflectTypeEntityToDeclaration(type, checker); + node = result.node; + from = result.from; + } catch (e) { + if (e instanceof TypeEntityToDeclarationError) { + return null; + } + throw e; + } + if (!isNamedClassDeclaration(node)) { throw new Error(`Expected named ClassDeclaration: ${nodeDebugInfo(node)}`); } + if (from !== null && !from.startsWith('.')) { // The symbol was imported using an absolute module specifier so return a reference that // uses that absolute module specifier as its best guess owning module. diff --git a/packages/compiler-cli/src/ngtsc/metadata/test/dts_spec.ts b/packages/compiler-cli/src/ngtsc/metadata/test/dts_spec.ts index 665015d5c17..4a168a13aaa 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/test/dts_spec.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/test/dts_spec.ts @@ -337,4 +337,42 @@ runInEachFileSystem(() => { expect(meta.inputs.toDirectMappedObject()).toEqual({input: 'input', otherInput: 'alias'}); expect(Array.from(meta.inputs).filter((i) => i.required)).toEqual([]); }); + + it('should not fail fatal at runtime when reference cannot be resolved', () => { + const externalPath = absoluteFrom('/external.d.ts'); + const {program} = makeProgram( + [ + { + name: externalPath, + contents: ` + import * as i0 from '@angular/core'; + import * as i2 from './relative'; + + export class ValidRef {} + + export declare class ExternalModule { + static ɵmod: i0.ɵɵNgModuleDeclaration; + } + `, + }, + ], + { + skipLibCheck: true, + lib: ['es6', 'dom'], + }, + ); + + const externalSf = getSourceFileOrError(program, externalPath); + const clazz = externalSf.statements[3]; + if (!isNamedClassDeclaration(clazz)) { + return fail('Expected class declaration'); + } + + const typeChecker = program.getTypeChecker(); + const dtsReader = new DtsMetadataReader(typeChecker, new TypeScriptReflectionHost(typeChecker)); + + const withoutOwningModule = dtsReader.getNgModuleMetadata(new Reference(clazz))!; + expect(withoutOwningModule.exports.length).toBe(1); + expect(withoutOwningModule.isPoisoned).toBe(true); + }); }); diff --git a/packages/compiler-cli/src/ngtsc/reflection/src/typescript.ts b/packages/compiler-cli/src/ngtsc/reflection/src/typescript.ts index c5e946e3c75..ce176ca50df 100644 --- a/packages/compiler-cli/src/ngtsc/reflection/src/typescript.ts +++ b/packages/compiler-cli/src/ngtsc/reflection/src/typescript.ts @@ -533,13 +533,21 @@ export function reflectIdentifierOfDeclaration(decl: ts.Declaration): ts.Identif return null; } +export class TypeEntityToDeclarationError extends Error {} + +/** + * @throws {TypeEntityToDeclarationError} if the type cannot be converted + * to a declaration. + */ export function reflectTypeEntityToDeclaration( type: ts.EntityName, checker: ts.TypeChecker, ): {node: ts.Declaration; from: string | null} { let realSymbol = checker.getSymbolAtLocation(type); if (realSymbol === undefined) { - throw new Error(`Cannot resolve type entity ${type.getText()} to symbol`); + throw new TypeEntityToDeclarationError( + `Cannot resolve type entity ${type.getText()} to symbol`, + ); } while (realSymbol.flags & ts.SymbolFlags.Alias) { realSymbol = checker.getAliasedSymbol(realSymbol); @@ -551,12 +559,14 @@ export function reflectTypeEntityToDeclaration( } else if (realSymbol.declarations !== undefined && realSymbol.declarations.length === 1) { node = realSymbol.declarations[0]; } else { - throw new Error(`Cannot resolve type entity symbol to declaration`); + throw new TypeEntityToDeclarationError(`Cannot resolve type entity symbol to declaration`); } if (ts.isQualifiedName(type)) { if (!ts.isIdentifier(type.left)) { - throw new Error(`Cannot handle qualified name with non-identifier lhs`); + throw new TypeEntityToDeclarationError( + `Cannot handle qualified name with non-identifier lhs`, + ); } const symbol = checker.getSymbolAtLocation(type.left); if ( @@ -564,20 +574,20 @@ export function reflectTypeEntityToDeclaration( symbol.declarations === undefined || symbol.declarations.length !== 1 ) { - throw new Error(`Cannot resolve qualified type entity lhs to symbol`); + throw new TypeEntityToDeclarationError(`Cannot resolve qualified type entity lhs to symbol`); } const decl = symbol.declarations[0]; if (ts.isNamespaceImport(decl)) { const clause = decl.parent!; const importDecl = clause.parent!; if (!ts.isStringLiteral(importDecl.moduleSpecifier)) { - throw new Error(`Module specifier is not a string`); + throw new TypeEntityToDeclarationError(`Module specifier is not a string`); } return {node, from: importDecl.moduleSpecifier.text}; } else if (ts.isModuleDeclaration(decl)) { return {node, from: null}; } else { - throw new Error(`Unknown import type?`); + throw new TypeEntityToDeclarationError(`Unknown import type?`); } } else { return {node, from: null}; diff --git a/packages/compiler-cli/src/ngtsc/scope/src/dependency.ts b/packages/compiler-cli/src/ngtsc/scope/src/dependency.ts index f7d8da7d219..85069799f0e 100644 --- a/packages/compiler-cli/src/ngtsc/scope/src/dependency.ts +++ b/packages/compiler-cli/src/ngtsc/scope/src/dependency.ts @@ -124,7 +124,7 @@ export class MetadataDtsModuleScopeResolver implements DtsModuleScopeResolver { const exportScope: ExportScope = { exported: { dependencies, - isPoisoned: false, + isPoisoned: meta.isPoisoned, }, }; this.cache.set(clazz, exportScope); diff --git a/packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts b/packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts index f77bce7f8f0..5a3a14e4b70 100644 --- a/packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts +++ b/packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts @@ -79,6 +79,7 @@ describe('LocalModuleScopeRegistry', () => { rawExports: null, decorator: null, mayDeclareProviders: false, + isPoisoned: false, }); const scope = scopeRegistry.getScopeOfModule(Module.node) as LocalModuleScope; @@ -101,6 +102,7 @@ describe('LocalModuleScopeRegistry', () => { rawExports: null, decorator: null, mayDeclareProviders: false, + isPoisoned: false, }); metaRegistry.registerNgModuleMetadata({ kind: MetaKind.NgModule, @@ -114,6 +116,7 @@ describe('LocalModuleScopeRegistry', () => { rawExports: null, decorator: null, mayDeclareProviders: false, + isPoisoned: false, }); metaRegistry.registerNgModuleMetadata({ kind: MetaKind.NgModule, @@ -127,6 +130,7 @@ describe('LocalModuleScopeRegistry', () => { rawExports: null, decorator: null, mayDeclareProviders: false, + isPoisoned: false, }); const scopeA = scopeRegistry.getScopeOfModule(ModuleA.node) as LocalModuleScope; @@ -149,6 +153,7 @@ describe('LocalModuleScopeRegistry', () => { rawExports: null, decorator: null, mayDeclareProviders: false, + isPoisoned: false, }); metaRegistry.registerNgModuleMetadata({ kind: MetaKind.NgModule, @@ -162,6 +167,7 @@ describe('LocalModuleScopeRegistry', () => { rawExports: null, decorator: null, mayDeclareProviders: false, + isPoisoned: false, }); const scopeA = scopeRegistry.getScopeOfModule(ModuleA.node) as LocalModuleScope; @@ -184,6 +190,7 @@ describe('LocalModuleScopeRegistry', () => { rawExports: null, decorator: null, mayDeclareProviders: false, + isPoisoned: false, }); metaRegistry.registerNgModuleMetadata({ kind: MetaKind.NgModule, @@ -197,6 +204,7 @@ describe('LocalModuleScopeRegistry', () => { rawExports: null, decorator: null, mayDeclareProviders: false, + isPoisoned: false, }); metaRegistry.registerNgModuleMetadata({ kind: MetaKind.NgModule, @@ -210,6 +218,7 @@ describe('LocalModuleScopeRegistry', () => { rawExports: null, decorator: null, mayDeclareProviders: false, + isPoisoned: false, }); const scope = scopeRegistry.getScopeOfModule(ModuleA.node) as LocalModuleScope; @@ -239,6 +248,7 @@ describe('LocalModuleScopeRegistry', () => { rawExports: null, decorator: null, mayDeclareProviders: false, + isPoisoned: false, }); const scope = scopeRegistry.getScopeOfModule(Module.node) as LocalModuleScope; @@ -260,6 +270,7 @@ describe('LocalModuleScopeRegistry', () => { rawExports: null, decorator: null, mayDeclareProviders: false, + isPoisoned: false, }); metaRegistry.registerNgModuleMetadata({ kind: MetaKind.NgModule, @@ -273,6 +284,7 @@ describe('LocalModuleScopeRegistry', () => { rawExports: null, decorator: null, mayDeclareProviders: false, + isPoisoned: false, }); const scopeA = scopeRegistry.getScopeOfModule(ModuleA.node) as LocalModuleScope; @@ -294,6 +306,7 @@ describe('LocalModuleScopeRegistry', () => { rawExports: null, decorator: null, mayDeclareProviders: false, + isPoisoned: false, }); metaRegistry.registerNgModuleMetadata({ kind: MetaKind.NgModule, @@ -307,6 +320,7 @@ describe('LocalModuleScopeRegistry', () => { rawExports: null, decorator: null, mayDeclareProviders: false, + isPoisoned: false, }); expect(scopeRegistry.getScopeOfModule(ModuleA.node)!.compilation.isPoisoned).toBeTrue(); diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index 769757a0385..fca81c9bac5 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -1215,6 +1215,43 @@ runInEachFileSystem((os: string) => { ); }); + it( + 'should not throw, but issue a diagnostic when an `NgModule` from `d.ts` references ' + + 'non-existent classes', + () => { + env.tsconfig(); + env.write( + 'test.ts', + ` + import {NgModule} from '@angular/core'; + import {MyModule} from './lib'; + + @NgModule({ + imports: [MyModule], + }) + export class Mod {} + `, + ); + env.write( + 'lib.d.ts', + ` + import * as i0 from '@angular/core'; + import {InvalidRef} from 'other-lib'; + + export declare class MyModule { + static ɵmod: i0.ɵɵNgModuleDeclaration + } + `, + ); + + const diagnostics = env.driveDiagnostics(); + expect(diagnostics.length).toBe(1); + expect(diagnostics[0].messageText).toBe( + 'This import contains errors, which may affect components that depend on this NgModule.', + ); + }, + ); + it('should respect imported module order while processing Directives and Components', () => { env.tsconfig({}); env.write(