Improve inference between types with multiple signatures #54448
Conversation
typescript-bot
added
Author: Team
For Milestone Bug
|
@typescript-bot test this |
|
Heya @ahejlsberg, I've started to run the diff-based user code test suite on this PR at 4027c38. You can monitor the build here. Update: The results are in! |
|
Heya @ahejlsberg, I've started to run the abridged perf test suite on this PR at 4027c38. You can monitor the build here. Update: The results are in! |
|
Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at 4027c38. You can monitor the build here. Update: The results are in! |
|
Heya @ahejlsberg, I've started to run the diff-based top-repos suite on this PR at 4027c38. You can monitor the build here. Update: The results are in! |
|
Heya @ahejlsberg, I've started to run the extended test suite on this PR at 4027c38. You can monitor the build here. |
|
@ahejlsberg Here they are:Comparison Report - main..54448
System
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Heya @ahejlsberg, I've run the RWC suite on this PR - assuming you're on the TS core team, you can view the resulting diff here. |
|
@ahejlsberg Here are the results of running the user test suite comparing There were infrastructure failures potentially unrelated to your change:
Otherwise... Everything looks good! |
|
@ahejlsberg Here are the results of running the top-repos suite comparing Everything looks good! |
|
Hey @ahejlsberg, the results of running the DT tests are ready. Branch only errors:Package: jquery |
|
@ahejlsberg This has an effect on the code reported in #53508 type Overloads<T> = T extends {
(...args: infer A1): infer R1;
(...args: infer A2): infer R2;
}
? [(...args: A1) => R1, (...args: A2) => R2]
: T extends {
(...args: infer A1): infer R1;
}
? [(...args: A1) => R1]
: any;
interface A {
hello(str: string): string
}
// was: [(str: string) => string]
// is: [(str: string) => string, (str: string) => string]
type HelloOverloadsOfA = Overloads<A['hello']>;
interface B {
hello(): number
hello(str: string): string
}
// was: [() => number, (str: string) => string]
// is: [() => number, (str: string) => string]
type HelloOverloadsOfB = Overloads<B['hello']>;
interface C {
hello(): number
}
// was: [(...args: unknown[]) => unknown, () => number]
// is: [() => number, () => number]
// requested by OP: [() => number]
type HelloOverloadsOfC = Overloads<C['hello']>; |
The current state of affairs of inference between types with multiple signatures is confusing to say the least. When the source has fewer signatures than the target, we make no inferences to the excess target signatures, leaving type variables in those signatures to assume their defaults (typically With the changes in this PR, our behavior is much more consistent: We'll always make inferences to each signature in the target, ensuring that every type variable in the target is covered. It doesn't give you the ability to measure overload arity with a ladder (because the first step will always succeed), but that ability wasn't really there to begin with. |
|
Just a note of clarification - I intentionally said that this has an effect on that code and not that it introduces a regression. After analyzing this a little bit better, I think this is a fine change. Just wanted to double-check by mentioning that issue since I distinctly remember that it existed and that it related to this PR. Maybe worth adding |
|
Let's wait until Monday for TS 5.3. |
|
Pushing a merge commit |
There was a problem hiding this comment.
I guess this is fine if the extended suites are OK with it - no worse than the existing inference logic, anyway.
What gets me is I know, since this logic differs from the logic in signaturesRelatedTo (which has way more special cases), we can infer things from a different order of signature pairs (with differing levels of instantiation) than we end up relating, so we can ultimately choose an inference that ends up failing relationship checks (which makes it a poor inference candidate). Again, already possible, so this doesn't change anything in that regard, just makes the behavioral drift between relating types and inferring between signatures in types a bit bigger.
…4448) Co-authored-by: Ryan Cavanaugh <RyanCavanaugh@users.noreply.github.com> Co-authored-by: TypeScript Bot <typescriptbot@microsoft.com>
Fix TypeScript 5.3 compatibility. @perfective/common build fails since the TypeScript v5.3.0-dev.20230824. Type inference for the `nothing()` and `nil()` function has changed (probably caused by microsoft/TypeScript#54448). The code like: ``` const maybe: Maybe<number> = nothing(); ``` started to fail, as the `nothing()` return type is inferenced as `number | null | undefined` instead of just `number`. One solution is to explicitly set the type: ``` const maybe: Maybe<number> = nothing<number>(); ``` This solution may require significant amount of code updates across users' codebase. The applied alternative solution was to change the `nothing()` and `nil()` return types to `Nothing<Present<T>>`. In this case, the calls of `nothing()`/`nil()` remain as is, but `Maybe` and `Match` had to use additional type casts internally. This approach is chosen for the v0.10.0, so the code is compatible with TypeScript v5.3. It may be reconsidered in v0.11.0 if the root cause of the different type inference will be located.
Fix TypeScript 5.3 compatibility. @perfective/common build fails since the TypeScript v5.3.0-dev.20230824. Type inference for the `nothing()` and `nil()` function has changed (probably caused by microsoft/TypeScript#54448). The code like: ``` const maybe: Maybe<number> = nothing(); ``` started to fail, as the `nothing()` return type is inferenced as `number | null | undefined` instead of just `number`. One solution is to explicitly set the type: ``` const maybe: Maybe<number> = nothing<number>(); ``` This solution may require significant amount of code updates across users' codebase. The applied alternative solution was to change the `nothing()` and `nil()` return types to `Nothing<Present<T>>`. In this case, the calls of `nothing()`/`nil()` remain as is, but `Maybe` and `Match` had to use additional type casts internally. This approach is chosen for the v0.10.0, so the code is compatible with TypeScript v5.3. It may be reconsidered in v0.11.0 if the root cause of the different type inference will be located.
Implements what I describe here.
Fixes #28867.