-
Notifications
You must be signed in to change notification settings - Fork 206
feat: cursor based pagination for assets and entries [CAPI-2342] #2588
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
base: master
Are you sure you want to change the base?
Conversation
3b7c099 to
032d7d8
Compare
032d7d8 to
19610d1
Compare
19610d1 to
efd6a8c
Compare
| // omit pagePrev and pageNext if the value is falsy | ||
| ...(pagePrev ? { pagePrev } : null), | ||
| ...(pageNext ? { pageNext } : null), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // omit pagePrev and pageNext if the value is falsy | |
| ...(pagePrev ? { pagePrev } : null), | |
| ...(pageNext ? { pageNext } : null), | |
| ...(!!pagePrev && { pagePrev }), | |
| ...(!!pageNext && { pageNext }), |
lib/types/collection.ts
Outdated
| */ | ||
| export type CursorPaginatedCollection<T = unknown> = CollectionBase<T> & CursorPagination | ||
|
|
||
| export type WithCursorPagination = { cursor: true } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wondering does this take into account that you can have just ?pageNext=abc and you will get a cursor based result?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This type is only for internal use in CollectionForQuery generic. Since we use the same makeGetEntries function for both offset and cursor pagination it's needed to type the response for cursor pagination properly here. Actually I think I can remove and inline it to avoid confusion 👍
In the context of getEntriesWithCursor method we return cursor paginated collection anyways, even when pageNext or pagePrev is not provided.
efd6a8c to
316097e
Compare
michaelphamcf
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Itty bitty typo I think. Otherwise, Looks good to me.
lib/types/query/query.ts
Outdated
| FixedPagedOptions | ||
|
|
||
| type CursorPaginationOptions = { | ||
| type ConcenptsCursorPaginationOptions = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| type ConcenptsCursorPaginationOptions = { | |
| type ConceptsCursorPaginationOptions = { |
Typo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, thank you! Fixed
f79c3bb to
e839302
Compare
e839302 to
c60677a
Compare
Uh oh!
There was an error while loading. Please reload this page.