Typescript: Array.prototype.sort make compareFn required in lib.d.ts

Created on 6 Sep 2017  路  9Comments  路  Source: microsoft/TypeScript

This is a suggestion to change the signature declaration of Array sort in lib.d.ts from

sort(compareFn?: (a: T, b: T) => number): this;

to

sort(this: string[], compareFn?: (a: string, b: string) => number): this;
sort(compareFn: (a: T, b: T) => number): this;

i.e. make compareFn non-optional for non-string arrays. This would result in errors in cases such as

[2, 10].sort() // returns [10, 2], i.e. sorted lexicographically, which is very unintuitive.

While this is strictly speaking a breaking change, it wouldn't be the first time the typings are stricter than the actually definitions.

lib.d.ts Experimentation Needed Suggestion

Most helpful comment

@DanielRosenwasser is going to try this and see what breaks

All 9 comments

Is this part of Typescript's mandate, to make built-in Javascript APIs safer to use?

We've done it in certain locations, like parseInt.

Seems reasonable. I do the same special casing for string[] when I write a distinct function.

As far as the goal part, it feels like goal:

  1. Statically identify constructs that are likely to be errors.

Personally, I can't conceive of a situation where this would be the desired functionality. TypeScript already prevents illogical comparisons and several other challenges. While this is slightly higher order, it does feel very much in the spirit of TypeScript. The only _downside_ is that the error message can't provide any additional information as to why it is an error. Of course the JSDoc _could_ provide some information in the overload as to why it is a _bad_ idea.

Support seems to be unanimous, if you tag this as Accepting PRs I'll submit one.

@DanielRosenwasser is going to try this and see what breaks

@DanielRosenwasser dooooo iiiittttt break the world go go go

I'd like to see this as well - here's an example where this mistake made it into production: microsoft/vscode#86440

Was this page helpful?
0 / 5 - 0 ratings

Related issues

RyanCavanaugh picture RyanCavanaugh  路  205Comments

jonathandturner picture jonathandturner  路  147Comments

Taytay picture Taytay  路  174Comments

disshishkov picture disshishkov  路  224Comments

nitzantomer picture nitzantomer  路  135Comments