discord.js
discord.js copied to clipboard
fix(collection)!: default sort comparison algorithm
Please describe the changes this PR makes and why it should be merged:
The documentation for Collection#sort
and Collection#toSorted
implies that if no comparison function is passed to the method, the default comparison behaviour mirrors that of Array.prototype.sort
– in other words, stringwise comparison by UTF-16 code points.
As things stand, that's not the case. There are a couple of significant issues with the current implementation of defaultSort
:
- The function doesn't implement stringwise comparison but instead just uses the
>
operator, which preferentially performs numeric comparison of its operands.- For example,
'1.0'
is sorted after1
with stringwise comparison, but'1.0' > 1
is false.
- For example,
- The function returns -1 (ie. A before B) if A is neither greater than B nor strict-equal to B. However, this isn't logically correct in a sort comparison context, and leads to violations of the required properties of a reliable comparator.
- For example:
const a = { a: 1 }; const b = { b: 2 }; defaultSort(a, b); // -1 defaultSort(b, a); // also -1
- For example:
I'm assuming that the intention is for the default behaviour to mirror that of Array.prototype.sort
, and so this PR rewrites the function to follow the ECMA-262 specification for the default comparison algorithm. The docblocks are also updated to reflect this more accurately.
Status and versioning classification:
- Code changes have been tested against the Discord API, or there are no code changes
- I know how to update typings and have done so, or typings don't need updating