search

troch / route-node

A package to create a tree of named routes
MIT License
20 stars 15 forks source link

`.add()` does not order the routes in a stable way #15

Closed davazp closed 6 years ago

davazp commented 6 years ago

Hi,

According to this comment in the .add() method:

https://github.com/troch/route-node/blob/master/modules/RouteNode.js#L111-L142

the order of the routes should be preserved if they are equivalent under all the conditions (same length of the last segment) and so on. However, it uses the sort method of an array, which does not ensure the sort is stable:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/sort

We found a weird issue where the order of the routes swapped two routes each time a new route was added to the router.

Please, let's add extra conditions to avoid returning 0 from the sort callback!

Thanks!

davazp commented 6 years ago

Perhaps replace sort with a stable sort like the one implemented at

https://stackoverflow.com/questions/1427608/fast-stable-sorting-algorithm-implementation-in-javascript

?

troch commented 6 years ago

Is your issue with more than 10 child routes in Chrome (V8)?

davazp commented 6 years ago

Yes, I have more than 10 routes and V8 (but on NodeJS).

troch commented 6 years ago

Sort is unstable on V8 for arrays with more than 10 items. Can you make a PR to not return 0 and instead return the index difference?