Overloads in Array.concat now handle ReadonlyArray#21462
Conversation
Previously it was union types, which is slower.
|
I'm not a fan of adding more overloads. Instead, I propose we introduce a new type interface InputArray<T> {
readonly length: number;
readonly [n: number]: T;
join(separator?: string): string;
}/**
* Combines two or more arrays.
* @param items Additional items to add to the end of array1.
*/
concat(...items: InputArray<T>[]): T[];
/**
* Combines two or more arrays.
* @param items Additional items to add to the end of array1.
*/
concat(...items: (T | InputArray<T>)[]): T[];This works because The baselines look fine with this change and checker performance when compiling the compiler improves by as much as 10%! |
|
sounds good to me. |
|
@DanielRosenwasser and I considered this approach but didn't use it because it didn't show as much performance improvement as the current one. |
|
Performance is the same between the additional overloads and the additional interface. I would say it's down to which we think is easier to understand in quickinfo. |
|
After some discussion with @weswigham, he points out that the additional interface is more dangerous because you can satisfy it with |
…ft/TypeScript into use-overloads-for-concat
|
@mhegazy @sandersn I looked at the original issue (#20268) in more depth and I'm now convinced that it isn't a bug. See this comment #20268 (comment). |
|
Nevermind, #20268 is indeed a bug as I explain here #20268 (comment). I think the best way to fix it is the interface InputArray<T> {
readonly length: number;
readonly [n: number]: T;
join(separator?: string): string;
slice(start?: number, end?: number): T[];
}I think it is extremely unlikely something will match by accident. And, even with the current definitions something could match by accident. That's the nature of structural typing. |
|
I don't think making the interface one method larger buys us that much safety in practice. I'd prefer to leave InputArray as it is. |
Should make it, respectively, easier to understand this specific type and harder to satisfy it by mistake.
* Overloads in Array.concat now handle ReadonlyArray Previously it was union types, which is slower. * Make arrayConcat3 test stricter * Switch to InputArray instead of adding overloads * Update baselines * Update baselines correctly * Rename to ConcatArray and add slice method Should make it, respectively, easier to understand this specific type and harder to satisfy it by mistake.
|
This is now in 2.7 as well. |
Previously it was union types, which is slower.