-
Notifications
You must be signed in to change notification settings - Fork 13.2k
Description
Suggestion
Fix generated d.ts files having incorrect variance (when compared to original source: #20979 ) by emitting the inferred variance explicitly in d.ts files using the 4.7 variance annotations feature.
As this would cause the minimum supported version of typescript to use those generated files to be 4.7, it might be a good idea to put this behind a flag (and maybe default it on at a later point).
Another option would be to add a compiler warning in this case (maybe an error in strict mode), and suggest manually adding the variance annotation. This mitigates the minimum supported typescript version by pushing it off to the library author to choose.
I think this approach would be a fully non-breaking change (since I think we allow added checks in strict mode).
🔍 Search Terms
variance annotations, d.ts, unsound, generics, private
✅ Viability Checklist
My suggestion meets these guidelines:
- This wouldn't be a breaking change in existing TypeScript/JavaScript code
- This wouldn't change the runtime behavior of existing JavaScript code
- This could be implemented without emitting different JS based on the types of the expressions
- This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
- This feature would agree with the rest of TypeScript's Design Goals.
⭐ Suggestion
Optionally include variance annotations in generated d.ts files when the omission of it results in incorrect variance due to omitting private field types.
It might also make sense to include them for all generic types to serve as documentation of what the variance is (easier for the user of the type to read that then infer it themself).
This could also help detect against changes in variance inference in the future version of typescript (this may be good, or bad).
📃 Motivating Example
A dependency injection / registry system that attempts to use generics to pair its behaviors with their implementations could easily end up with an unsafe API when exposed to external users, but appear safe to the authors which test it directly.
export class Regestration<Tag> {
private readonly typeBrand!: Tag;
public constructor(public readonly value: number) {}
}
// Generates:
// export declare class Regestration<Tag> {
// readonly value: number;
// private readonly typeBrand;
// constructor(value: number);
// }
export class Foo{
protected makeThisANominalType!: unknown;
}
export let fooRegistry: Regestration<Foo>;
const foo1 = new Regestration<Foo>(1);
const foo2 = new Regestration<Foo>(2);
const other = new Regestration<string>(100);
fooRegistry = foo1; // Good
fooRegistry = other; // Should not build, but currently builds if using generated d.ts filesIf it generated:
export declare class Regestration<out Tag> {
readonly value: number;
private readonly typeBrand;
constructor(value: number);
}Then developers unaware of this specific problem would not accidently encounter it.
💻 Use Cases
For developers aware of this specific issue, it won't make a lot of difference (there are easy workarounds),
but a lot of TypeScript developers can fall into this trap, and never know it since it just impacts their users (and only when they code a bug which shouldn't build). Having the compiler handle this automatically not only protected these users, but gives people who know about this one less thing to worry about.
Current work around: make field protected or manually include variance annotation.