MichaelSolati / geofirestore-js

Location-based querying and filtering using Firebase Firestore.
https://geofirestore.com
MIT License
504 stars 58 forks source link

Option to sort by distance when a near query is used? #101

Closed adevine closed 5 years ago

adevine commented 5 years ago

When a "near" query is used with a center and radius, would be ideal to add an option to sort results by distance from the center. Right now the distance field is placed on the docs that come back, but the results are not sorted by distance.

MichaelSolati commented 5 years ago

I don't necessarily want to add a distance sort, as it's something that you as a developer can easily do, and it adds an extra layer to the library (which depending on your query size can slow down the result). Keep in mind we do a filter each time a query emits new values, and on average one geoquery is about 8 firestore queries covering the area you want.

There is however a little "hack" you could do... The limit method on a geoquery DOES sort the documents so that we ensure we show you the closest documents. So just add... .limit(1000000) (or some other absurdly high value) to your query and we will sort for you...

adevine commented 5 years ago

Thanks for the response @MichaelSolati . I understand your reasoning for not adding sort, but just wanted to comment that I don't believe your statement about using limit is correct, so don't want someone else to try it and then get stuck:

  1. Using a limit only sorts the results if you actually had more results in your returned result set than your limit, so setting a high limit won't work: https://github.com/geofirestore/geofirestore-js/blob/v3.2.3/src/GeoJoinerOnSnapshot.ts#L58 .
  2. Using a limit doesn't actually sort the results that come back in the docs array. It does a sort on a copy of the docs array, but then it uses that sort to remove elements from the docs array, but the docs array itself isn't actually sorted. And at least in my tests, where I had results that were of distances (for example) of 7, 11, 1, 4 (in that order), when I set a limit of 3 the furthest doc was removed, but the results still weren't sorted - then came back in order 7, 1, 4.

It's that #2 behavior that I find particularly strange. Ideally I think it would be best if setting a limit DID return the query docs in sorted order, but that doesn't appear to be the case.

MichaelSolati commented 5 years ago

Well I'm glad someone knows my work better than me 😅, it's been a while since I've looked at that part, and you're absolutely right! So I can't explain my reasoning for not returning the limited docs sorted... I guess I didn't see any reason for it as I imagined a user expected just a limit on items returned and I thought it made sense to limit based on order, but not to pass the order to a user. I'm open to this being changed, but I don't see it being something I'll be rushing to do (I'm open to a PR for this though).

For those of you who do need their snapshots sorted, heres a lil code for ya:

geoFirestore.collection('retaurants')
  .near({ center: new firebase.firestore.GeoPoint(1, 2), radius: 1000 })
  .onSnapshot((snapshot) => {
    const sorted = snapshot.docs.sort((a, b) => a.distance - b.distance);
  });