Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature Request: Pagination cursors #90

Open
wovalle opened this issue Oct 8, 2019 · 13 comments
Open

Feature Request: Pagination cursors #90

wovalle opened this issue Oct 8, 2019 · 13 comments
Labels
help wanted Extra attention is needed planning
Milestone

Comments

@wovalle
Copy link
Owner

wovalle commented Oct 8, 2019

Firestore supports some pagination cursors such as startAt, endAt and startAfter (the use of offset is uncouraged by google.

I find the naming confusing and non-generic. Let's use this issue to discuss how an implementation of those function would look like in fireorm and from there create issues to do the actual implementation

@wovalle wovalle added help wanted Extra attention is needed planning labels Oct 8, 2019
@wovalle wovalle added this to the next-major milestone Oct 8, 2019
@dmythro
Copy link

dmythro commented Jun 9, 2020

Hi @wovalle, is there a proper way to apply a cursor with current version? I see private readonly firestoreColRef and no option to easily extend this. Don't care about naming, need pagination cursors to at least work as they are defined on Firestore side.

@wovalle
Copy link
Owner Author

wovalle commented Jun 10, 2020

I don't have a solution for this yet 😔, haven't put too much attention into a way make a clean api (which is my current limitation).

Do you have any fresh ideas?

@dmythro
Copy link

dmythro commented Jun 10, 2020

We have a quick update to use at least startAfter. Could submit it as PR. Because without it we'll have to switch to something else or fork a module.

But for full-featured update and other cursors will need some more work.

@dmythro
Copy link

dmythro commented Jun 10, 2020

Currently it works by extending BaseFirestoreRepository, QueryBuilder , AbstractFirestoreRepository with couple required props and methods.

@garviand
Copy link

garviand commented Dec 8, 2020

@wovalle I'm interested in working on this. After thinking about it for a bit, the only ways I see this working are:

  1. Adding functions to the query builder that are similar to functions in the standard library like startAfter, startAt, etc.

OR

  1. Update the query builder execute function to return an object that has a cursor and results, which would significantly change the return type of queries. I know this is a breaking change but I see it returning something like:
{
  results: T[], # results of query
  cursor:  DocumentRef # document ref of last result
}

And you could add a function like after(cursor) to a query to get the next page of results. Let me know what you think.

@garviand
Copy link

garviand commented Dec 8, 2020

I guess it's technically possible to add the ref as metadata for the returned entities and use that to pass to the next query.

@garviand
Copy link

garviand commented Dec 8, 2020

I built a super hacky proof of concept, there are some very obvious issues but it might make sense to consider this approach: garviand#3

@wovalle
Copy link
Owner Author

wovalle commented Dec 9, 2020

As mentioned in #211 (comment) I'll take a look at this next week. I didn't have any idea of how to implement this one so I have dedicate some time to actually test it. Thanks for your great work!

@garviand
Copy link

garviand commented Feb 9, 2021

Hey @wovalle I rebased my pagination branch and made some updates. Here is the new draft PR: garviand#5

If you have some time, please take a look and let me know what you think. I took some shortcuts to get this prototype done and have some ideas on how to improve it but I'm curious whether you think I am on the right track! Thanks!

@garviand
Copy link

@wovalle Just a heads up, my approach won't work. When you stringify an array, the named properties are removed so when passing the cursor via HTTP for example, it will not be included in the response. So basically this would only work if the array is passed as a JS object which doesn't really help in practice.

@tzvc
Copy link

tzvc commented Apr 22, 2021

Any updates on this?

@SimonSch
Copy link

Any updates?

@wovalle
Copy link
Owner Author

wovalle commented Aug 27, 2021 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed planning
Projects
None yet
Development

No branches or pull requests

5 participants