feat(query-graphql): add offset field to cursor pagination#452
Open
jtomaszewski wants to merge 1 commit into
Open
feat(query-graphql): add offset field to cursor pagination#452jtomaszewski wants to merge 1 commit into
jtomaszewski wants to merge 1 commit into
Conversation
Allow combining offset with cursor pagination fields (first/after, last/before) for additional skipping within cursor-based pagination. - Add offset field to CursorPagingType interface - Add offset GraphQL field with validation (@ISINT, @min(0)) - Update limit-offset pager strategy to add offset to cursor position Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR adds support for combining an Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 Tip Migrating from UI to YAML configuration.Use the |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run-many --target=lint --all |
❌ Failed | 15s | View ↗ |
nx run-many --target=e2e --all |
✅ Succeeded | 1m 53s | View ↗ |
nx run-many --target=test --all |
✅ Succeeded | 42s | View ↗ |
nx run-many --target=build --all |
✅ Succeeded | 1s | View ↗ |
nx run workspace:version |
✅ Succeeded | <1s | View ↗ |
☁️ Nx Cloud last updated this comment at 2026-03-21 21:21:27 UTC
@ptc-org/nestjs-query-core
@ptc-org/nestjs-query-graphql
@ptc-org/nestjs-query-mikro-orm
@ptc-org/nestjs-query-mongoose
@ptc-org/nestjs-query-sequelize
@ptc-org/nestjs-query-typegoose
@ptc-org/nestjs-query-typeorm
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
documentation/docs/graphql/queries/paging.mdx (1)
389-407: Good documentation addition for the new offset feature.The example and explanation clearly demonstrate the primary use case of combining
offsetwith cursor-based pagination.Consider expanding the documentation to cover:
- Using
offsetwith justfirst(withoutafter) - e.g.,{ first: 2, offset: 5 }to skip the first 5 records- Clarifying whether
offsetworks with backward paging (last/before) and what behavior to expect- A brief note on validation (non-negative integers only)
This would provide users with a complete picture of the feature's capabilities.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@documentation/docs/graphql/queries/paging.mdx` around lines 389 - 407, Update the paging docs for the new offset feature by adding (1) an example using offset with only forward paging (e.g., todoItems(paging: { first: 2, offset: 5 })) and a short sentence explaining it skips the first N records then returns the next first items, (2) a clarification about backward paging behavior with last/before (whether offset is applied after moving to the before cursor or applied before backward slicing — document the chosen semantics for todoItems' paging args: last, before, offset), and (3) a brief validation note stating offset accepts only non-negative integers and how invalid values are handled; reference the todoItems(paging: { first/last, after/before, offset }) argument names so the maintainer can place these additions near the existing cursor+offset example.packages/query-graphql/__tests__/types/query/cursor-query-args.type.spec.ts (1)
110-144: Good test coverage for the newoffsetvalidation scenarios.The tests appropriately verify:
first + offsetcombination passes validationfirst + after + offsetcombination passes validation- Negative
offsetvalues are rejected with a validation errorThe test patterns align with existing tests in this file.
Consider adding test coverage for backward paging with offset (
last + before + offset) to verify validation behavior is consistent across both paging directions. This would match the implementation which allowsoffsetwith backward paging.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/query-graphql/__tests__/types/query/cursor-query-args.type.spec.ts` around lines 110 - 144, Add a unit test that mirrors the forward-paging cases but for backward paging: create a TestCursorQuery instance with paging { last: <number>, before: '<cursor>', offset: <number> } (use plainToClass(TestCursorQuery, ...)) and assert validateSync(queryInstance) returns an empty array; also add a case that uses last+offset without before if desired. Target the paging.offset validation paths by referencing TestCursorQuery, paging, offset, last, before, plainToClass and validateSync so the behavior allowing offset with backward paging is covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@documentation/docs/graphql/queries/paging.mdx`:
- Around line 389-407: Update the paging docs for the new offset feature by
adding (1) an example using offset with only forward paging (e.g.,
todoItems(paging: { first: 2, offset: 5 })) and a short sentence explaining it
skips the first N records then returns the next first items, (2) a clarification
about backward paging behavior with last/before (whether offset is applied after
moving to the before cursor or applied before backward slicing — document the
chosen semantics for todoItems' paging args: last, before, offset), and (3) a
brief validation note stating offset accepts only non-negative integers and how
invalid values are handled; reference the todoItems(paging: { first/last,
after/before, offset }) argument names so the maintainer can place these
additions near the existing cursor+offset example.
In `@packages/query-graphql/__tests__/types/query/cursor-query-args.type.spec.ts`:
- Around line 110-144: Add a unit test that mirrors the forward-paging cases but
for backward paging: create a TestCursorQuery instance with paging { last:
<number>, before: '<cursor>', offset: <number> } (use
plainToClass(TestCursorQuery, ...)) and assert validateSync(queryInstance)
returns an empty array; also add a case that uses last+offset without before if
desired. Target the paging.offset validation paths by referencing
TestCursorQuery, paging, offset, last, before, plainToClass and validateSync so
the behavior allowing offset with backward paging is covered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a9368cc8-d3f7-4cd2-8305-293dc2c7fa29
⛔ Files ignored due to path filters (10)
packages/query-graphql/__tests__/resolvers/__snapshots__/read.resolver.spec.ts.snapis excluded by!**/*.snappackages/query-graphql/__tests__/resolvers/federation/__snapshots__/federation.resolver.spec.ts.snapis excluded by!**/*.snappackages/query-graphql/__tests__/resolvers/relations/read-relation/many/__snapshots__/1.spec.ts.snapis excluded by!**/*.snappackages/query-graphql/__tests__/resolvers/relations/read-relation/many/__snapshots__/2.spec.ts.snapis excluded by!**/*.snappackages/query-graphql/__tests__/resolvers/relations/read-relation/many/__snapshots__/3.spec.ts.snapis excluded by!**/*.snappackages/query-graphql/__tests__/resolvers/relations/read-relation/many/__snapshots__/4.spec.ts.snapis excluded by!**/*.snappackages/query-graphql/__tests__/resolvers/relations/read-relation/many/__snapshots__/5.spec.ts.snapis excluded by!**/*.snappackages/query-graphql/__tests__/types/query/__snapshots__/cursor-query-args-with-decorator.spec.ts.snapis excluded by!**/*.snappackages/query-graphql/__tests__/types/query/__snapshots__/cursor-query-args.type.spec.ts.snapis excluded by!**/*.snappackages/query-graphql/__tests__/types/query/__snapshots__/paging.type.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (6)
documentation/docs/graphql/queries/paging.mdxpackages/query-graphql/__tests__/types/connection/cursor-connection.type.spec.tspackages/query-graphql/__tests__/types/query/cursor-query-args.type.spec.tspackages/query-graphql/src/types/connection/cursor/pager/strategies/limit-offset.pager-strategy.tspackages/query-graphql/src/types/query/paging/cursor-paging.type.tspackages/query-graphql/src/types/query/paging/interfaces.ts
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.
Currently a collection can either have cursor-based or offset-based pagination. What if I liked both, depending on the client usage? (e.g. offset in superadmin apps; cursor in consumer apps)
This allows for this.
Summary
offsetfield to cursor pagination, allowing it to be combined with cursor fields (first/after,last/before) for additional skippingChanges
offsetfield toCursorPagingTypeinterfaceoffsetGraphQL field with@IsIntand@Min(0)validationLimitOffsetPagerStrategy.getOffset()to include additional offsetExample Usage
Test plan
first + offsetandfirst + after + offsetcombinations🤖 Generated with Claude Code