-
Notifications
You must be signed in to change notification settings - Fork 40
feat(use-pagination-query): create useScopedPaginationQuery composable and pagination flag to useServiceQueryKey
#5897
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
Conversation
Signed-off-by: samuel.park <samuel.park@megazone.com>
…posable Signed-off-by: samuel.park <samuel.park@megazone.com>
…pagination query Signed-off-by: samuel.park <samuel.park@megazone.com>
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
|
🎉 @seungyeoneeee has been randomly selected as the reviewer! Please review. 🙏 |
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.
Pull Request Overview
Adds a new pagination-aware composable and enhances the existing query-key generator to support stable keys by stripping paging parameters.
- Introduces
useScopedPaginationQueryfor infinite, scope-validated pagination with automatic page fetching. - Extends
useServiceQueryKeywith apaginationflag and helper to omit page params from cache keys. - Provides
pagination-query-helperutilities to add or remove paging fields by API verb.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| apps/web/src/query/query-key/use-service-query-key.ts | Added pagination option and _omitPageParamsByVerb to strip page fields from query keys. |
| apps/web/src/query/pagination/use-scoped-pagination-query.ts | New wrapper around useScopedInfiniteQuery injecting paging logic and watching thisPage. |
| apps/web/src/query/pagination/pagination-query-helper.ts | New helpers omitPageFromLoadParams, omitPageQueryParams, and addPageToVerbParams. |
| apps/web/src/query/composables/use-scoped-infinite-query.ts | New scope-based infinite query hook enforcing grant-scope checks. |
Comments suppressed due to low confidence (3)
apps/web/src/query/pagination/pagination-query-helper.ts:10
- New helper functions lack dedicated unit tests; adding tests for
omitPageFromLoadParamsandomitPageQueryParamswill ensure correct behavior across edge cases.
export const omitPageFromLoadParams = <T extends LoadParams>(params: T): Omit<T, 'page'> => {
apps/web/src/query/composables/use-scoped-infinite-query.ts:1
- The new scoped infinite query hook would benefit from tests around scope validation and enabled-state logic to prevent regressions.
/**
apps/web/src/query/pagination/use-scoped-pagination-query.ts:12
- The imported
QueryKeyArraytype isn't used in this file; consider removing it to keep imports clean.
import type { QueryKeyArray } from '@/query/query-key/_types/query-key-type';
| const calls = Array.from({ length: val - currentLength }); | ||
| await Promise.all(calls.map(() => query.fetchNextPage())); |
Copilot
AI
May 29, 2025
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.
Concurrent calls to fetchNextPage may race and break page ordering. Consider fetching pages sequentially or chaining with for/reduce to preserve correct order.
| const calls = Array.from({ length: val - currentLength }); | |
| await Promise.all(calls.map(() => query.fetchNextPage())); | |
| for (let i = currentLength; i < val; i++) { | |
| await query.fetchNextPage(); | |
| } |
| const copiedQuery = params.query ? { ...params.query } : undefined; | ||
| if (copiedQuery) delete copiedQuery.page; | ||
|
|
Copilot
AI
May 29, 2025
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.
When params.query is undefined, this helper still adds a query: undefined field, altering object shape. Consider returning params unchanged in that case to preserve the original structure.
| const copiedQuery = params.query ? { ...params.query } : undefined; | |
| if (copiedQuery) delete copiedQuery.page; | |
| if (!params.query) { | |
| return params; | |
| } | |
| const copiedQuery = { ...params.query }; | |
| delete copiedQuery.page; |
Signed-off-by: samuel.park <samuel.park@megazone.com>
Skip Review (optional)
style,chore,ci,test,docs)Description (optional)
Introduce
useScopedPaginationQueryA wrapper around
useInfiniteQueryto handle paginated resource fetching with:load, list, analyze, stat)paginationflag inuseServiceQueryKeyMotivation
이번 PR에서는 다음 두 가지 주요 기능이 추가됩니다.
useScopedPaginationQueryuseInfiniteQuery를 감싼 pagination 전용 래퍼입니다.useServiceQueryKey에pagination옵션 추가도입 배경
Things to Talk About (optional)