rocicorp / fractional-indexing

Fractional Indexing in JavaScript
https://npmjs.com/fractional-indexing
Creative Commons Zero v1.0 Universal
316 stars 23 forks source link

Document expected sort method #20

Open rosszurowski opened 11 months ago

rosszurowski commented 11 months ago

I've been working with this library to implement a reorderable interface, and ran into a point of confusion.

As someone who's worked with JS a while, I avoid the default .sort() method by default, since it commonly gives incorrect results when sorting strings. My default inclination is to do something like the following:

const items = [{ id: "1", sortOrder: "a0" }, { id: "2", sortOrder: "a0l" }, { id: "3", sortOrder: "a0V" }]
const sortedItems = items.sort((a, b) => a.sortOrder.localeCompare(b.sortOrder))

Using localeCompare caused my sort order to be incorrect, which resulted in a separate reorder method throwing errors when calling generateKeyBetween.

I saw from #19 that the preferred method is .sort(), or I suppose a re-implementation of the default sort algorithm, like so:

const sortedKeys = keys.sort((a, b) => {
  if (a.sortOrder < b.sortOrder) return -1
  if (a.sortOrder > b.sortOrder) return 1
  return 0
})

It might be nice to document that in the readme somewhere, so others don't run into the same issue.

arv commented 11 months ago

Makes sense to me. Care to add something to the README?