Suggestion: Array.prototype.sort - remove the need for compareFn to handle undefined values

[Brookke]

Search Terms

Array.prototype.sort undefined, Array.prototype.sort, sort undefined, extends undefined ? never : T, Array prototype sort, Array.prototype

Suggestion

Based on my understanding of the JS spec the Array.prototype.sort compareFunction is never called on undefined values in an array (they are always sorted to the end of the array).

all undefined elements are sorted to the end of the array, with no call to the compareFunction
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/sort

However, currently the inputs of the compareFn have the type T where T is the types of values in the array. Which forces the developer to make their compare function handle undefined values, despite the compare function never being called with undefined values.

Current

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

Suggested

sort(compareFn?: (a: T extends undefined ? never : T, b: T extends undefined ? never : T) => number

Use Cases / Examples

The below which is not currently valid would now be valid.

[1, undefined, 2].sort((a: number, b: number) => a - b)

Checklist

My suggestion meets these guidelines:

• This wouldn't be a breaking change in existing TypeScript/JavaScript code
• This wouldn't change the runtime behavior of existing JavaScript code
• This could be implemented without emitting different JS based on the types of the expressions
• This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)
• This feature would agree with the rest of TypeScript's Design Goals.

[Brookke]

Not sure if this is classed as a bug fix or a suggestion, so out of caution I have marked it as a suggestion. If this gets approved as an issue then I will open a PR with a fix!

[RyanCavanaugh]

Making this a conditional type is going to make a lot of things a lot worse in terms of assignability and contextual typing, and honestly this is the first I've ever even heard of this behavior. Doesn't seem like a benefit on net given the current rules around conditional types.

[Brookke]

@RyanCavanaugh Forgive my lack of understanding, but could you please explain how this will make assignability and contextual typing worse? I have looked at the documentation for contextual typing and I don’t understand how it would make things worse.

Additionally, I have created a playground to demo the behaviour Playground Link and a more complete example that may help make the benefit more obvious.

Example below errors, but would compile with my proposed change

function compareWordLength(a: string, b: string) {
     return a.length - b.length;
}

console.log(["hi", undefined, "hello", "hey"].sort(compareWordLength))

Example that currently compiles, but requires far more code that is never actually called and only leads to confusion.

/**
* considers undefined words to have length 0, 
* however because sort() never calls this with undefined values, 
* undefined values are always placed at the end of the array
*/
function compareWordLength(a: string | undefined, b: string | undefined) {
   let lengthA, lengthB: number;

   if (a === undefined) {
       console.log("I am never called");
       lengthA = 0;
   } else {
       lengthA = a.length;
   }
   
   if (b === undefined) {
       console.log("I am never called");
       lengthB = 0;    
   } else {
       lengthB = b.length;
   }
   
   return lengthA - lengthB;
}

console.log(["hi", undefined, "hello"].sort(compareWordLength))

[RyanCavanaugh]

Mmm, maybe it's not as bad as I expected.

[Brookke]

I have a branch with a fix ready for a PR once this gets marked as help wanted/added to the backlog

[Brookke]

A PR is now open with a fix for this

[Brookke]

Hey @RyanCavanaugh is there any chance I could get someone to have a look over this issue and the corresponding pr, for more feedback? ☺️