[Bug fix] Make pagination work with deletes #45
Conversation
commit: |
ianmacartney
left a comment
ianmacartney
left a comment
There was a problem hiding this comment.
I'll have to dig in later for a deeper review.
At a high level I'd love to re-use some of the work we've done with helpers & paginator that already simulates pagination with user-space cursor keys.
Having this + the helpers + the core implementation feels like a recipe for behavior inconsistency.. but I haven't thought much about how easy it'd be to map the paginator/streams code in here
| if (e instanceof Error && e.message.includes("expected JSON array")) { | ||
| throw e; | ||
| } | ||
| throw new Error("Invalid cursor format: must be a JSON array or null"); |
There was a problem hiding this comment.
nit: the user won't be passing in a JSON array - they'll be passing an opaque string and likely will be confused if they see this. Maybe guide them that the cursor needs to be null or a string returned from paginate
|
let me know when I should take a look
…On Tue, Jul 15, 2025 at 2:46 PM Jordan Hunt ***@***.***> wrote:
@jordanhunt22 <https://github.com/jordanhunt22> requested your review on:
#45 <#45> [Bug fix] Make
pagination work with deletes.
—
Reply to this email directly, view it on GitHub
<#45 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACZQW6ELXGGX3OQHLFM2C33IVZFDAVCNFSM6AAAAACBQC5NO6VHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJYGY2DOMZTGAYTOMI>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
ianmacartney
left a comment
ianmacartney
left a comment
There was a problem hiding this comment.
@jordanhunt22 btw just noticed we never pushed this through
yeah, unfortunately i think the refactor will take a bit of work. i'll pick this back up when i have some free time. |
While doing pagination, if the document at the end of a page was deleted, it would cause pagination to break because we used that document's
_idas the cursor. Thus, when the document was deleted, we couldn't find the beginning of the next page since the cursor's_idno longer existed.I fixed this behavior by using the index keys of the last document in a page as the cursor. That way, if the document is deleted, we can still resume the paginated query at the correct place.
I added tests to confirm this behavior.