fix(compiler-cli): remove classes in .d.ts files from provider checks ()

This commit temporarily excludes classes declared in .d.ts files from checks
regarding whether providers are actually injectable.

Such classes used to be ignored (on accident) because the
`TypeScriptReflectionHost.getConstructorParameters()` method did not return
constructor parameters from d.ts files, mostly as an oversight. This was
recently fixed, but caused more providers to be exposed to this check, which
created a breakage in g3.

This commit temporarily fixes the breakage by continuing to exclude such
providers from the check, until g3 can be patched.

PR Close 
This commit is contained in:
Alex Rickabaugh 2020-12-14 14:10:18 -08:00
parent 2a7443117b
commit 973bb403a5
2 changed files with 21 additions and 11 deletions
packages/compiler-cli
src/ngtsc/annotations/src
test/ngtsc

@ -516,7 +516,13 @@ export function resolveProvidersRequiringFactory(
} }
} }
if (tokenClass !== null && reflector.isClass(tokenClass.node)) { // TODO(alxhub): there was a bug where `getConstructorParameters` would return `null` for a
// class in a .d.ts file, always, even if the class had a constructor. This was fixed for
// `getConstructorParameters`, but that fix causes more classes to be recognized here as needing
// provider checks, which is a breaking change in g3. Avoid this breakage for now by skipping
// classes from .d.ts files here directly, until g3 can be cleaned up.
if (tokenClass !== null && !tokenClass.node.getSourceFile().isDeclarationFile &&
reflector.isClass(tokenClass.node)) {
const constructorParameters = reflector.getConstructorParameters(tokenClass.node); const constructorParameters = reflector.getConstructorParameters(tokenClass.node);
// Note that we only want to capture providers with a non-trivial constructor, // Note that we only want to capture providers with a non-trivial constructor,

@ -7493,16 +7493,20 @@ export const Foo = Foo__PRE_R3__;
expect(diags.length).toBe(0); expect(diags.length).toBe(0);
}); });
it('should error when an undecorated class with a non-trivial constructor in a declaration file is provided via useClass', // TODO(alxhub): this test never worked correctly, as it used to declare a constructor with a
() => { // body, which real declaration files don't have. Without the body, the ReflectionHost used to
env.write('node_modules/@angular/core/testing/index.d.ts', ` // not return any constructor data, preventing an error from showing. That bug was fixed, but
// the error for declaration files is disabled until g3 can be updated.
xit('should error when an undecorated class with a non-trivial constructor in a declaration file is provided via useClass',
() => {
env.write('node_modules/@angular/core/testing/index.d.ts', `
export declare class NgZone {} export declare class NgZone {}
export declare class Testability { export declare class Testability {
constructor(ngZone: NgZone) {} constructor(ngZone: NgZone);
} }
`); `);
env.write('test.ts', ` env.write('test.ts', `
import {NgModule, Injectable} from '@angular/core'; import {NgModule, Injectable} from '@angular/core';
import {Testability} from '@angular/core/testing'; import {Testability} from '@angular/core/testing';
@ -7515,10 +7519,10 @@ export const Foo = Foo__PRE_R3__;
export class SomeModule {} export class SomeModule {}
`); `);
const diags = env.driveDiagnostics(); const diags = env.driveDiagnostics();
expect(diags.length).toBe(1); expect(diags.length).toBe(1);
expect(diags[0].messageText).toContain('cannot be created via dependency injection'); expect(diags[0].messageText).toContain('cannot be created via dependency injection');
}); });
it('should not error when an class with a factory definition and a non-trivial constructor in a declaration file is provided via useClass', it('should not error when an class with a factory definition and a non-trivial constructor in a declaration file is provided via useClass',
() => { () => {
@ -7529,7 +7533,7 @@ export const Foo = Foo__PRE_R3__;
export declare class Testability { export declare class Testability {
static ɵfac: i0.ɵɵFactoryDef<Testability, never>; static ɵfac: i0.ɵɵFactoryDef<Testability, never>;
constructor(ngZone: NgZone) {} constructor(ngZone: NgZone);
} }
`); `);
env.write('test.ts', ` env.write('test.ts', `