From 2d89b5d13d22cdb50c09217c54932c29484386b0 Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Fri, 21 Feb 2020 14:00:44 -0800 Subject: [PATCH] fix(ivy): provide a more detailed error message for NG6002/NG6003 (#35620) NG6002/NG6003 are errors produced when an NgModule being compiled has an imported or exported type which does not have the proper metadata (that is, it doesn't appear to be an @NgModule, or @Directive, etc. depending on context). Previously this error message was a bit sparse. However, Github issues show that this is the most common error users receive when for whatever reason ngcc wasn't able to handle one of their libraries, or they just didn't run it. So this commit changes the error message to offer a bit more useful context, instructing users differently depending on whether the class in question is from their own project, from NPM, or from a monorepo-style local dependency. PR Close #35620 --- .../compiler-cli/src/ngtsc/scope/src/local.ts | 29 +++++++- .../compiler-cli/test/ngtsc/ngtsc_spec.ts | 70 +++++++++++++++++++ 2 files changed, 96 insertions(+), 3 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/scope/src/local.ts b/packages/compiler-cli/src/ngtsc/scope/src/local.ts index 95a4728fbd..5d04d2f145 100644 --- a/packages/compiler-cli/src/ngtsc/scope/src/local.ts +++ b/packages/compiler-cli/src/ngtsc/scope/src/local.ts @@ -551,9 +551,32 @@ function invalidRef( const code = type === 'import' ? ErrorCode.NGMODULE_INVALID_IMPORT : ErrorCode.NGMODULE_INVALID_EXPORT; const resolveTarget = type === 'import' ? 'NgModule' : 'NgModule, Component, Directive, or Pipe'; - return makeDiagnostic( - code, identifierOfNode(decl.node) || decl.node, - `Appears in the NgModule.${type}s of ${nodeNameForError(clazz)}, but could not be resolved to an ${resolveTarget} class`); + let message = + `Appears in the NgModule.${type}s of ${nodeNameForError(clazz)}, but could not be resolved to an ${resolveTarget} class.` + + '\n\n'; + const library = decl.ownedByModuleGuess !== null ? ` (${decl.ownedByModuleGuess})` : ''; + const sf = decl.node.getSourceFile(); + + // Provide extra context to the error for the user. + if (!sf.isDeclarationFile) { + // This is a file in the user's program. + const annotationType = type === 'import' ? '@NgModule' : 'Angular'; + message += `Is it missing an ${annotationType} annotation?`; + } else if (sf.fileName.indexOf('node_modules') !== -1) { + // This file comes from a third-party library in node_modules. + message += + `This likely means that the library${library} which declares ${decl.debugName} has not ` + + 'been processed correctly by ngcc, or is not compatible with Angular Ivy. Check if a ' + + 'newer version of the library is available, and update if so. Also consider checking ' + + 'with the library\'s authors to see if the library is expected to be compatible with Ivy.'; + } else { + // This is a monorepo style local dependency. Unfortunately these are too different to really + // offer much more advice than this. + message += + `This likely means that the dependency${library} which declares ${decl.debugName} has not been processed correctly by ngcc.`; + } + + return makeDiagnostic(code, identifierOfNode(decl.node) || decl.node, message); } /** diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index e838b3e12f..695c4cc2b9 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -3769,6 +3769,76 @@ runInEachFileSystem(os => { // Success is enough to indicate that this passes. }); + describe('NgModule invalid import/export errors', () => { + function verifyThrownError(errorCode: ErrorCode, errorMessage: string) { + const errors = env.driveDiagnostics(); + expect(errors.length).toBe(1); + const {code, messageText} = errors[0]; + expect(code).toBe(ngErrorCode(errorCode)); + expect(trim(messageText as string)).toContain(errorMessage); + } + + it('should provide a hint when importing an invalid NgModule from node_modules', () => { + env.write('node_modules/external/index.d.ts', ` + export declare class NotAModule {} + `); + env.write('test.ts', ` + import {NgModule} from '@angular/core'; + import {NotAModule} from 'external'; + + @NgModule({ + imports: [NotAModule], + }) + export class Module {} + `); + + verifyThrownError( + ErrorCode.NGMODULE_INVALID_IMPORT, + 'This likely means that the library (external) which declares NotAModule has not ' + + 'been processed correctly by ngcc, or is not compatible with Angular Ivy.'); + }); + + it('should provide a hint when importing an invalid NgModule from a local library', () => { + env.write('libs/external/index.d.ts', ` + export declare class NotAModule {} + `); + + env.write('test.ts', ` + import {NgModule} from '@angular/core'; + import {NotAModule} from './libs/external'; + + @NgModule({ + imports: [NotAModule], + }) + export class Module {} + `); + + verifyThrownError( + ErrorCode.NGMODULE_INVALID_IMPORT, + 'This likely means that the dependency which declares NotAModule has not ' + + 'been processed correctly by ngcc.'); + }); + + it('should provide a hint when importing an invalid NgModule in the current program', () => { + env.write('invalid.ts', ` + export class NotAModule {} + `); + + env.write('test.ts', ` + import {NgModule} from '@angular/core'; + import {NotAModule} from './invalid'; + + @NgModule({ + imports: [NotAModule], + }) + export class Module {} + `); + + verifyThrownError( + ErrorCode.NGMODULE_INVALID_IMPORT, 'Is it missing an @NgModule annotation?'); + }); + }); + describe('when processing external directives', () => { it('should not emit multiple references to the same directive', () => { env.write('node_modules/external/index.d.ts', `