Skip to content

Type checking differs when depending on source VS compiled d.ts file for classes with generic type parameters used in private fields #36906

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

Closed
CraigMacomber opened this issue Feb 20, 2020 · 5 comments
Labels
Duplicate An existing issue was already created

Comments

@CraigMacomber
Copy link

CraigMacomber commented Feb 20, 2020

TypeScript Version: 3.7.5 (Also confirmed in 3.5.3)

Search Terms:
unused generics

Expected behavior:
Compiling against a d.ts file produces the same type checking as checking as compiling against the original source the d.ts was built from (ex: cross package or cross project dependencies type check the same as cross module or within the file).

One way to interpret this is that for nominal typing (classes with private fields), their generic type parameters should be included in type checking, even if unused (that would fix this, but also impact other cases, though this would be an improvement for those cases as well in my opinion).

Also I expect 'no overlap' type errors to not be given between an unconstrained generic type parameter and a concreate type.

Actual behavior:
The compiled d.ts files omit the generic types from the private fields resulting in the generic type parameters being unused allowing code that should not type check to compile without error, but only when using the d.ts file instead of the source directly.

Specifically in my case, my codebase gives no type checking errors when built with projects, but gives type errors when built as a single project. I think this is a "mismatch between the checker visibility rules and the emitter output rules" (as described in #1445)

Some of the errors in my code are 'no overlap' type errors to when comparing an unconstrained generic type parameter and a concreate type: these should not be errors in the first place (I'm not sure if this is related and/or should get a separate bug), but they also shouldn't differ between the two build configurations. There were also more legitimate compile errors that were only found when doing the non-project based build that should have been detected in both builds.

As a workaround I can add dummy public fields to use the generic parameters in addition to the dummy private ones I use to ensure nominal typing. Then I can add type assertions to suppress the no overlap errors.

Related Issues:
#7797 : this issue is similar, however it is in a cause using structural typing, so the explanation for why it acts as it does does not apply to my case which uses nominal typing due to private fields. Also that issue this not really reach a conclusion.

#33877 : the example is similar, but I think it is a distinct issue.

Code

// An example generic class that uses nominal typing
export class MakeNominalSource<A> {
	private _useA!: A;
}

// This is what gets generated in the d.ts file.
// I'm assuming depending on the d.ts OR source should not impact type checking, but it does: (thus a bug)
// Using this version results in different type checking that depending on the source directly
// since the generic type paramater is unused.
// It seems that either the type checking related to unused generic type paramaters is wrong,
// and/or the generation of the d.ts file is incorrect.
export declare class MakeNominalCompiled<A> {
    private _useA;
}

function useSource(): void {
	const a: MakeNominalSource<number> = {} as any;
	const b: MakeNominalSource<string> = a; // Does not build (and should not)
}

function useCompiled(): void {
	const a: MakeNominalCompiled<number> = {} as any;
	const b: MakeNominalCompiled<string> = a; // Builds (but should not)
}

function fCompiled<T>(): boolean {
	const a: MakeNominalCompiled<T> = {} as any;
	const b: MakeNominalCompiled<number> = {} as any;
	return a === b;
}

function fSource<T>(): boolean {
	const a: MakeNominalSource<T> = {} as any;
    const b: MakeNominalSource<number> = {} as any;

    // Error: "This condition will always return 'false' since the types 'MakeNominalSource<T>' and 'MakeNominalSource<number>' have no overlap."
	return a === b; // Does not build, but should
}

// Example generic class which uses its type paramater, but not in the API surface.
// This typechecks the same as MakeNominalCompiled: to get the desired type checking the generic type paramater
// aparently must appear in one of the member types: being used within a method is not enough.
export class MakeNominalSource2<A> {
    private f(): void {
        const a: A = {} as any;
    }
}

function useSource2(): void {
	const a: MakeNominalSource2<number> = {} as any;
	const b: MakeNominalSource2<string> = a; // Builds (but should not)
}

// Thus is appears that if you want to mark a class as having its generics matter for type checking, use nominal typing,
// and have it work when consumed from other packages or projects via the d.ts files,
// you need to add a dummy private member (for nominal typing), and a dummy public member that uses the generic types.
// To avoid breaking compatability and remove the need for public fields here,
// maybe we could get a compiler flag to actually include unused generic parmaters in type checking?

// Also there is aparently a typechecker bug that erros saying no overlap on places that clearly have overlap:
// Since fixing this will strictly accept more programs, it should not be a breaking change.
Output
// An example generic class that uses nominal typing
export class MakeNominalSource {
}
function useSource() {
    const a = {};
    const b = a; // Does not build (and should not)
}
function useCompiled() {
    const a = {};
    const b = a; // Builds (but should not)
}
function fCompiled() {
    const a = {};
    const b = {};
    return a === b;
}
function fSource() {
    const a = {};
    const b = {};
    // Error: "This condition will always return 'false' since the types 'MakeNominalSource<T>' and 'MakeNominalSource<number>' have no overlap."
    return a === b; // Does not build, but should
}
// Example generic class which uses its type paramater, but not in the API surface.
// This typechecks the same as MakeNominalCompiled: to get the desired type checking the generic type paramater
// aparently must appear in one of the member types: being used within a method is not enough.
export class MakeNominalSource2 {
    f() {
        const a = {};
    }
}
function useSource2() {
    const a = {};
    const b = a; // Builds (but should not)
}
// Thus is appears that if you want to mark a class as having its generics matter for type checking, use nominal typing,
// and have it work when consumed from other packages or projects via the d.ts files,
// you need to add a dummy private member (for nominal typing), and a dummy public member that uses the generic types.
// To avoid breaking compatability and remove the need for public fields here,
// maybe we could get a compiler flag to actually include unused generic parmaters in type checking?
// Also there is aparently a typechecker bug that erros saying no overlap on places that clearly have overlap:
// Since fixing this will strictly accept more programs, it should not be a breaking change.
Compiler Options
{
  "compilerOptions": {
    "noImplicitAny": true,
    "strictNullChecks": true,
    "strictFunctionTypes": true,
    "strictPropertyInitialization": true,
    "strictBindCallApply": true,
    "noImplicitThis": true,
    "noImplicitReturns": true,
    "useDefineForClassFields": false,
    "alwaysStrict": true,
    "allowUnreachableCode": false,
    "allowUnusedLabels": false,
    "downlevelIteration": false,
    "noEmitHelpers": false,
    "noLib": false,
    "noStrictGenericChecks": false,
    "noUnusedLocals": false,
    "noUnusedParameters": false,
    "esModuleInterop": true,
    "preserveConstEnums": false,
    "removeComments": false,
    "skipLibCheck": false,
    "checkJs": false,
    "allowJs": false,
    "declaration": true,
    "experimentalDecorators": false,
    "emitDecoratorMetadata": false,
    "target": "ES2017",
    "module": "ESNext"
  }
}

Playground Link: Provided

@tadhgmister
Copy link

tadhgmister commented Feb 21, 2020

Does your actual use case use the generic in any non private fields? If the answer is yes I'm pretty sure the type checking would be consistent, and if the answer is no then I'd ask what that generic actually represents if it must be specified externally but has no impact on the external API of the class.

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label Feb 27, 2020
@RyanCavanaugh
Copy link
Member

Duplicate #20979

@CraigMacomber
Copy link
Author

@tadhgmister One of my use cases (we have a couple in our codebase) looks a lot like a dependency injection system, and I'm associating types with the keys I use to look up registrations via a generic type parameter. This allows type safety on both the registration and on the lookup side. These key types don't actually use the generic parameter for anything other than this type checking though so I added the unused private field for the only purpose of making type checking do what I want (get nominal typing and including the generic types in the type checking).

@CraigMacomber
Copy link
Author

Duplicate #20979

The discussion there notes I can make my field protected instead of public. That's a good point and makes the workaround cleaner.

@typescript-bot
Copy link
Collaborator

This issue has been marked as a 'Duplicate' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

4 participants