Skip to content

Commit 09eb125

Browse files
alxhubzarend
authored andcommitted
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 9cda866 commit 09eb125

File tree

5 files changed

+124
-8
lines changed

5 files changed

+124
-8
lines changed

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';
@@ -693,7 +693,7 @@ class AngularCompilerProgram implements Program {
693693
private _addStructuralDiagnostics(error: Error) {
694694
const diagnostics = this._structuralDiagnostics || (this._structuralDiagnostics = []);
695695
if (isSyntaxError(error)) {
696-
diagnostics.push(...syntaxErrorToDiagnostics(error));
696+
diagnostics.push(...syntaxErrorToDiagnostics(error, this.tsProgram));
697697
} else {
698698
diagnostics.push({
699699
messageText: error.toString(),
@@ -1039,7 +1039,7 @@ function diagnosticChainFromFormattedDiagnosticChain(chain: FormattedMessageChai
10391039
};
10401040
}
10411041

1042-
function syntaxErrorToDiagnostics(error: Error): Diagnostic[] {
1042+
function syntaxErrorToDiagnostics(error: Error, program: ts.Program): Diagnostic[] {
10431043
const parserErrors = getParseErrors(error);
10441044
if (parserErrors && parserErrors.length) {
10451045
return parserErrors.map<Diagnostic>(e => ({
@@ -1061,6 +1061,33 @@ function syntaxErrorToDiagnostics(error: Error): Diagnostic[] {
10611061
position: error.position
10621062
}];
10631063
}
1064+
1065+
const ngModuleErrorData = getMissingNgModuleMetadataErrorData(error);
1066+
if (ngModuleErrorData !== null) {
1067+
// This error represents the import or export of an `NgModule` that didn't have valid metadata.
1068+
// This _might_ happen because the NgModule in question is an Ivy-compiled library, and we want
1069+
// to show a more useful error if that's the case.
1070+
const ngModuleClass =
1071+
getDtsClass(program, ngModuleErrorData.fileName, ngModuleErrorData.className);
1072+
if (ngModuleClass !== null && isIvyNgModule(ngModuleClass)) {
1073+
return [{
1074+
messageText: `The NgModule '${ngModuleErrorData.className}' in '${
1075+
ngModuleErrorData
1076+
.fileName}' is imported by this compilation, but appears to be part of a library compiled for Angular Ivy. This may occur because:
1077+
1078+
1) the library was processed with 'ngcc'. Removing and reinstalling node_modules may fix this problem.
1079+
1080+
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.
1081+
1082+
See https://angular.io/errors/NG6999 for more information.
1083+
`,
1084+
category: ts.DiagnosticCategory.Error,
1085+
code: DEFAULT_ERROR_CODE,
1086+
source: SOURCE,
1087+
}];
1088+
}
1089+
}
1090+
10641091
// Produce a Diagnostic anyway since we know for sure `error` is a SyntaxError
10651092
return [{
10661093
messageText: error.message,
@@ -1069,3 +1096,38 @@ function syntaxErrorToDiagnostics(error: Error): Diagnostic[] {
10691096
source: SOURCE,
10701097
}];
10711098
}
1099+
1100+
function getDtsClass(program: ts.Program, fileName: string, className: string): ts.ClassDeclaration|
1101+
null {
1102+
const sf = program.getSourceFile(fileName);
1103+
if (sf === undefined || !sf.isDeclarationFile) {
1104+
return null;
1105+
}
1106+
for (const stmt of sf.statements) {
1107+
if (!ts.isClassDeclaration(stmt)) {
1108+
continue;
1109+
}
1110+
if (stmt.name === undefined || stmt.name.text !== className) {
1111+
continue;
1112+
}
1113+
1114+
return stmt;
1115+
}
1116+
1117+
// No classes found that matched the given name.
1118+
return null;
1119+
}
1120+
1121+
function isIvyNgModule(clazz: ts.ClassDeclaration): boolean {
1122+
for (const member of clazz.members) {
1123+
if (!ts.isPropertyDeclaration(member)) {
1124+
continue;
1125+
}
1126+
if (ts.isIdentifier(member.name) && member.name.text === 'ɵmod') {
1127+
return true;
1128+
}
1129+
}
1130+
1131+
// No Ivy 'ɵmod' property found.
1132+
return false;
1133+
}

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)