mirror of
https://github.com/angular/angular
synced 2026-05-24 09:28:37 +00:00
fix(compiler-cli): do not fail fatal when references to non-existent module are discovered (#58515)
Currently when application source code references e.g. an NgModule that points to references that aren't available, the compiler will break at runtime without any actionable/reasonable error. This could happen for example when a library is referenced in code, but the library is simply not available in the `node_modules`. Clearly, TypeScript would issue import diagnostics here, but the Angular compiler shouldn't break fatally. This is useful for migrations which may run against projects which aren't fully compilable. The compiler handles this fine in all cases, except when processing `.d.ts` currently... and the runtime exception invalides all other information of the program etc. This commit fixes this by degrading such unexpected cases for `.d.ts` metadata reading to be handled gracefully. This matches existing logic where the `.d.ts` doesn't necessarily match the "expecation"/"expected format". The worst case is that the Angular compiler will not have type information for some directives of e.g. a library that just isn't installed in local `node_modules`; compared to magical errors and unexpected runtime behavior. PR Close #58515
This commit is contained in:
parent
e85ac5c7cb
commit
d4d76ead80
9 changed files with 194 additions and 26 deletions
|
|
@ -734,6 +734,7 @@ export class NgModuleDecoratorHandler
|
|||
rawExports: analysis.rawExports,
|
||||
decorator: analysis.decorator,
|
||||
mayDeclareProviders: analysis.providers !== null,
|
||||
isPoisoned: false,
|
||||
});
|
||||
|
||||
this.injectableRegistry.registerInjectable(node, {
|
||||
|
|
|
|||
|
|
@ -25,6 +25,12 @@ export interface NgModuleMeta {
|
|||
exports: Reference<ClassDeclaration>[];
|
||||
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.
|
||||
*
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<ClassDeclaration>[] {
|
||||
): {result: Reference<ClassDeclaration>[]; isIncomplete: boolean} {
|
||||
if (!ts.isTupleTypeNode(def)) {
|
||||
return [];
|
||||
return {result: [], isIncomplete: false};
|
||||
}
|
||||
|
||||
return def.elements.map((element) => {
|
||||
const result: Reference<ClassDeclaration>[] = [];
|
||||
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<ClassDeclaration> | 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.
|
||||
|
|
|
|||
|
|
@ -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<RelativeModule, [], never, [typeof i2.InvalidTypesForIt, typeof ValidRef]>;
|
||||
}
|
||||
`,
|
||||
},
|
||||
],
|
||||
{
|
||||
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);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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};
|
||||
|
|
|
|||
|
|
@ -124,7 +124,7 @@ export class MetadataDtsModuleScopeResolver implements DtsModuleScopeResolver {
|
|||
const exportScope: ExportScope = {
|
||||
exported: {
|
||||
dependencies,
|
||||
isPoisoned: false,
|
||||
isPoisoned: meta.isPoisoned,
|
||||
},
|
||||
};
|
||||
this.cache.set(clazz, exportScope);
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
|
|
|
|||
|
|
@ -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<Module, [], never, [typeof InvalidRef]>
|
||||
}
|
||||
`,
|
||||
);
|
||||
|
||||
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(
|
||||
|
|
|
|||
Loading…
Reference in a new issue