Skip to content

Commit 973bb40

Browse files
committed
fix(compiler-cli): remove classes in .d.ts files from provider checks (#40118)
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 #40118
1 parent 2a74431 commit 973bb40

File tree

2 files changed

+21
-11
lines changed

2 files changed

+21
-11
lines changed

packages/compiler-cli/src/ngtsc/annotations/src/util.ts

+7-1
Original file line numberDiff line numberDiff line change
@@ -516,7 +516,13 @@ export function resolveProvidersRequiringFactory(
516516
}
517517
}
518518

519-
if (tokenClass !== null && reflector.isClass(tokenClass.node)) {
519+
// TODO(alxhub): there was a bug where `getConstructorParameters` would return `null` for a
520+
// class in a .d.ts file, always, even if the class had a constructor. This was fixed for
521+
// `getConstructorParameters`, but that fix causes more classes to be recognized here as needing
522+
// provider checks, which is a breaking change in g3. Avoid this breakage for now by skipping
523+
// classes from .d.ts files here directly, until g3 can be cleaned up.
524+
if (tokenClass !== null && !tokenClass.node.getSourceFile().isDeclarationFile &&
525+
reflector.isClass(tokenClass.node)) {
520526
const constructorParameters = reflector.getConstructorParameters(tokenClass.node);
521527

522528
// Note that we only want to capture providers with a non-trivial constructor,

packages/compiler-cli/test/ngtsc/ngtsc_spec.ts

+14-10
Original file line numberDiff line numberDiff line change
@@ -7493,16 +7493,20 @@ export const Foo = Foo__PRE_R3__;
74937493
expect(diags.length).toBe(0);
74947494
});
74957495

7496-
it('should error when an undecorated class with a non-trivial constructor in a declaration file is provided via useClass',
7497-
() => {
7498-
env.write('node_modules/@angular/core/testing/index.d.ts', `
7496+
// TODO(alxhub): this test never worked correctly, as it used to declare a constructor with a
7497+
// body, which real declaration files don't have. Without the body, the ReflectionHost used to
7498+
// not return any constructor data, preventing an error from showing. That bug was fixed, but
7499+
// the error for declaration files is disabled until g3 can be updated.
7500+
xit('should error when an undecorated class with a non-trivial constructor in a declaration file is provided via useClass',
7501+
() => {
7502+
env.write('node_modules/@angular/core/testing/index.d.ts', `
74997503
export declare class NgZone {}
75007504
75017505
export declare class Testability {
7502-
constructor(ngZone: NgZone) {}
7506+
constructor(ngZone: NgZone);
75037507
}
75047508
`);
7505-
env.write('test.ts', `
7509+
env.write('test.ts', `
75067510
import {NgModule, Injectable} from '@angular/core';
75077511
import {Testability} from '@angular/core/testing';
75087512
@@ -7515,10 +7519,10 @@ export const Foo = Foo__PRE_R3__;
75157519
export class SomeModule {}
75167520
`);
75177521

7518-
const diags = env.driveDiagnostics();
7519-
expect(diags.length).toBe(1);
7520-
expect(diags[0].messageText).toContain('cannot be created via dependency injection');
7521-
});
7522+
const diags = env.driveDiagnostics();
7523+
expect(diags.length).toBe(1);
7524+
expect(diags[0].messageText).toContain('cannot be created via dependency injection');
7525+
});
75227526

75237527
it('should not error when an class with a factory definition and a non-trivial constructor in a declaration file is provided via useClass',
75247528
() => {
@@ -7529,7 +7533,7 @@ export const Foo = Foo__PRE_R3__;
75297533
75307534
export declare class Testability {
75317535
static ɵfac: i0.ɵɵFactoryDef<Testability, never>;
7532-
constructor(ngZone: NgZone) {}
7536+
constructor(ngZone: NgZone);
75337537
}
75347538
`);
75357539
env.write('test.ts', `

0 commit comments

Comments
 (0)