Reuse "getBestMatchingType" logic during elaboration to allow for more specific elaborations#35278
Conversation
|
The original example just used interface Stuff {
a?: () => Promise<number[]>;
b: () => Promise<string>;
c: () => Promise<string>;
d: () => Promise<string>;
e: () => Promise<string>;
f: () => Promise<string>;
g: () => Promise<string>;
h: () => Promise<string>;
i: () => Promise<string>;
j: () => Promise<string>;
k: () => Promise<number>;
}
function foo(): Stuff | Date {
return {
a() { return [123] },
b: () => "hello",
c: () => "hello",
d: () => "hello",
e: () => "hello",
f: () => "hello",
g: () => "hello",
h: () => "hello",
i: () => "hello",
j: () => "hello",
k: () => 123
}
} |
It doesn't~ |
weswigham
changed the title
|
@DanielRosenwasser and now it does |
|
@typescript-bot test this |
|
Heya @DanielRosenwasser, I've started to run the extended test suite on this PR at 476318e. You can monitor the build here. It should now contribute to this PR's status checks. |
|
Heya @DanielRosenwasser, I've started to run the perf test suite on this PR at 476318e. You can monitor the build here. It should now contribute to this PR's status checks. Update: The results are in! |
|
Heya @DanielRosenwasser, I've started to run the parallelized community code test suite on this PR at 476318e. You can monitor the build here. It should now contribute to this PR's status checks. |
|
The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master. |
|
@DanielRosenwasser I merged the user update into this PR and the RWC update looks good. |
|
@DanielRosenwasser Here they are:Comparison Report - master..35278
System
Hosts
Scenarios
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
amcasey
left a comment
amcasey
left a comment
There was a problem hiding this comment.
I didn't read the code, but the new error messages look great.
| @@ -0,0 +1,77 @@ | |||
| tests/cases/compiler/errorOnUnionVsObjectShouldDeeplyDisambiguate.ts(17,7): error TS2322: Type '() => number[]' is not assignable to type '() => Promise<number[]>'. | |||
| Type 'number[]' is missing the following properties from type 'Promise<number[]>': then, catch | |||
There was a problem hiding this comment.
As a separate change, it might be nice to special case the message Foo is not assignable to Promise<Foo> to say something about adding await, rather than listing missing properties.
There was a problem hiding this comment.
I don't think it's as much about missing await since it's the target - but #35300 covers the basic idea here.
| @@ -0,0 +1,77 @@ | |||
| tests/cases/compiler/errorOnUnionVsObjectShouldDeeplyDisambiguate.ts(17,7): error TS2322: Type '() => number[]' is not assignable to type '() => Promise<number[]>'. | |||
| Type 'number[]' is missing the following properties from type 'Promise<number[]>': then, catch | |||
| tests/cases/compiler/errorOnUnionVsObjectShouldDeeplyDisambiguate.ts(18,16): error TS2322: Type 'string' is not assignable to type 'Promise<string>'. | |||
There was a problem hiding this comment.
But these ones didn't list missing properties?
There was a problem hiding this comment.
Primitives skip the property listing check. An array is treated like an arbitrary object type.
|
I guess there's a risk here - if two types have the same overlappiness, the compiler skips ahead to an arbitrary level of elaboration. Imagine type A = { a: string, b: number };
type B = { b: number, c: string };
let x: A | B = {
b: 123,
};That can be even more confusing for users in some cases. Maybe the filtering logic needs an option where you can set it to say of "return undefined if there's ambiguity between two constituents." Also would like to get @RyanCavanaugh's take on these changes. |
|
@DanielRosenwasser the error message in that case (and the excess property case) is unchanged with this, as the error for those isn't issued on the property, but rather on the parent of the property. |
|
@DanielRosenwasser @RyanCavanaugh you wanna do a thing with this at some point? |
|
I'm not either of those people, but this could fix the most common source of "I don't know where to begin" errors on my team. |
|
@DanielRosenwasser does that 👍 mean you want me to |
?
|
🚤 |
6 participants
Fixes #35248