From 21119ea3300dc5d3f6bf8d122fb76549bad483c3 Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Fri, 9 Apr 2021 11:31:08 -0400 Subject: [PATCH] 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 --- .../public-api/compiler-cli/error_code.d.ts | 1 + .../src/ngtsc/diagnostics/src/error_code.ts | 6 ++ .../compiler-cli/src/transformers/program.ts | 68 ++++++++++++++++++- packages/compiler-cli/test/ngc_spec.ts | 28 ++++++++ packages/compiler/src/metadata_resolver.ts | 29 ++++++-- 5 files changed, 124 insertions(+), 8 deletions(-) diff --git a/goldens/public-api/compiler-cli/error_code.d.ts b/goldens/public-api/compiler-cli/error_code.d.ts index b9ed59e4694c..ffccf549c1c2 100644 --- a/goldens/public-api/compiler-cli/error_code.d.ts +++ b/goldens/public-api/compiler-cli/error_code.d.ts @@ -27,6 +27,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, diff --git a/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts b/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts index b1e767dcf25e..d0d9fc5d736b 100644 --- a/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts +++ b/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts @@ -99,6 +99,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. */ diff --git a/packages/compiler-cli/src/transformers/program.ts b/packages/compiler-cli/src/transformers/program.ts index d8af2c9f6216..be760254ed56 100644 --- a/packages/compiler-cli/src/transformers/program.ts +++ b/packages/compiler-cli/src/transformers/program.ts @@ -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'; @@ -676,7 +676,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(), @@ -1022,7 +1022,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(e => ({ @@ -1044,6 +1044,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. +`, + 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, @@ -1052,3 +1079,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; +} \ No newline at end of file diff --git a/packages/compiler-cli/test/ngc_spec.ts b/packages/compiler-cli/test/ngc_spec.ts index 0e35c3ee8ca9..7128f6fdbc64 100644 --- a/packages/compiler-cli/test/ngc_spec.ts +++ b/packages/compiler-cli/test/ngc_spec.ts @@ -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(`{ diff --git a/packages/compiler/src/metadata_resolver.ts b/packages/compiler/src/metadata_resolver.ts index 17a247b4bdd2..26016a17ee69 100644 --- a/packages/compiler/src/metadata_resolver.ts +++ b/packages/compiler/src/metadata_resolver.ts @@ -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, @@ -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);