-
Notifications
You must be signed in to change notification settings - Fork 180
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
(feat, cli): add has-next-page
property to IR
#4241
Conversation
propertyComponents: paginationPropertyComponents.step | ||
}) | ||
: undefined, | ||
hasNextPage: | ||
paginationPropertyComponents.step != 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.
This should be hasNextPage
, right?
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.
thank you yup
@@ -5,6 +5,10 @@ All notable changes to this project will be documented in this file. | |||
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), | |||
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). | |||
|
|||
## [v53.5.0] - 2024-08-05 \*\* (TODO: Make required in next major) | |||
|
|||
- Feature: Support a `hasOffset |
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.
nit: hasNextPage
if (primitiveType == null) { | ||
return false; | ||
} | ||
return primitiveType === "BOOLEAN"; |
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.
I could see us eventually supporting string
so that it can be used similar to an empty cursor token, but let's start with this for now, especially because there aren't any IR changes required.
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.
Looks great overall! The change in convertOffsetPagination.ts
is the only blocker, but approving in the meantime.
5856975
to
77fcd7c
Compare
2b6709d
to
823dc8d
Compare
This PR introduces a
has-next-page
property to IR for offset pagination. If the response from an offset paginated contains a boolean about whether or not the next page exists, then this will allow the SDK to compute if the next page exists without making an RPC call.