Skip to content

Commit

Permalink
fix(ivy): throw better error for missing generic type in ModuleWithPr…
Browse files Browse the repository at this point in the history
…oviders (angular#33187)

Currently if a `ModuleWithProviders` is missng its generic type, we throw a cryptic error like:

```
error TS-991010: Value at position 3 in the NgModule.imports of TodosModule is not a reference: [object Object]
```

These changes add a better error to make it easier to debug.

PR Close angular#33187
  • Loading branch information
crisbeto authored and ODAVING committed Oct 18, 2019
1 parent 7ffda3c commit 97245b6
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 5 deletions.
16 changes: 13 additions & 3 deletions packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts
Expand Up @@ -358,7 +358,7 @@ export class NgModuleDecoratorHandler implements DecoratorHandler<NgModuleAnalys
ts.FunctionExpression): ts.Expression|null {
const type = node.type || null;
return type &&
(this._reflectModuleFromTypeParam(type) || this._reflectModuleFromLiteralType(type));
(this._reflectModuleFromTypeParam(type, node) || this._reflectModuleFromLiteralType(type));
}

/**
Expand All @@ -367,7 +367,9 @@ export class NgModuleDecoratorHandler implements DecoratorHandler<NgModuleAnalys
* @param type The type to reflect on.
* @returns the identifier of the NgModule type if found, or null otherwise.
*/
private _reflectModuleFromTypeParam(type: ts.TypeNode): ts.Expression|null {
private _reflectModuleFromTypeParam(
type: ts.TypeNode,
node: ts.FunctionDeclaration|ts.MethodDeclaration|ts.FunctionExpression): ts.Expression|null {
// Examine the type of the function to see if it's a ModuleWithProviders reference.
if (!ts.isTypeReferenceNode(type)) {
return null;
Expand Down Expand Up @@ -395,7 +397,15 @@ export class NgModuleDecoratorHandler implements DecoratorHandler<NgModuleAnalys

// If there's no type parameter specified, bail.
if (type.typeArguments === undefined || type.typeArguments.length !== 1) {
return null;
const parent =
ts.isMethodDeclaration(node) && ts.isClassDeclaration(node.parent) ? node.parent : null;
const symbolName = (parent && parent.name ? parent.name.getText() + '.' : '') +
(node.name ? node.name.getText() : 'anonymous');
throw new FatalDiagnosticError(
ErrorCode.NGMODULE_MODULE_WITH_PROVIDERS_MISSING_GENERIC, type,
`${symbolName} returns a ModuleWithProviders type without a generic type argument. ` +
`Please add a generic type argument to the ModuleWithProviders type. If this ` +
`occurrence is in library code you don't control, please contact the library authors.`);
}

const arg = type.typeArguments[0];
Expand Down
8 changes: 7 additions & 1 deletion packages/compiler-cli/src/ngtsc/diagnostics/src/code.ts
Expand Up @@ -58,6 +58,12 @@ export enum ErrorCode {
*/
NGMODULE_INVALID_REEXPORT = 6004,

/**
* Raised when a `ModuleWithProviders` with a missing
* generic type argument is passed into an `NgModule`.
*/
NGMODULE_MODULE_WITH_PROVIDERS_MISSING_GENERIC = 6005,

/**
* Raised when ngcc tries to inject a synthetic decorator over one that already exists.
*/
Expand Down Expand Up @@ -87,4 +93,4 @@ export enum ErrorCode {

export function ngErrorCode(code: ErrorCode): number {
return parseInt('-99' + code);
}
}
3 changes: 2 additions & 1 deletion packages/compiler-cli/test/ngtsc/fake_core/index.ts
Expand Up @@ -44,7 +44,8 @@ export const Output = callablePropDecorator();
export const ViewChild = callablePropDecorator();
export const ViewChildren = callablePropDecorator();

export type ModuleWithProviders<T> = any;
// T defaults to `any` to reflect what is currently in core.
export type ModuleWithProviders<T = any> = any;

export class ChangeDetectorRef {}
export class ElementRef {}
Expand Down
25 changes: 25 additions & 0 deletions packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
Expand Up @@ -1469,6 +1469,31 @@ runInEachFileSystem(os => {
'i0.ɵɵNgModuleDefWithMeta<TestModule, never, [typeof i1.RouterModule], never>');
});

it('should throw if ModuleWithProviders is missing its generic type argument', () => {
env.write(`test.ts`, `
import {NgModule} from '@angular/core';
import {RouterModule} from 'router';
@NgModule({imports: [RouterModule.forRoot()]})
export class TestModule {}
`);

env.write('node_modules/router/index.d.ts', `
import {ModuleWithProviders, ɵɵNgModuleDefWithMeta} from '@angular/core';
declare class RouterModule {
static forRoot(): ModuleWithProviders;
static ɵmod: ɵɵNgModuleDefWithMeta<RouterModule, never, never, never>;
}
`);
const errors = env.driveDiagnostics();
expect(trim(errors[0].messageText as string))
.toContain(
`RouterModule.forRoot returns a ModuleWithProviders type without a generic type argument. ` +
`Please add a generic type argument to the ModuleWithProviders type. If this ` +
`occurrence is in library code you don't control, please contact the library authors.`);
});

it('should extract the generic type if it is provided as qualified type name', () => {
env.write(`test.ts`, `
import {NgModule} from '@angular/core';
Expand Down

0 comments on commit 97245b6

Please sign in to comment.