feat: ez.paginated()#3245
Merged
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds two-style pagination support: new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant GeneratedClient
participant Router
participant Handler
participant DataStore
Client->>GeneratedClient: GET /v2/users/list?limit&offset&roles
GeneratedClient->>Router: HTTP request to /v2/users/list
Router->>Handler: invoke listUsersPaginatedEndpoint with parsed input
Handler->>DataStore: filter by roles, compute total, slice by offset/limit
DataStore-->>Handler: return page items + total
Handler-->>Router: respond { users, total, limit, offset }
Router-->>GeneratedClient: HTTP 200 JSON
GeneratedClient->>Client: deliver response
Client->>Client: Client.hasMore(response) checks nextCursor or total vs offset+limit
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Clean, well-typed feature. The schema construction, overloads, hasMore logic, example, tests, and OpenAPI output all look correct. Two issues worth addressing — one is a real bug involving unvalidated defaults, the other a minor modifier ordering quirk.
Contributor
|
Responded to @RobinTail's question about the Zod 4 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
example/index.spec.ts (1)
114-119: Add direct coverage forClient.hasMore().This new test proves the endpoint works, but it still leaves the new public helper unexercised. A small-page request here with an explicit
Client.hasMore(...)assertion would make regressions in the generated client API visible.Possible test extension
- test("Should respond with paginated list (ez.paginated)", async () => { - const response = await fetch(`http://localhost:${port}/v2/users/list`); + test("Should respond with paginated list (ez.paginated)", async () => { + const response = await fetch(`http://localhost:${port}/v2/users/list?limit=1`); expect(response.status).toBe(200); - const json = await response.json(); + const json = (await response.json()) as { + data: Parameters<typeof Client.hasMore>[0]; + }; expect(json).toMatchSnapshot(); + expect(Client.hasMore(json.data)).toBe(true); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@example/index.spec.ts` around lines 114 - 119, Extend the existing test "Should respond with paginated list (ez.paginated)" to directly exercise the generated helper by creating a Client from the response payload and asserting Client.hasMore(...) for the small-page request to catch pagination regressions; after parsing json from the fetch to /v2/users/list, call Client.hasMore(json) (or the appropriate static/helper signature) and add an explicit expect(...) assertion (true/false depending on the known small-page fixture) so the new public helper Client.hasMore is covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@example/endpoints/list-users-paginated.ts`:
- Around line 30-33: Update the JSDoc comment above the endpoint to refer to
"users" instead of "items": clarify that ez.paginated() produces a response
shape of { users, total, limit, offset } (since itemsName is set to "users").
Edit the comment that currently reads "items, total, limit, offset" to the
correct "users, total, limit, offset" so it matches the itemsName configuration
and ez.paginated() behavior.
In `@express-zod-api/src/paginated-schema.ts`:
- Around line 55-69: The OffsetInput and CursorInput type aliases use the
non-existent z.ZodCoercedNumber for the limit and offset fields; update those
field types to z.ZodDefault<z.ZodNumber> (i.e., replace z.ZodCoercedNumber with
z.ZodNumber) for limit in both CursorInput and OffsetInput and for offset in
OffsetInput so the TypeScript types match schemas produced by
z.coerce.number()/z.number().
In `@express-zod-api/tests/paginated-schema.spec.ts`:
- Around line 184-230: The paginated builder allows itemsName that can clash
with fixed pagination keys and should reject reserved names; update the
paginated/schema builder (paginated-schema.ts) to validate the itemsName
parameter in the function that constructs the output schema (the code that
builds the object literal with [itemsName] plus fields like limit, offset,
total, nextCursor) and throw or zod-refine when itemsName equals any reserved
key ("limit", "offset", "total", "nextCursor", and any other fixed output field
such as "items"); then add a regression test in paginated-schema.spec.ts that
attempts to create paginated with itemsName set to a reserved key (e.g.,
"limit") and asserts that creation fails (throws or returns an error) to prevent
silent overwrites.
- Around line 56-70: The paginated builder currently applies
.default(defaultLimit) to limitSchema which allows a default that may exceed
maxLimit; update the ez.paginated implementation to validate the config upfront
(e.g., in the paginated factory function) by checking if defaultLimit > maxLimit
and throw a clear Error if so, and keep the limitSchema creation (limitSchema)
as-is or change to a pattern that ensures defaults are validated; also add a
test case to paginated-schema.spec.ts that constructs ez.paginated with
defaultLimit: 30 and maxLimit: 20 and asserts that it throws/ rejects the
invalid configuration.
---
Nitpick comments:
In `@example/index.spec.ts`:
- Around line 114-119: Extend the existing test "Should respond with paginated
list (ez.paginated)" to directly exercise the generated helper by creating a
Client from the response payload and asserting Client.hasMore(...) for the
small-page request to catch pagination regressions; after parsing json from the
fetch to /v2/users/list, call Client.hasMore(json) (or the appropriate
static/helper signature) and add an explicit expect(...) assertion (true/false
depending on the known small-page fixture) so the new public helper
Client.hasMore is covered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 35273d44-bb9b-48bf-9de0-40a0e82ab97a
⛔ Files ignored due to path filters (3)
example/__snapshots__/index.spec.ts.snapis excluded by!**/*.snapexpress-zod-api/tests/__snapshots__/index.spec.ts.snapis excluded by!**/*.snapexpress-zod-api/tests/__snapshots__/integration.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (12)
example/config.tsexample/endpoints/list-users-paginated.tsexample/example.client.tsexample/example.documentation.yamlexample/index.spec.tsexample/routing.tsexpress-zod-api/src/integration-base.tsexpress-zod-api/src/integration.tsexpress-zod-api/src/paginated-schema.tsexpress-zod-api/src/proprietary-schemas.tsexpress-zod-api/src/typescript-api.tsexpress-zod-api/tests/paginated-schema.spec.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
express-zod-api/src/paginated-schema.ts (1)
159-170:⚠️ Potential issue | 🟠 MajorReject invalid
defaultLimitvalues before building the schema.
z.default()in v4 short-circuits parsing, sodefaultLimit = 0,-1, or1.5bypasses.int().min(1)and becomes the returnedlimitwhenever the client omits it. GuarddefaultLimitwith the same invariants aslimitSchema.Suggested guard
if (maxLimit <= 0) throw new Error("ez.paginated: maxLimit must be greater than 0"); + if (!Number.isInteger(defaultLimit) || defaultLimit < 1) { + throw new Error( + "ez.paginated: defaultLimit must be a positive integer", + ); + } if (defaultLimit > maxLimit) { throw new Error( "ez.paginated: defaultLimit must not be greater than maxLimit", ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@express-zod-api/src/paginated-schema.ts` around lines 159 - 170, The function that builds paginated schemas (the factory accepting OffsetPaginatedConfig | CursorPaginatedConfig and returning OffsetPaginatedResult | CursorPaginatedResult) must validate the provided defaultLimit before constructing limitSchema because z.default() short-circuits parsing; add guards for defaultLimit to ensure it's an integer >= 1 and <= maxLimit (same invariants as limitSchema) and throw descriptive errors (e.g., "ez.paginated: defaultLimit must be an integer >= 1 and <= maxLimit") when the checks fail; perform these checks immediately after reading the maxLimit and defaultLimit parameters so limitSchema and any z.default(defaultLimit) cannot accept invalid defaults.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@express-zod-api/src/paginated-schema.ts`:
- Around line 112-154: The codegen is collapsing both pagination shapes into a
single Pagination type, causing offset endpoints to show cursor fields and vice
versa; update the generator logic in integration-base so it no longer emits one
shared Pagination = { nextCursor } | { total, limit, offset } for all endpoints
but instead emits a discriminated union or distinct types and references the
correct one per endpoint: detect when an endpoint uses paginated() as
OffsetPaginatedResult vs CursorPaginatedResult (symbols: paginated,
OffsetPaginatedResult, CursorPaginatedResult) and either (A) generate Pagination
= { style: "offset"; total: number; limit: number; offset: number } | { style:
"cursor"; nextCursor?: string } and use the matching branch per endpoint, or (B)
generate separate PaginationOffset and PaginationCursor types and have the
endpoint-specific output reference the appropriate type; change the code that
currently constructs the shared Pagination symbol in integration-base to pick
the proper shape based on the endpoint's paginated input/export type.
---
Duplicate comments:
In `@express-zod-api/src/paginated-schema.ts`:
- Around line 159-170: The function that builds paginated schemas (the factory
accepting OffsetPaginatedConfig | CursorPaginatedConfig and returning
OffsetPaginatedResult | CursorPaginatedResult) must validate the provided
defaultLimit before constructing limitSchema because z.default() short-circuits
parsing; add guards for defaultLimit to ensure it's an integer >= 1 and <=
maxLimit (same invariants as limitSchema) and throw descriptive errors (e.g.,
"ez.paginated: defaultLimit must be an integer >= 1 and <= maxLimit") when the
checks fail; perform these checks immediately after reading the maxLimit and
defaultLimit parameters so limitSchema and any z.default(defaultLimit) cannot
accept invalid defaults.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ce75a239-863b-4f88-8b7f-8db459d7420c
📒 Files selected for processing (3)
example/endpoints/list-users-paginated.tsexpress-zod-api/src/paginated-schema.tsexpress-zod-api/tests/paginated-schema.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- express-zod-api/tests/paginated-schema.spec.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Line 620: Add documentation for the Client.hasMore() method in the "Generating
a Frontend Client" section: describe its purpose (returns whether additional
pages are available), specify the expected inputs (e.g., response and
pagination/cursor info) and return value (boolean), and include a short usage
example showing calling Client.hasMore(response, pagination) to decide whether
to fetch the next page; update the link target or anchor text if needed so the
reference at line 620 points to this new explanation.
- Line 641: The inline comment "// or nextCursor" is ambiguous; update the
README examples to clearly separate offset and cursor pagination instead of
implying they can be mixed: replace or remove the comment next to the return {
users, total, limit, offset } example and add two explicit examples — one
showing an offset-style handler returning { users, total, limit, offset } (use
input: { limit, offset } and db.getUsers) and one showing a cursor-style handler
returning { users, nextCursor, limit } (use input: { limit, cursor } and
db.getUsersCursor); reference the return lines and handler examples in the
README so readers can see both formats clearly.
RobinTail
commented
Mar 8, 2026
RobinTail
commented
Mar 8, 2026
RobinTail
commented
Mar 8, 2026
RobinTail
commented
Mar 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ez.paginated()to simplify common tasks on paginating dataClient::hasMore()helperSummary by CodeRabbit
New Features
Documentation
Tests