Avoid creating rest elements with errorType when any is spread
#57116
Conversation
typescript-bot
added
the
For Uncommitted Bug
|
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
| @@ -24,10 +24,10 @@ declare const itNum: Iterable<number> | |||
|
|
|||
| ;(function(a, ...rest) {})('', true, ...itNum) | |||
| >(function(a, ...rest) {})('', true, ...itNum) : void | |||
| >(function(a, ...rest) {}) : (a: string, rest_0: boolean, ...rest_1: any[]) => void | |||
| >function(a, ...rest) {} : (a: string, rest_0: boolean, ...rest_1: any[]) => void | |||
| >(function(a, ...rest) {}) : (a: string, rest_0: boolean, ...rest_1: Iterable<number>[]) => void | |||
There was a problem hiding this comment.
This test targets es5 where Iterable doesn't exist. This change shows how now an unresolved type continues to be displayed with its alias name~. Just like here:
type Test = {
foo: Thing; // even though it's unresolved and errors, it is still displayed as `Thing`
};And actually, this matches the display of those IIFEs above this one. So it turns out that this one was going through createNormalizedTupleType that accidentally lost this display value.
I still haven’t figured out how to interpret these tildes in your text |
|
😅 I use this as a softener of sorts - when I'm not sure if the used term is 100% precise etc ;p Sorry for making it confusing, I'm not sure where I got it from. |
sandersn
added
the
For Milestone Bug
typescript-bot
removed
the
For Milestone Bug
|
Turns out this is a fix for a break introduced in #57031 and reported a few days ago in #57389 @ahejlsberg @Andarist reported this as bug in #57389. Can you confirm and decide whether it's worth taking in 5.4? |
|
@typescript-bot test top100 |
|
Heya @sandersn, I've started to run the diff-based top-repos suite on this PR at a08d7d7. You can monitor the build here. Update: The results are in! |
|
Heya @sandersn, I've started to run the diff-based user code test suite on this PR at a08d7d7. You can monitor the build here. Update: The results are in! |
|
Heya @sandersn, I've started to run the parallelized Definitely Typed test suite on this PR at a08d7d7. You can monitor the build here. Update: The results are in! |
|
Hey @sandersn, I've packed this into an installable tgz. You can install it for testing by referencing it in your and then running There is also a playground for this build and an npm module you can use via |
|
@sandersn Here are the results of running the user test suite comparing There were infrastructure failures potentially unrelated to your change:
Otherwise... Something interesting changed - please have a look. Details
|
|
Hey @sandersn, the results of running the DT tests are ready. |
|
@sandersn Here are the results of running the top-repos suite comparing Everything looks good! |
|
@typescript-bot test top200 |
|
Heya @DanielRosenwasser, I've started to run the diff-based top-repos suite on this PR at a08d7d7. You can monitor the build here. Update: The results are in! |
|
@DanielRosenwasser Here are the results of running the top-repos suite comparing Everything looks good! |
|
@typescript-bot perf test this |
|
Heya @jakebailey, I've started to run the regular perf test suite on this PR at a08d7d7. You can monitor the build here. Update: The results are in! |
|
@jakebailey Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
tsserverComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
startupComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
This is also observable with something like this: type SomeCallback<Args extends readonly any[]> = (...args: Args) => void;
function call<Callback extends SomeCallback<any>>(x: Callback, args: Parameters<Callback>) {
x(...args); // error, pre-PR: Type 'Parameters<Callback>' must have a '[Symbol.iterator]()' method that returns an iterator.(2488)
}Maybe this is just a simpler #55932 but I only knew how to fix this particular issue (which I actually ran into) because I've been reading the TypeScript issue tracker. |
This isn't particularly observable as far as I know. However, it turns out that currently
[...any]is normalized~ to[...errorType].errorTypeis a specialanyand displays as such, so this went unnoticed. However, since[...any]is legal it doesn't make sense to produceerrorTypein this situation at all.EDIT:// this is more observable than what I've described above. I just pushed out a test for this. This PR fixes #55932 that was incorrectly~ attributed to be a duplicate of #29919
fixes #57389