-
Notifications
You must be signed in to change notification settings - Fork 0
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: Add pagination to /devices/list #236
Conversation
Bundle |
@@ -52,7 +83,38 @@ export default withRouteSpec({ | |||
) | |||
} | |||
|
|||
devices = sortBy(devices, ["created_at", "device_id"]) |
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.
shouldn't we do this in the db level?
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, but I did this intentionally here for now as we will need to factor all of this core logic out for pagination support on all endpoints. As part of that, every query will need to ensure it's ordered.
page_cursor: z | ||
.string() | ||
.optional() | ||
.nullable() |
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.
why nullable? What case the client passes null in this field?
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.
It makes type DX much better, otherwise users have to use "?? undefined" or add null checks. But null / undefined are basically the same here.
I found nullable is easier for type DX. There is no functional difference in this case and otherwise you have to do a lot of |
No description provided.