samuelgozi / firebase-firestore-lite

A lightweight cloud firestore library for the browser
MIT License
212 stars 12 forks source link

Querying using startAt or endAt don't work, and don't allow specification of before = false #38

Closed cyderize closed 4 years ago

cyderize commented 4 years ago

At the moment, startAt and endAt take a document reference, which I don't think actually actually works.

I believe the problem is that the firestore REST API doesn't actually support using document references for cursors (as far as I can tell) - they're meant to take values corresponding to the fields of orderBy. By specifying a document reference, firestore thinks we're trying to set the cursor to a position where the orderBy field is the given document reference.

(Or at least I've only been able to make startAt work by changing it to take a value rather than doc reference)

There's also no way that I can see to specify before: false to get startAfter/endAfter behavior.

I'd be happy to do a pull request to fix this.

issue-label-bot[bot] commented 4 years ago

Issue-Label Bot is automatically applying the label bug to this issue, with a confidence of 0.66. Please mark this comment with :thumbsup: or :thumbsdown: to give our bot feedback!

Links: app homepage, dashboard and code for this bot.

samuelgozi commented 4 years ago

@cyderize Thanks for the issue. Over the last few weeks, I've been very busy with some other things, so I don't reply and fix things as fast as usual, but I'm gonna try to fix this in the next few days.

I do remember that there was a weird gotcha with startAt and endAt in the REST API, but I don't remember what it was.

samuelgozi commented 4 years ago

I got it: https://stackoverflow.com/questions/58602609/how-do-cursors-work-on-firestores-rest-api I asked this a long time ago.

cyderize commented 4 years ago

@samuelgozi I've opened a pull request #39 with my fix for this - when you have time it'd be great if you could take a look, or if you have a better way to dealing with this then feel free to close it.

samuelgozi commented 4 years ago

I've been working on this today(i saw your pull request) to experiment with different APIs and to check if there is a need to report a bug to the firebase team, but found some other small bugs in the library so im working on them too.

As I said before, I usually fix things in a matter of single days, but I really had some crazy past few weeks, so im sorry for the time it takes.

cyderize commented 4 years ago

No problem, thanks for writing this great library!

samuelgozi commented 4 years ago

Finally had some time to work on in. I've fixed it, and im currently writing some tests. I will close this issue when I push the changes. Please let me know if it works as expected. And feel free to tell me about any suggestions or changes you think can help!

samuelgozi commented 4 years ago

Update: Fixed the issue in v1.0.0-RC2.1. Currently startAt and endAt receive only one argument which should be an instance of Document. I plan on adding support for regular objects and also a before/after option hopefully soon. I just wanted to make sure a fix is ready ASAP.

Again, please let me know if something doesn't work as expected.

cyderize commented 4 years ago

Looks great! Once before/after works it'll be perfect. Thanks!

I've closed the pull request since it won't be needed anymore.