feat: api-common supporting numeric cursors #832
Conversation
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Pagination Schema Renaming and Type Widening packages/app/api-common/src/apiSchemas.ts, packages/app/api-common/src/apiSchemas.spec.ts |
Renamed multiCursorMandatoryPaginationSchema → encodedCursorMandatoryPaginationSchema and multiCursorOptionalPaginationSchema → encodedCursorOptionalPaginationSchema. CursorType generic now allows number. zMeta.hasMore is required. Updated tests and snapshots for GUIDs and encoded number cursors. |
Cursor Codec Expansion packages/app/api-common/src/cursorCodec.ts, packages/app/api-common/src/cursorCodec.spec.ts |
encodeCursor() now accepts `Record<string, unknown> |
Pagination Utilities Refactoring packages/app/api-common/src/paginationUtils.ts, packages/app/api-common/src/paginationUtils.spec.ts |
Reordered getMetaForNextPage to (currentPageData, pageLimit, cursorKeys?). createPaginatedResponse() gains optional cursorKeys parameter and overloads; cursors can be derived from single or multiple keys and numeric values are encoded. Removed getPaginatedEntries export and related tests. |
Dependency Management packages/app/api-common/package.json |
Removed runtime dependency @lokalise/universal-ts-utils from dependencies. |
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~45 minutes
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title 'feat: api-common supporting numeric cursors' is concise and clearly describes the main change—adding numeric cursor support to pagination utilities. |
| Description check | ✅ Passed | The description includes the required 'Changes' section explaining the feature and all three checklist items are marked complete, meeting the template structure. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing touches
- 📝 Generate docstrings
📜 Recent review details
Configuration used: Repository: lokalise/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/app/api-common/src/apiSchemas.spec.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/app/api-common/src/apiSchemas.spec.ts (3)
packages/app/api-common/src/apiSchemas.ts (2)
encodedCursorMandatoryPaginationSchema(36-47)encodedCursorOptionalPaginationSchema(48-52)packages/app/api-common/src/index.ts (1)
encodeCursor(2-2)packages/app/api-common/src/cursorCodec.ts (1)
encodeCursor(44-45)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build (22.x)
- GitHub Check: build (24.x)
🔇 Additional comments (5)
packages/app/api-common/src/apiSchemas.spec.ts (5)
1-17: LGTM!The imports and test setup correctly reflect the API surface rename from
multiCursor*toencodedCursor*, and the schema update fromz.uuid()toz.guid()is consistent with the expected error message changes later in the file.
37-48: LGTM!Good coverage for the new numeric cursor support. The test properly validates the round-trip encoding and decoding of numeric cursors.
114-140: LGTM!The error message expectations are correctly updated to reflect the GUID format validation. The inline snapshot accurately captures the expected error structure.
142-163: LGTM!Excellent negative test case for numeric cursor validation. This properly ensures that cursor schema constraints (like
.positive()) are enforced during parsing.
166-181: LGTM!The schema rename is consistent with the API surface changes, and the test correctly validates that
limitremains optional in theencodedCursorOptionalPaginationSchema.
Comment @coderabbitai help to get the list of available commands and usage tips.
CarlosGamero
changed the title
CarlosGamero
requested review from
a team,
dariacm and
kibertoad
as code owners
| }, | ||
| "dependencies": { | ||
| "@lokalise/universal-ts-utils": "^4.2.3" |
There was a problem hiding this comment.
No longer needed
| code: z.ZodIssueCode.custom, | ||
| params: { message: result.error.message }, | ||
| code: 'custom', | ||
| params: { message: result.error?.message }, |
There was a problem hiding this comment.
z.ZodIssueCode is deprecated
| export const zMeta = z.object({ | ||
| count: z.number(), | ||
| cursor: z.string().optional().describe('Pagination cursor, a last item id from this result set'), | ||
| hasMore: z.boolean().optional().describe('Whether there are more items to fetch'), |
There was a problem hiding this comment.
Only a couple of endpoints are using meta without hasMore, adding it there should be pretty simple.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @packages/app/api-common/src/cursorCodec.ts:
- Around line 66-67: The type guard isObjectOrNumber incorrectly allows null
because typeof null === 'object'; update its implementation to explicitly
exclude null (e.g., check value !== null) so the predicate only returns true for
non-null objects or numbers; modify the function isObjectOrNumber to test value
!== null && (typeof value === 'object' || typeof value === 'number') so the type
narrowing is correct wherever the guard is used.
In @packages/app/api-common/src/paginationUtils.ts:
- Around line 42-48: The current branch handling a single cursor key unsafely
casts the extracted value to number; update the logic in paginationUtils.ts to
validate the runtime type of value (from lastElement[cursorKeys[0]]) before
encoding: if typeof value === 'string' use it as-is, else if typeof value ===
'number' call encodeCursor(value), otherwise fall back to the multi-key behavior
(encodeCursor(pick(lastElement, cursorKeys))) or throw/handle a clear error for
unsupported types; ensure you use the existing encodeCursor, cursorKeys,
lastElement and pick symbols and remove the unsafe cast.
🧹 Nitpick comments (2)
packages/app/api-common/src/cursorCodec.spec.ts (1)
23-26: Consider adding edge case tests for numeric cursors.The test covers the basic case (42), but numeric cursors may include edge cases like
0, negative numbers, and floating-point values. Consider adding tests for these scenarios to ensure robustness.💡 Suggested additional test cases
it('encode and decode works for numbers', () => { const testValue = 42 expect(decodeCursor(encodeCursor(testValue))).toEqual({ result: testValue }) }) + + it('encode and decode works for zero', () => { + expect(decodeCursor(encodeCursor(0))).toEqual({ result: 0 }) + }) + + it('encode and decode works for negative numbers', () => { + expect(decodeCursor(encodeCursor(-100))).toEqual({ result: -100 }) + }) + + it('encode and decode works for floating-point numbers', () => { + expect(decodeCursor(encodeCursor(3.14))).toEqual({ result: 3.14 }) + })packages/app/api-common/src/paginationUtils.ts (1)
4-20: Consider using the sharedpickutility.A
pickfunction exists inpackages/app/universal-ts-utils/src/public/object/pick.ts. Unless there's a specific reason to avoid the dependency (e.g., bundle size), consider reusing it to reduce code duplication.
📜 Review details
Configuration used: Repository: lokalise/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
packages/app/api-common/package.jsonpackages/app/api-common/src/apiSchemas.spec.tspackages/app/api-common/src/apiSchemas.tspackages/app/api-common/src/cursorCodec.spec.tspackages/app/api-common/src/cursorCodec.tspackages/app/api-common/src/paginationUtils.spec.tspackages/app/api-common/src/paginationUtils.ts
💤 Files with no reviewable changes (1)
- packages/app/api-common/package.json
🧰 Additional context used
🧬 Code graph analysis (4)
packages/app/api-common/src/cursorCodec.spec.ts (2)
packages/app/api-common/src/cursorCodec.ts (2)
decodeCursor(51-64)encodeCursor(44-45)packages/app/api-common/src/index.ts (2)
decodeCursor(2-2)encodeCursor(2-2)
packages/app/api-common/src/apiSchemas.spec.ts (2)
packages/app/api-common/src/apiSchemas.ts (2)
encodedCursorMandatoryPaginationSchema(36-47)encodedCursorOptionalPaginationSchema(48-52)packages/app/api-common/src/cursorCodec.ts (1)
encodeCursor(44-45)
packages/app/api-common/src/paginationUtils.ts (3)
packages/app/api-common/src/apiSchemas.ts (2)
PaginationMeta(60-60)PaginatedResponse(67-70)packages/app/api-common/src/cursorCodec.ts (1)
encodeCursor(44-45)packages/app/universal-ts-utils/src/public/object/pick.ts (1)
pick(42-58)
packages/app/api-common/src/apiSchemas.ts (2)
packages/app/api-common/src/cursorCodec.ts (1)
decodeCursor(51-64)packages/app/api-common/src/index.ts (1)
decodeCursor(2-2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build (22.x)
- GitHub Check: build (24.x)
🔇 Additional comments (13)
packages/app/api-common/src/cursorCodec.spec.ts (1)
12-21: LGTM!Test description appropriately updated to clarify the test is for object cursors.
packages/app/api-common/src/apiSchemas.spec.ts (3)
37-48: LGTM!Good coverage for numeric cursor decoding. The test validates both the encoding and proper type transformation through the schema.
142-163: LGTM!Excellent test for validating schema constraints on numeric cursors. This ensures that encoded numbers still go through proper Zod validation.
4-5: LGTM!Test imports and references correctly updated to reflect the renamed
encodedCursor*schema APIs.Also applies to: 11-11, 19-20
packages/app/api-common/src/paginationUtils.spec.ts (2)
104-119: LGTM!This test properly validates that single numeric cursor keys are encoded using
encodeCursor, which aligns with the implementation inpaginationUtils.ts(lines 44-45).
46-51: LGTM!Good defensive test ensuring empty
cursorKeysarrays throw an appropriate error.packages/app/api-common/src/cursorCodec.ts (2)
44-45: LGTM!Clean extension of
encodeCursorto support numeric values while maintaining backward compatibility with objects.
51-64: LGTM!The
decodeCursorfunction correctly handles both object and number results with proper error handling.packages/app/api-common/src/apiSchemas.ts (3)
57-57: Breaking change:hasMoreis now required.This is a breaking change making
hasMoremandatory in the pagination meta. Ensure this is documented in the changelog for the major release.
22-34: LGTM!The
decodeCursorHookcorrectly handles decoding with proper error propagation through Zod's custom error mechanism.
36-47: Reconsider the generic constraint syntax for Zod v4.In Zod v4,
ZodSchemahas two generic parameters:OutputandInput. The constraintCursorType extends z.ZodSchema<unknown, Record<string, unknown> | number | undefined>sets the Output tounknownand constrains the Input type. This defeats type inference for the final output of thecursorfield, which will always be typed asunknownregardless of whatcursorTypeproduces. The constraint appears inverted—you likely want to constrain the Output type instead to ensure proper type safety downstream.packages/app/api-common/src/paginationUtils.ts (2)
57-72: LGTM!Excellent JSDoc documentation clearly explaining the cursor formation behavior for different scenarios (no keys, single string key, single number key, multiple keys).
113-131: LGTM!
getPaginatedEntriesByHasMorecorrectly leverages the now-requiredhasMorefield for pagination control.
CarlosGamero
changed the title
Changes
Adds support for numeric cursors in pagination utilities and, since this is a major release, removes deprecated functionality.
Checklist
major,minor,patchorskip-releaseSummary by CodeRabbit
New Features
Refactor
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.