Skip to content

Commit

Permalink
fix(compiler-cli): show a more specific error for Ivy NgModules
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
alxhub committed Apr 9, 2021
1 parent 10a7c87 commit 7c45335
Show file tree
Hide file tree
Showing 3 changed files with 117 additions and 8 deletions.
68 changes: 65 additions & 3 deletions packages/compiler-cli/src/transformers/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
* found in the LICENSE file at https://angular.io/license
*/

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

function syntaxErrorToDiagnostics(error: Error): Diagnostic[] {
function syntaxErrorToDiagnostics(error: Error, program: ts.Program): Diagnostic[] {
const parserErrors = getParseErrors(error);
if (parserErrors && parserErrors.length) {
return parserErrors.map<Diagnostic>(e => ({
Expand All @@ -1058,6 +1058,33 @@ function syntaxErrorToDiagnostics(error: Error): Diagnostic[] {
position: error.position
}];
}

const ngModuleErrorData = getMissingNgModuleMetadataErrorData(error);
if (ngModuleErrorData !== null) {
// This error represents the import or export of an `NgModule` that didn't have valid metadata.
// This _might_ happen because the NgModule in question is an Ivy-compiled library, and we want
// to show a more useful error if that's the case.
const ngModuleClass =
getDtsClass(program, ngModuleErrorData.fileName, ngModuleErrorData.className);
if (ngModuleClass !== null && isIvyNgModule(ngModuleClass)) {
return [{
messageText: `The NgModule '${ngModuleErrorData.className}' in '${
ngModuleErrorData
.fileName}' is imported by this compilation, but appears to be part of a library compiled for Angular Ivy. This may occur because:
1) the library was processed with 'ngcc'. Removing and reinstalling node_modules may fix this problem.
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.
See https://angular.io/errors/NG1234 for more information.
`,
category: ts.DiagnosticCategory.Error,
code: DEFAULT_ERROR_CODE,
source: SOURCE,
}];
}
}

// Produce a Diagnostic anyway since we know for sure `error` is a SyntaxError
return [{
messageText: error.message,
Expand All @@ -1066,3 +1093,38 @@ function syntaxErrorToDiagnostics(error: Error): Diagnostic[] {
source: SOURCE,
}];
}

function getDtsClass(program: ts.Program, fileName: string, className: string): ts.ClassDeclaration|
null {
const sf = program.getSourceFile(fileName);
if (sf === undefined || !sf.isDeclarationFile) {
return null;
}
for (const stmt of sf.statements) {
if (!ts.isClassDeclaration(stmt)) {
continue;
}
if (stmt.name === undefined || stmt.name.text !== className) {
continue;
}

return stmt;
}

// No classes found that matched the given name.
return null;
}

function isIvyNgModule(clazz: ts.ClassDeclaration): boolean {
for (const member of clazz.members) {
if (!ts.isPropertyDeclaration(member)) {
continue;
}
if (ts.isIdentifier(member.name) && member.name.text === 'ɵmod') {
return true;
}
}

// No Ivy 'ɵmod' property found.
return false;
}
28 changes: 28 additions & 0 deletions packages/compiler-cli/test/ngc_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,34 @@ describe('ngc transformer command-line', () => {
});
});

it('should give a specific error when an Angular Ivy NgModule is imported', () => {
writeConfig(`{
"extends": "./tsconfig-base.json",
"files": ["mymodule.ts"]
}`);
write('node_modules/test/index.d.ts', `
export declare class FooModule {
static ɵmod = null;
}
`);
write('mymodule.ts', `
import {NgModule} from '@angular/core';
import {FooModule} from 'test';
@NgModule({
imports: [FooModule],
})
export class TestModule {}
`);

const exitCode = main(['-p', basePath], errorSpy);
expect(errorSpy).toHaveBeenCalledTimes(1);
const message = errorSpy.calls.mostRecent().args[0];

// The error message should mention Ivy specifically.
expect(message).toContain('Angular Ivy');
});

describe('compile ngfactory files', () => {
it('should compile ngfactory files that are not referenced by root files', () => {
writeConfig(`{
Expand Down
29 changes: 24 additions & 5 deletions packages/compiler/src/metadata_resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,18 @@ export type ErrorCollector = (error: any, type?: any) => void;

export const ERROR_COMPONENT_TYPE = 'ngComponentType';

const MISSING_NG_MODULE_METADATA_ERROR_DATA = 'ngMissingNgModuleMetadataErrorData';
export interface MissingNgModuleMetadataErrorData {
fileName: string;
className: string;
}


export function getMissingNgModuleMetadataErrorData(error: any): MissingNgModuleMetadataErrorData|
null {
return error[MISSING_NG_MODULE_METADATA_ERROR_DATA] ?? null;
}

// Design notes:
// - don't lazily create metadata:
// For some metadata, we need to do async work sometimes,
Expand Down Expand Up @@ -563,11 +575,18 @@ export class CompileMetadataResolver {
this.getNgModuleSummary(importedModuleType, alreadyCollecting);
alreadyCollecting.delete(importedModuleType);
if (!importedModuleSummary) {
this._reportError(
syntaxError(`Unexpected ${this._getTypeDescriptor(importedType)} '${
stringifyType(importedType)}' imported by the module '${
stringifyType(moduleType)}'. Please add a @NgModule annotation.`),
moduleType);
const err = syntaxError(`Unexpected ${this._getTypeDescriptor(importedType)} '${
stringifyType(importedType)}' imported by the module '${
stringifyType(moduleType)}'. Please add a @NgModule annotation.`);
// If possible, record additional context for this error to enable more useful
// diagnostics on the compiler side.
if (importedType instanceof StaticSymbol) {
(err as any)[MISSING_NG_MODULE_METADATA_ERROR_DATA] = {
fileName: importedType.filePath,
className: importedType.name,
} as MissingNgModuleMetadataErrorData;
}
this._reportError(err, moduleType);
return;
}
importedModules.push(importedModuleSummary);
Expand Down

0 comments on commit 7c45335

Please sign in to comment.