Fixed some variance measurements in type aliases of generic functions#54866
Fixed some variance measurements in type aliases of generic functions#54866Andarist wants to merge 1 commit intomicrosoft:mainfrom
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. |
| export function Stack<T>(): Stack<T>; | ||
| export function Stack<T>(collection: Iterable<T>): Stack<T>; | ||
| export interface Stack<T> extends Collection.Indexed<T> { | ||
| ~~~~~ |
There was a problem hiding this comment.
I have a hard time assessing if this is a good change or not, could use some help with that 😅
There was a problem hiding this comment.
This is probably a good change. This used to stack out, using more variance calculations removed the stack out, but I think have been hiding the error with incorrect results.
weswigham
left a comment
weswigham
left a comment
There was a problem hiding this comment.
I thought this change looked familiar - I'd built this into the suite of changes in #43887 long ago. I do think this is very much still needed, and since it can stand on its' own, we should probably take it (though I am biased and should get someone else to peek at it). There's a bit more that you should handle, like propagating this "erased" type through indexed access operations, similarly to how wildcardType already works, and any other type constructor that wildcards propagate through.
| export function Stack<T>(): Stack<T>; | ||
| export function Stack<T>(collection: Iterable<T>): Stack<T>; | ||
| export interface Stack<T> extends Collection.Indexed<T> { | ||
| ~~~~~ |
There was a problem hiding this comment.
This is probably a good change. This used to stack out, using more variance calculations removed the stack out, but I think have been hiding the error with incorrect results.
Do u know what kind of tests I should use to verify that I'm moving in the right direction? I guess that for the most part I can heavily borrow from #43887 but since you mentioned that was a broader change... I'm not really able to tell quickly which parts are borrowable and which ones are not 😅 |
|
With 6.0 out as the final release vehicle for this codebase, we're closing all PRs that don't fit the merge criteria for post-6.0 patches. If you think this was a mistake and this PR fits the post-6.0 patch criteria, please post to the 6.0 iteration issue with details (specifically, which PR and which patch criteria it satisfies). Next steps for PRs:
|
6 participants
An experiment on how the variance measurement could be improved when intersections and unions are involved in situations like this.
related to the issue found here, cc @RyanCavanaugh