You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Option strictMethodType: strictFunctionType for method syntax.Β #57783
The TS community is now more used to the strict subset of TypeScript. And this is now the standard and recommended way to use TypeScript. StrictFunctionType brought a lot of security, but it didn't apply to methods because it broke too much code at the time.
The difference between function properties and methods is not clear to many users. The bivariance of methods can even be seen as a bug from a normal user's point of view. It is also more convenient to use the method syntax, because it is shorter and seems more idiomatic. For these reasons, users don't get the security of `strictFunctionType' when writing interfaces, classes or object types.
I think it is time to introduce a new option strictMethodType, which applies strict variance to methods.
This could also be the opportunity to make method readonly by default under this new option.
To ensure backwards compatibility, the option could be disabled by default for the time being.
π Motivating Example
interfaceComparer<T>{compare(a: T,b: T): number;}declareletanimalComparer: Comparer<Animal>;declareletdogComparer: Comparer<Dog>;animalComparer=dogComparer;// Error under `strictMethodSyntax`dogComparer=animalComparer;// Ok
π» Use Cases
I want to get the safety benefit of strictFunctionType without loosing readability when writing interfaces and classes.
changed the title [-]`strictMethodType`: `strictFunctionType` for method syntax.[/-][+]Option `strictMethodType`: `strictFunctionType` for method syntax.[/+]on Mar 14, 2024
This wouldn't be a breaking change in existing TypeScript/JavaScript code
Anyone who turns on such a flag would undoubtedly see lots of breakage. Arrays are (unsoundly but conveniently) covariant in TypeScript and lots of code depends on that.
I love soundness but if I introduced a flag that broke everything when turned on, I don't think saying "it'll be off by default" would help much. This would be breaky.
Particularly since, because of how TS works, enabling the flag for your own project probably requires any dependencies you have to successfully compile against it, even if they donβt need it themselves. With options like this, in a lot of ways theyβre only βoptionalβ in theory.
Anyone who turns on such a flag would undoubtedly see lots of breakage. Arrays are (unsoundly but conveniently) covariant in TypeScript and lots of code depends on that.
I checked This wouldn't be a breaking change [...] because I propose to gate this behind an option.
My guess is that this should be less of an issue now than it was 7 years ago when `strictFunctionType' was introduced.
Arrays are (unsoundly but conveniently) covariant in TypeScript and lots of code depends on that.
Have you some practical examples? I am curious about that.
I love soundness but if I introduced a flag that broke everything when turned on, I don't think saying "it'll be off by default" would help much. This would be breaky.
Still, it makes sense for many projects to have this option enabled.
Particularly since, because of how TS works, enabling the flag for your own project probably requires any dependencies you have to successfully compile against it, even if they donβt need it themselves. With options like this, in a lot of ways theyβre only βoptionalβ in theory.
I am unsure to follow you. This could be checked at usage site? Similarly to strictFunctionType?
Assuming that enabling such an option doesn't break everything, having the option at least allows users to file issues and PRs on dependencies that don't work with it and work toward making the ecosystem compatible.
because even though every element of animals is an Animal, the array animals would not be an Animal[]. That is sound but would break lots of stuff. Indeed the standard way to avoid making a distributive conditional type is to wrap the type parameter in a one-tuple like
During development of this feature, we discovered a large number of inherently unsafe class hierarchies, including some in the DOM. Because of this, the setting only applies to functions written in function syntax, not to those in method syntax.
So it's not just Array (does anyone have a DOM example to put in here)?
I checked This wouldn't be a breaking change [...] because I propose to gate this behind an option.
I love soundness but if I introduced a flag that broke everything when turned on, I don't think saying "it'll be off by default" would help much. This would be breaky.
Still, it makes sense for many projects to have this option enabled.
You apparently didn't get my point. Having the flag off by default doesn't mean it's not a breaking change. Anyone who wants to use your option would see breakage. Just having the option exist means that large amounts of TypeScript code (including the standard libraries provided with TypeScript itself) would need to change so that people who turn on the flag only see problems in their own code and not in a large chunk of the libraries they depend on. You're not proposing merely adding a flag; you're proposing adding a flag and rewriting a lot of TypeScript declarations. It really doesn't help if the only way for this not to be a breaking change is if the flag is off. We can do that right now by not introducing the flag.
And again, I'm not saying "this is a bad suggestion", I'm saying "this would be a breaking change". TypeScript does sometimes make breaking changes. Even --strictFunctionTypes was a breaking change. Breaking changes happen. I'm saying that we should be clear about the effects of introducing such a flag.
Assuming that enabling such an option doesn't break everything
This actually does break everything; it would make it roughly impossible to obtain an array or similar "actually invariant" object.
For example, you can't write
constarr: number[]=[];
because the RHS is never[], so it's illegal to push a number into it.
This can't be fixed with contextual typing of literals since there are other cases like:
// Correct program rejected --// Invalid, RHS is number[]constarr: unknown[]=[1,2,3].slice();
The underlying problem is there's no available distinction between these two methods:
classArrayHolder{privatearr: number[]=[];// Safe to covariantly alias the return of this functiongetCopy(){returnarr.slice();}// Not safe to covariantly alias the return of this functiongetSelf(){returnarr;}}
Both are just "number[]" even though they have fundamentally different sound aliasing semantics.
For this flag to even be plausible, there'd need to be a reasonable way to solve this problem.
Most of its issues could be solved by using readonly arrays and carefully redesigning some function signatures (for example replacing T in includes by unknown or by a type parameter that extends T) in order to make readonly arrays covariant.
However, I accept that this could involve large breaking changes. Another possibility could create an exception for arrays. This is what Java is doing by making arrays covariant.
Given the current philosophy of TypeScript, I am no longer sure that this issue has any chance of being accepted.
Joe Calzaretta (@jcalz) i'd love to see the breakage in my code. i've carefully avoided bivariance where possible, but i don't feel confident at all that i didn't accidentally mess up somewhere the compiler didn't want to check for me.
Your dependencies on bivariance are almost certainly implicit.
Out of the gate, the core lib is broken:
../../github/TypeScript/built/local/lib.es2015.generator.d.ts:21:11-errorTS2430: Interface 'Generator<T,TReturn,TNext>' incorrectly extends interface 'Iterator<T,TReturn,TNext>'.Typesofproperty 'return' areincompatible.Type'(value: TReturn) => IteratorResult<T, TReturn>'isnotassignabletotype'(value?: TReturn | undefined) => IteratorResult<T, TReturn>'.Typesofparameters'value'and'value'areincompatible.Type 'TReturn|undefined' is not assignable to type 'TReturn'. 'TReturn' could be instantiated with an arbitrary type which could be unrelated to 'TReturn|undefined'.21interfaceGenerator<T=unknown,TReturn=any,TNext=unknown>extendsIterator<T,TReturn,TNext>{~~~~~~~~~../../github/TypeScript/built/local/lib.es2018.asyncgenerator.d.ts:21:11-errorTS2430: Interface 'AsyncGenerator<T,TReturn,TNext>' incorrectly extends interface 'AsyncIterator<T,TReturn,TNext>'.Typesofproperty 'return' areincompatible.Type'(value: TReturn | PromiseLike<TReturn>) => Promise<IteratorResult<T, TReturn>>'isnotassignabletotype'(value?: TReturn | PromiseLike<TReturn> | undefined) => Promise<IteratorResult<T, TReturn>>'.Typesofparameters'value'and'value'areincompatible.Type 'TReturn|PromiseLike<TReturn>|undefined' is not assignable to type 'TReturn|PromiseLike<TReturn>'.Type 'undefined' is not assignable to type 'TReturn|PromiseLike<TReturn>'.
21interfaceAsyncGenerator<T=unknown,TReturn=any,TNext=unknown>extendsAsyncIterator<T,TReturn,TNext>{~~~~~~~~~~~~~~../../github/TypeScript/built/local/lib.es5.d.ts:331:11-errorTS2430: Interface'CallableFunction'incorrectlyextendsinterface'Function'.Typesofproperty'apply'areincompatible.Type'{ <T, R>(this: (this: T) => R, thisArg: T): R; <T, A extends any[], R>(this: (this: T, ...args: A) => R, thisArg: T, args: A): R; }'isnotassignabletotype'(this: Function, thisArg: any, argArray?: any) => any'.The'this'typesofeachsignatureareincompatible.Type'Function'isnotassignabletotype'(this: any) => any'.Type'Function'providesnomatchforthesignature'(this: any): any'.331interfaceCallableFunctionextendsFunction{~~~~~~~~~~~~~~~~../../github/TypeScript/built/local/lib.es5.d.ts:368:11-errorTS2430: Interface'NewableFunction'incorrectlyextendsinterface'Function'.Typesofproperty'apply'areincompatible.Type'{ <T>(this: new () => T, thisArg: T): void; <T, A extends any[]>(this: new (...args: A) => T, thisArg: T, args: A): void; }'isnotassignabletotype'(this: Function, thisArg: any, argArray?: any) => any'.The'this'typesofeachsignatureareincompatible.Type'Function'isnotassignabletotype'new () => any'.Type'Function'providesnomatchforthesignature'new (): any'.368interfaceNewableFunctionextendsFunction{~~~~~~~~~~~~~~~
The DOM also has hundreds of errors because Events are not meant to be called from supertype aliases. It makes HTMLElement not assignable to Node due to addEventListener, so any DOM-related code gets broken quite hard.
This also breaks correct assumptions about array variance, specifically for readonly arrays. This code is sound by inspection:
But it errors because from TS's perspective, a method returning an invariant type must make the entire type invariant.
We'd need a number of fairly invasive scaffolding features to make this work palatably. As-is, "just" making methods covariant creates a lot of incorrect errors.
Alternatively we could require explicit notation o make a parameter/return type bivariant.
For instance, we could mark a parameter as covariant (contravariant could be implied) and a return type as contravariant (covariance could be implied):
This can't be fixed with contextual typing of literals since there are other cases like:
// Correct program rejected --
// Invalid, RHS is number[] const arr: unknown[] = [1, 2, 3].slice();
Why is this supposed to be a correct program? It looks more like some kind of hack/workaround. Who would intentionally cast a known type to an unknown in a normal project?
And for a specific hack, you can always cast it explicitly:
constarr: unknown[]=[1,2,3].slice()asunknown[]
.
This also breaks correct assumptions about array variance, specifically for readonly arrays. This code is sound by inspection:
But it errors because from TS's perspective, a method returning an invariant type must make the entire type invariant.
Yes, this code should fail. Firstly, a readonly array shoud have readonly [i: number]: T. And also, the push method of the original object is not supposed to accept a boolean type. Why should we allow to do this? Isn't it a design error? Why not declare the push method as a generic for this purpose?
So this also looks like some kind of hack to me. Trying to wrap one object in another type... I don't think it's really necessary in most projects.
The other examples in this thread were incorrect due to the lack of readonly.
I'd like to see a real problem with 'strictMethodType' in properly designed code. So far I only see weird examples that I wouldn't use.
As for DOM, this library itself is bad, because it relies on global variables. This is far from safe programming anyway. So when using it, 'strictMethodType' option could be disabled.
A similar option should also be added that makes { x: number } not subtype { x: number | string } (but still subtype { readonly x: number | string }). The setter for x of the second accepts the weaker type number | string and therefore should not be a supertype. This would help catch bugs such as the following at compile time:
Activity
[-]`strictMethodType`: `strictFunctionType` for method syntax.[/-][+]Option `strictMethodType`: `strictFunctionType` for method syntax.[/+]jcalz commentedon Mar 15, 2024
Anyone who turns on such a flag would undoubtedly see lots of breakage. Arrays are (unsoundly but conveniently) covariant in TypeScript and lots of code depends on that.
I love soundness but if I introduced a flag that broke everything when turned on, I don't think saying "it'll be off by default" would help much. This would be breaky.
fatcerberus commentedon Mar 15, 2024
Particularly since, because of how TS works, enabling the flag for your own project probably requires any dependencies you have to successfully compile against it, even if they donβt need it themselves. With options like this, in a lot of ways theyβre only βoptionalβ in theory.
Conaclos commentedon Mar 15, 2024
I checked
This wouldn't be a breaking change [...]because I propose to gate this behind an option.My guess is that this should be less of an issue now than it was 7 years ago when `strictFunctionType' was introduced.
Have you some practical examples? I am curious about that.
Still, it makes sense for many projects to have this option enabled.
I am unsure to follow you. This could be checked at usage site? Similarly to
strictFunctionType?ehoogeveen-medweb commentedon Mar 15, 2024
Assuming that enabling such an option doesn't break everything, having the option at least allows users to file issues and PRs on dependencies that don't work with it and work toward making the ecosystem compatible.
jcalz commentedon Mar 15, 2024
So, you're presumably aware that
Arrayhas apush()method likeAnd this would be contravariant with your proposed flag, making
Array<T>invariant inT. Then the following idiomatic TS code would break:because even though every element of
animalsis anAnimal, the arrayanimalswould not be anAnimal[]. That is sound but would break lots of stuff. Indeed the standard way to avoid making a distributive conditional type is to wrap the type parameter in a one-tuple likeAnd that would stop working and break all the things.
In the docs for
--strictFunctionTypesit says:So it's not just
Array(does anyone have a DOM example to put in here)?You apparently didn't get my point. Having the flag off by default doesn't mean it's not a breaking change. Anyone who wants to use your option would see breakage. Just having the option exist means that large amounts of TypeScript code (including the standard libraries provided with TypeScript itself) would need to change so that people who turn on the flag only see problems in their own code and not in a large chunk of the libraries they depend on. You're not proposing merely adding a flag; you're proposing adding a flag and rewriting a lot of TypeScript declarations. It really doesn't help if the only way for this not to be a breaking change is if the flag is off. We can do that right now by not introducing the flag.
And again, I'm not saying "this is a bad suggestion", I'm saying "this would be a breaking change". TypeScript does sometimes make breaking changes. Even
--strictFunctionTypeswas a breaking change. Breaking changes happen. I'm saying that we should be clear about the effects of introducing such a flag.RyanCavanaugh commentedon Mar 15, 2024
This actually does break everything; it would make it roughly impossible to obtain an array or similar "actually invariant" object.
For example, you can't write
because the RHS is
never[], so it's illegal to push anumberinto it.This can't be fixed with contextual typing of literals since there are other cases like:
The underlying problem is there's no available distinction between these two methods:
Both are just "
number[]" even though they have fundamentally different sound aliasing semantics.For this flag to even be plausible, there'd need to be a reasonable way to solve this problem.
Conaclos commentedon Mar 15, 2024
Most of its issues could be solved by using
readonlyarrays and carefully redesigning some function signatures (for example replacingTinincludesbyunknownor by a type parameter that extendsT) in order to make readonly arrays covariant.However, I accept that this could involve large breaking changes. Another possibility could create an exception for arrays. This is what Java is doing by making arrays covariant.
Given the current philosophy of TypeScript, I am no longer sure that this issue has any chance of being accepted.
ritschwumm commentedon Mar 19, 2024
Joe Calzaretta (@jcalz) i'd love to see the breakage in my code. i've carefully avoided bivariance where possible, but i don't feel confident at all that i didn't accidentally mess up somewhere the compiler didn't want to check for me.
RyanCavanaugh commentedon Mar 19, 2024
Your dependencies on bivariance are almost certainly implicit.
Out of the gate, the core lib is broken:
The DOM also has hundreds of errors because Events are not meant to be called from supertype aliases. It makes
HTMLElementnot assignable toNodedue toaddEventListener, so any DOM-related code gets broken quite hard.This also breaks correct assumptions about array variance, specifically for readonly arrays. This code is sound by inspection:
But it errors because from TS's perspective, a method returning an invariant type must make the entire type invariant.
We'd need a number of fairly invasive scaffolding features to make this work palatably. As-is, "just" making methods covariant creates a lot of incorrect errors.
You can try out this branch. https://github.com/RyanCavanaugh/TypeScript/tree/strictMethodTypes . It creates a new option "demoStrictMethodTypes"
Conaclos commentedon Mar 19, 2024
Alternatively we could require explicit notation o make a parameter/return type bivariant.
For instance, we could mark a parameter as covariant (contravariant could be implied) and a return type as contravariant (covariance could be implied):
I proposed a similar mechanism for Eiffel ten years ago.
ethanresnick commentedon Jun 12, 2024
Would it make sense to do this just for method syntax in object literals?
Perfectoff commentedon Nov 16, 2024
Why is this supposed to be a correct program? It looks more like some kind of hack/workaround. Who would intentionally cast a known type to an unknown in a normal project?
And for a specific hack, you can always cast it explicitly:
.
Yes, this code should fail. Firstly, a readonly array shoud have
readonly [i: number]: T. And also, the push method of the original object is not supposed to accept a boolean type. Why should we allow to do this? Isn't it a design error? Why not declare the push method as a generic for this purpose?So this also looks like some kind of hack to me. Trying to wrap one object in another type... I don't think it's really necessary in most projects.
The other examples in this thread were incorrect due to the lack of readonly.
I'd like to see a real problem with 'strictMethodType' in properly designed code. So far I only see weird examples that I wouldn't use.
As for DOM, this library itself is bad, because it relies on global variables. This is far from safe programming anyway. So when using it, 'strictMethodType' option could be disabled.
zwyz commentedon Feb 3, 2025
A similar option should also be added that makes
{ x: number }not subtype{ x: number | string }(but still subtype{ readonly x: number | string }). The setter forxof the second accepts the weaker typenumber | stringand therefore should not be a supertype. This would help catch bugs such as the following at compile time: