Skip to content

Commit c9aa87c

Browse files
alxhubzarend
alxhub
authored and
zarend
committed
fix(compiler-cli): show a more specific error for Ivy NgModules (#41534)
When an Ivy NgModule is imported into a View Engine build, it doesn't have metadata.json files that describe it as an NgModule, so it appears to VE builds as a plain, undecorated class. The error message shown in this situation generic and confusing, since it recommends adding an @NgModule annotation to a class from a library. This commit adds special detection into the View Engine compiler to give a more specific error message when an Ivy NgModule is imported. PR Close #41534
1 parent fe60029 commit c9aa87c

5 files changed

Lines changed: 124 additions & 8 deletions

File tree

‎goldens/public-api/compiler-cli/error_code.d.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ export declare enum ErrorCode {
2929
NGMODULE_MODULE_WITH_PROVIDERS_MISSING_GENERIC = 6005,
3030
NGMODULE_REEXPORT_NAME_COLLISION = 6006,
3131
NGMODULE_DECLARATION_NOT_UNIQUE = 6007,
32+
NGMODULE_VE_DEPENDENCY_ON_IVY_LIB = 6999,
3233
SCHEMA_INVALID_ELEMENT = 8001,
3334
SCHEMA_INVALID_ATTRIBUTE = 8002,
3435
MISSING_REFERENCE_TARGET = 8003,

‎packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,12 @@ export enum ErrorCode {
110110
*/
111111
NGMODULE_DECLARATION_NOT_UNIQUE = 6007,
112112

113+
/**
114+
* Not actually raised by the compiler, but reserved for documentation of a View Engine error when
115+
* a View Engine build depends on an Ivy-compiled NgModule.
116+
*/
117+
NGMODULE_VE_DEPENDENCY_ON_IVY_LIB = 6999,
118+
113119
/**
114120
* An element name failed validation against the DOM schema.
115121
*/

‎packages/compiler-cli/src/transformers/program.ts‎

Lines changed: 65 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* found in the LICENSE file at https://angular.io/license
88
*/
99

10-
import {AotCompiler, AotCompilerOptions, core, createAotCompiler, FormattedMessageChain, GeneratedFile, getParseErrors, isFormattedError, isSyntaxError, MessageBundle, NgAnalyzedFileWithInjectables, NgAnalyzedModules, ParseSourceSpan, PartialModule, Serializer, Xliff, Xliff2, Xmb} from '@angular/compiler';
10+
import {AotCompiler, AotCompilerOptions, core, createAotCompiler, FormattedMessageChain, GeneratedFile, getMissingNgModuleMetadataErrorData, getParseErrors, isFormattedError, isSyntaxError, MessageBundle, NgAnalyzedFileWithInjectables, NgAnalyzedModules, ParseSourceSpan, PartialModule, Serializer, Xliff, Xliff2, Xmb} from '@angular/compiler';
1111
import * as fs from 'fs';
1212
import * as path from 'path';
1313
import * as ts from 'typescript';
@@ -690,7 +690,7 @@ class AngularCompilerProgram implements Program {
690690
private _addStructuralDiagnostics(error: Error) {
691691
const diagnostics = this._structuralDiagnostics || (this._structuralDiagnostics = []);
692692
if (isSyntaxError(error)) {
693-
diagnostics.push(...syntaxErrorToDiagnostics(error));
693+
diagnostics.push(...syntaxErrorToDiagnostics(error, this.tsProgram));
694694
} else {
695695
diagnostics.push({
696696
messageText: error.toString(),
@@ -1036,7 +1036,7 @@ function diagnosticChainFromFormattedDiagnosticChain(chain: FormattedMessageChai
10361036
};
10371037
}
10381038

1039-
function syntaxErrorToDiagnostics(error: Error): Diagnostic[] {
1039+
function syntaxErrorToDiagnostics(error: Error, program: ts.Program): Diagnostic[] {
10401040
const parserErrors = getParseErrors(error);
10411041
if (parserErrors && parserErrors.length) {
10421042
return parserErrors.map<Diagnostic>(e => ({
@@ -1058,6 +1058,33 @@ function syntaxErrorToDiagnostics(error: Error): Diagnostic[] {
10581058
position: error.position
10591059
}];
10601060
}
1061+
1062+
const ngModuleErrorData = getMissingNgModuleMetadataErrorData(error);
1063+
if (ngModuleErrorData !== null) {
1064+
// This error represents the import or export of an `NgModule` that didn't have valid metadata.
1065+
// This _might_ happen because the NgModule in question is an Ivy-compiled library, and we want
1066+
// to show a more useful error if that's the case.
1067+
const ngModuleClass =
1068+
getDtsClass(program, ngModuleErrorData.fileName, ngModuleErrorData.className);
1069+
if (ngModuleClass !== null && isIvyNgModule(ngModuleClass)) {
1070+
return [{
1071+
messageText: `The NgModule '${ngModuleErrorData.className}' in '${
1072+
ngModuleErrorData
1073+
.fileName}' is imported by this compilation, but appears to be part of a library compiled for Angular Ivy. This may occur because:
1074+
1075+
1) the library was processed with 'ngcc'. Removing and reinstalling node_modules may fix this problem.
1076+
1077+
2) the library was published for Angular Ivy and v12+ applications only. Check its peer dependencies carefully and ensure that you're using a compatible version of Angular.
1078+
1079+
See https://angular.io/errors/NG6999 for more information.
1080+
`,
1081+
category: ts.DiagnosticCategory.Error,
1082+
code: DEFAULT_ERROR_CODE,
1083+
source: SOURCE,
1084+
}];
1085+
}
1086+
}
1087+
10611088
// Produce a Diagnostic anyway since we know for sure `error` is a SyntaxError
10621089
return [{
10631090
messageText: error.message,
@@ -1066,3 +1093,38 @@ function syntaxErrorToDiagnostics(error: Error): Diagnostic[] {
10661093
source: SOURCE,
10671094
}];
10681095
}
1096+
1097+
function getDtsClass(program: ts.Program, fileName: string, className: string): ts.ClassDeclaration|
1098+
null {
1099+
const sf = program.getSourceFile(fileName);
1100+
if (sf === undefined || !sf.isDeclarationFile) {
1101+
return null;
1102+
}
1103+
for (const stmt of sf.statements) {
1104+
if (!ts.isClassDeclaration(stmt)) {
1105+
continue;
1106+
}
1107+
if (stmt.name === undefined || stmt.name.text !== className) {
1108+
continue;
1109+
}
1110+
1111+
return stmt;
1112+
}
1113+
1114+
// No classes found that matched the given name.
1115+
return null;
1116+
}
1117+
1118+
function isIvyNgModule(clazz: ts.ClassDeclaration): boolean {
1119+
for (const member of clazz.members) {
1120+
if (!ts.isPropertyDeclaration(member)) {
1121+
continue;
1122+
}
1123+
if (ts.isIdentifier(member.name) && member.name.text === 'ɵmod') {
1124+
return true;
1125+
}
1126+
}
1127+
1128+
// No Ivy 'ɵmod' property found.
1129+
return false;
1130+
}

‎packages/compiler-cli/test/ngc_spec.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,34 @@ describe('ngc transformer command-line', () => {
330330
});
331331
});
332332

333+
it('should give a specific error when an Angular Ivy NgModule is imported', () => {
334+
writeConfig(`{
335+
"extends": "./tsconfig-base.json",
336+
"files": ["mymodule.ts"]
337+
}`);
338+
write('node_modules/test/index.d.ts', `
339+
export declare class FooModule {
340+
static ɵmod = null;
341+
}
342+
`);
343+
write('mymodule.ts', `
344+
import {NgModule} from '@angular/core';
345+
import {FooModule} from 'test';
346+
347+
@NgModule({
348+
imports: [FooModule],
349+
})
350+
export class TestModule {}
351+
`);
352+
353+
const exitCode = main(['-p', basePath], errorSpy);
354+
expect(errorSpy).toHaveBeenCalledTimes(1);
355+
const message = errorSpy.calls.mostRecent().args[0];
356+
357+
// The error message should mention Ivy specifically.
358+
expect(message).toContain('Angular Ivy');
359+
});
360+
333361
describe('compile ngfactory files', () => {
334362
it('should compile ngfactory files that are not referenced by root files', () => {
335363
writeConfig(`{

‎packages/compiler/src/metadata_resolver.ts‎

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,18 @@ export type ErrorCollector = (error: any, type?: any) => void;
2929

3030
export const ERROR_COMPONENT_TYPE = 'ngComponentType';
3131

32+
const MISSING_NG_MODULE_METADATA_ERROR_DATA = 'ngMissingNgModuleMetadataErrorData';
33+
export interface MissingNgModuleMetadataErrorData {
34+
fileName: string;
35+
className: string;
36+
}
37+
38+
39+
export function getMissingNgModuleMetadataErrorData(error: any): MissingNgModuleMetadataErrorData|
40+
null {
41+
return error[MISSING_NG_MODULE_METADATA_ERROR_DATA] ?? null;
42+
}
43+
3244
// Design notes:
3345
// - don't lazily create metadata:
3446
// For some metadata, we need to do async work sometimes,
@@ -563,11 +575,18 @@ export class CompileMetadataResolver {
563575
this.getNgModuleSummary(importedModuleType, alreadyCollecting);
564576
alreadyCollecting.delete(importedModuleType);
565577
if (!importedModuleSummary) {
566-
this._reportError(
567-
syntaxError(`Unexpected ${this._getTypeDescriptor(importedType)} '${
568-
stringifyType(importedType)}' imported by the module '${
569-
stringifyType(moduleType)}'. Please add a @NgModule annotation.`),
570-
moduleType);
578+
const err = syntaxError(`Unexpected ${this._getTypeDescriptor(importedType)} '${
579+
stringifyType(importedType)}' imported by the module '${
580+
stringifyType(moduleType)}'. Please add a @NgModule annotation.`);
581+
// If possible, record additional context for this error to enable more useful
582+
// diagnostics on the compiler side.
583+
if (importedType instanceof StaticSymbol) {
584+
(err as any)[MISSING_NG_MODULE_METADATA_ERROR_DATA] = {
585+
fileName: importedType.filePath,
586+
className: importedType.name,
587+
} as MissingNgModuleMetadataErrorData;
588+
}
589+
this._reportError(err, moduleType);
571590
return;
572591
}
573592
importedModules.push(importedModuleSummary);

0 commit comments

Comments
 (0)