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(ivy): throw better error for missing generic type in ModuleWithProviders #33187

Closed
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
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