Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(compiler-cli): show a more specific error for Ivy NgModules #41534

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions goldens/public-api/compiler-cli/error_code.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export declare enum ErrorCode {
NGMODULE_MODULE_WITH_PROVIDERS_MISSING_GENERIC = 6005,
NGMODULE_REEXPORT_NAME_COLLISION = 6006,
NGMODULE_DECLARATION_NOT_UNIQUE = 6007,
NGMODULE_VE_DEPENDENCY_ON_IVY_LIB = 6999,
SCHEMA_INVALID_ELEMENT = 8001,
SCHEMA_INVALID_ATTRIBUTE = 8002,
MISSING_REFERENCE_TARGET = 8003,
Expand Down
6 changes: 6 additions & 0 deletions packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,12 @@ export enum ErrorCode {
*/
NGMODULE_DECLARATION_NOT_UNIQUE = 6007,

/**
* Not actually raised by the compiler, but reserved for documentation of a View Engine error when
* a View Engine build depends on an Ivy-compiled NgModule.
*/
NGMODULE_VE_DEPENDENCY_ON_IVY_LIB = 6999,

/**
* An element name failed validation against the DOM schema.
*/
Expand Down
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/NG6999 for more information.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a corresponding PR that adds this page?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not yet - I reached out to devrel about what content we want to add here.

`,
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