chore(shared): Improve cache key creation #7217
Conversation
🦋 Changeset detectedLatest commit: 6bc27c5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 22 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughHook pagination APIs were refactored to accept a single params object ({ fetcher, config, keys }); a generic Changes
Sequence Diagram(s)sequenceDiagram
participant Consumer as Hook Consumer
participant Hook as usePagesOrInfinite
participant Keys as createCacheKeys / keys
participant Query as Query Layer (RQ/SWR)
participant Fetcher as Fetcher
rect `#E8F5E9`
Note over Consumer,Hook: Single-object call shape
Consumer->>Hook: call({ fetcher, config, keys })
Hook->>Keys: read keys.queryKey (stablePrefix, authenticated, tracked, args)
Hook->>Query: build query key from keys.queryKey (append "-inf" if config.infinite)
Query->>Fetcher: invoke(fetcher(args + pagination from config))
Fetcher-->>Query: return paged data
Query-->>Hook: return PaginatedResources
Hook-->>Consumer: return resources/state
end
rect `#FFF3E0`
Note over Hook,Query: Sign-out invalidation uses auth flag in key
Hook->>Query: invalidate queries where key[1] == true and key[0] matches stablePrefix (or "-inf")
Query-->>Hook: invalidated
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
# Conflicts: # packages/shared/src/react/hooks/createBillingPaginatedHook.tsx
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@clerk/upgrade
@clerk/vue
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/shared/src/react/hooks/usePagesOrInfinite.swr.tsx (1)
88-106: Consider improving type safety around fetcher parameter narrowing.The
@ts-ignorecomments on lines 95-96 bypass type checking when calling the fetcher with the result ofgetDifferentKeys. While the runtime logic appears correct (stripping tracked keys before passing to fetcher), this creates a type safety gap.Consider one of these approaches:
- Use a type assertion with explanation instead of
@ts-ignore- Refactor
getDifferentKeysto return a type that TypeScript can verify as compatible withParams- Add a generic constraint to ensure the transformation is type-safe
Example with type assertion:
- // @ts-ignore - fetcher expects Params subset; narrowing at call-site - return fetcher(requestParams); + return fetcher(requestParams as Params);packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx (1)
68-78: Remove outdated comment.The comment on lines 75-76 references
getDifferentKeysand questions the need for parameter transformation, but the code now directly usesargsfrom the query key. This comment is outdated and should be removed.if (!fetcher) { return undefined as any; } - // Why do we need this ? can we just specify which `args` to use in the key ? - // const requestParams = getDifferentKeys(key, cacheKeys); return fetcher(args);packages/shared/src/react/hooks/usePageOrInfinite.types.ts (1)
7-39: Remove commented-out code blocks.The commented-out code from the previous API signature can be removed. Git history preserves the old implementation if needed for reference. Keeping large blocks of commented code can confuse future developers and creates maintenance burden.
-// export type UsePagesOrInfiniteSignature = < -// Params extends PagesOrInfiniteOptions, -// ... -// ) => PaginatedResources<ExtractData<FetcherReturnData>, TConfig['infinite']>; - type Config = PagesOrInfiniteConfig & PagesOrInfiniteOptions;And:
export type UsePagesOrInfiniteSignature = < Params, FetcherReturnData extends Record<string, any>, TCacheKeys extends { queryKey: QueryKeyWithArgs<Params>; invalidationKey: AnyQueryKey; stableKey: string; }, - // CacheKeys extends Record<string, unknown> = Record<string, unknown>, TConfig extends Config = Config, >( - // /** - // * The parameters will be passed to the fetcher. - // */ - // params: Params, - ... params: { fetcher: ((p: Params) => FetcherReturnData | Promise<FetcherReturnData>) | undefined; config: TConfig; keys: TCacheKeys; }, ) => PaginatedResources<ExtractData<FetcherReturnData>, TConfig['infinite']>;Also applies to: 82-95
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts(25 hunks)packages/shared/src/react/hooks/createBillingPaginatedHook.tsx(3 hunks)packages/shared/src/react/hooks/createCacheKeys.ts(1 hunks)packages/shared/src/react/hooks/useAPIKeys.ts(2 hunks)packages/shared/src/react/hooks/useOrganization.tsx(2 hunks)packages/shared/src/react/hooks/useOrganizationList.tsx(2 hunks)packages/shared/src/react/hooks/usePageOrInfinite.types.ts(1 hunks)packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx(6 hunks)packages/shared/src/react/hooks/usePagesOrInfinite.swr.tsx(4 hunks)packages/shared/src/react/hooks/useSubscription.rq.tsx(3 hunks)packages/shared/src/react/types.ts(0 hunks)
💤 Files with no reviewable changes (1)
- packages/shared/src/react/types.ts
🧰 Additional context used
🧬 Code graph analysis (8)
packages/shared/src/react/hooks/useAPIKeys.ts (4)
packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx (1)
usePagesOrInfinite(21-294)packages/shared/src/react/hooks/usePagesOrInfinite.swr.tsx (1)
usePagesOrInfinite(37-237)packages/shared/src/types/clerk.ts (1)
GetAPIKeysParams(1980-1983)packages/shared/src/react/hooks/createCacheKeys.ts (1)
createCacheKeys(4-23)
packages/shared/src/react/hooks/createBillingPaginatedHook.tsx (2)
packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx (1)
usePagesOrInfinite(21-294)packages/shared/src/react/hooks/createCacheKeys.ts (1)
createCacheKeys(4-23)
packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx (4)
packages/shared/src/react/hooks/usePagesOrInfinite.swr.tsx (1)
usePagesOrInfinite(37-237)packages/shared/src/react/hooks/usePageOrInfinite.types.ts (1)
UsePagesOrInfiniteSignature(71-101)packages/shared/src/react/clerk-rq/useQuery.ts (1)
useClerkQuery(40-42)packages/shared/src/react/clerk-rq/useInfiniteQuery.ts (1)
useClerkInfiniteQuery(42-44)
packages/shared/src/react/hooks/useOrganization.tsx (2)
packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx (1)
usePagesOrInfinite(21-294)packages/shared/src/react/hooks/createCacheKeys.ts (1)
createCacheKeys(4-23)
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts (2)
packages/shared/src/react/hooks/createCacheKeys.ts (1)
createCacheKeys(4-23)packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx (1)
usePagesOrInfinite(21-294)
packages/shared/src/react/hooks/usePageOrInfinite.types.ts (1)
packages/shared/src/react/types.ts (2)
PagesOrInfiniteConfig(104-128)PagesOrInfiniteOptions(133-146)
packages/shared/src/react/hooks/usePagesOrInfinite.swr.tsx (3)
packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx (1)
usePagesOrInfinite(21-294)packages/shared/src/react/hooks/usePageOrInfinite.types.ts (1)
UsePagesOrInfiniteSignature(71-101)packages/shared/src/react/hooks/usePagesOrInfinite.shared.ts (1)
getDifferentKeys(75-89)
packages/shared/src/react/hooks/useOrganizationList.tsx (3)
packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx (1)
usePagesOrInfinite(21-294)packages/shared/src/react/hooks/usePagesOrInfinite.swr.tsx (1)
usePagesOrInfinite(37-237)packages/shared/src/react/hooks/createCacheKeys.ts (1)
createCacheKeys(4-23)
⏰ 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). (4)
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (13)
packages/shared/src/react/hooks/useSubscription.rq.tsx (3)
24-35: LGTM: Clean separation of query and invalidation keys.The
subscriptionQueryhelper effectively separatesqueryKeyfrominvalidationKey, enabling broader cache invalidation (invalidating all queries with matchingtrackedKeysregardless ofuntrackedKeys). The hardcoded'commerce-subscription'stableKey is appropriate given the function's internal, subscription-specific purpose.Note: When
untrackedKeysis omitted, thequeryKeywill be[stableKey, trackedKeys, undefined], which React Query treats distinctly from a two-element array. This appears intentional to reserve space for future untracked parameters.
63-70: LGTM: Correct usage of the new helper.The refactor properly leverages
subscriptionQueryto generate keys, and theuseMemodependencies correctly track all values used intrackedKeys. The absence ofuntrackedKeysis appropriate for the current use case and can be added later if needed.
85-88: LGTM: Improved invalidation strategy.Switching from
queryKeytoinvalidationKeyis the core improvement—it enables broader cache invalidation by matching on[stableKey, trackedKeys]only, ensuring all related queries are refreshed regardless of theiruntrackedKeys. This is a more flexible and maintainable approach.packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts (2)
16-53: LGTM! Well-structured test helpers.The test scaffolding helpers effectively reduce boilerplate and standardize the test setup for the new object-based API. The
buildConfigandbuildKeysabstractions are clean and therenderUsePagesOrInfinitewrapper provides a consistent rendering interface.
71-860: Test migration looks comprehensive.All test cases have been consistently updated to use the new object-based API through the helper functions. The test coverage remains thorough, covering pagination, infinite mode, caching, error handling, and edge cases.
packages/shared/src/react/hooks/usePagesOrInfinite.swr.tsx (2)
37-57: Signature refactor correctly implemented.The update to accept a single params object and source
initialPage/pageSizefromconfiginstead ofparamsis consistent with the PR's goal of standardizing the API. Cache key construction correctly destructures thekeys.queryKeyarray.
124-153: Infinite mode refactor aligns with pagination mode.The infinite mode updates correctly mirror the pagination mode changes, using
keys.queryKeyelements for cache key construction. The same type safety consideration from the previous comment applies to lines 147-150.packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx (4)
11-11: Good simplification by removing getDifferentKeys.The React Query implementation no longer needs
getDifferentKeyssince it can accessargsdirectly from the query key structure. This is cleaner than the transformation approach.
21-28: Signature refactor correctly implemented.The update to accept a single params object and destructure
fetcher,config, andkeysis consistent with the SWR implementation and the broader PR refactor.
115-136: Verify the scope of the sign-out query clear predicate.The predicate on line 126 clears all queries where
key[2] === true(authenticated queries). This is broader than the previous implementation which matched specific query key patterns. If other authenticated queries exist in the application (e.g., user profile, organization data), they will also be cleared on sign-out.Please verify this is the intended behavior. If the goal is to clear only pagination-related queries on sign-out, consider a more specific predicate:
queryClient.removeQueries({ predicate: query => { const key = query.queryKey; // Clear only pagination queries that are authenticated return Array.isArray(key) && key[2] === true && (typeof key[0] === 'string' && key[0].includes('your-pagination-prefix')); }, });Alternatively, if clearing all authenticated queries on sign-out is intentional, consider adding a comment explaining this behavior.
86-109: Infinite query refactor correctly implemented.The infinite query updates are consistent with the single-page query changes, using
keys.queryKeyelements for construction and directly accessingargsfrom the query key in thequeryFn.packages/shared/src/react/hooks/usePageOrInfinite.types.ts (2)
40-69: New type definitions support the refactored signature.The internal type definitions (
Config,Register,AnyQueryKey,QueryArgs,QueryKeyWithArgs) correctly support the new object-based API. TheQueryKeyWithArgstype properly defines the expected structure of cache keys as[string, boolean, Record<string, unknown>, QueryArgs<Params>, ...Array<unknown>], which aligns with the output ofcreateCacheKeys.
71-101: Signature successfully refactored to object-based API.The updated
UsePagesOrInfiniteSignaturecorrectly defines the new single-parameter API whereparamscontainsfetcher,config, andkeys. The generic constraints properly enforce thatTCacheKeysincludesqueryKey,invalidationKey, andstableKeywith the expected shapes.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts(25 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts (1)
packages/shared/src/react/hooks/createCacheKeys.ts (1)
createCacheKeys(4-23)
⏰ 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). (3)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (3)
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts (3)
5-5: LGTM! Clean test helper utilities.The new test helpers (
ConfigOverrides,buildConfig,buildKeys,renderUsePagesOrInfinite) effectively reduce duplication and align with the refactored object-based API. The use of nullish coalescing inbuildConfigcorrectly prioritizes overrides, then params, then defaults.Also applies to: 16-53
71-71: LGTM! Comment accurately reflects query client usage.The updated wording correctly references the query client, aligning with the refactored implementation.
80-84: LGTM! Consistent test refactoring.The tests have been successfully refactored to use the new helper utilities while preserving all original assertions. The object-based API (
{ fetcher, config, keys }) is applied consistently across all test cases.Also applies to: 132-136, 174-178, 229-233, 275-279, 310-314, 343-347, 376-380, 406-410, 441-445, 482-486, 520-524, 563-567, 606-610, 663-667, 677-681, 755-759, 788-792, 819-823
| const params = { initialPage: 1, pageSize: 3 }; | ||
| const config = buildConfig(params, { isSignedIn: false }); | ||
| const keys = buildKeys('t-signedin-false', params); |
There was a problem hiding this comment.
Consider aligning authenticated flag with isSignedIn config.
In these tests, the config specifies isSignedIn: false but buildKeys defaults authenticated to true. While this doesn't break the tests (since the query is disabled), it creates a semantic mismatch—the cache keys should reflect the authentication state for consistency and clarity.
Apply this diff to align the authenticated flag:
const params = { initialPage: 1, pageSize: 3 };
const config = buildConfig(params, { isSignedIn: false });
- const keys = buildKeys('t-signedin-false', params);
+ const keys = buildKeys('t-signedin-false', params, {}, false);And similarly for line 261:
const params = { initialPage: 1, pageSize: 3 };
const config = buildConfig(params, { infinite: true, isSignedIn: false });
- const keys = buildKeys('t-signedin-false-inf', params);
+ const keys = buildKeys('t-signedin-false-inf', params, {}, false);Also applies to: 259-261
🤖 Prompt for AI Agents
In packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts around
lines 245-247 (and similarly 259-261), the test builds a config with isSignedIn:
false but calls buildKeys which defaults authenticated to true, causing a
semantic mismatch; update the buildKeys calls in these line ranges to pass
authenticated: false (or derive authenticated from the config) so the generated
cache keys reflect the authentication state consistently with isSignedIn.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx (2)
75-77: Remove or address the TODO comment.The comment on lines 75-76 contains an unresolved question about the implementation. Either remove it if the current approach is confirmed correct, or open an issue to track the investigation.
86-90: Consider using a more structured approach for the infinite key suffix.The string concatenation
stablePrefix + '-inf'works but assumesstablePrefixis a string and doesn't prevent duplicate suffixes. Consider whether the key structure should enforce this at the type level or use a more explicit infinite flag in the key tuple.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx (4)
packages/shared/src/react/hooks/usePagesOrInfinite.swr.tsx (1)
usePagesOrInfinite(37-237)packages/shared/src/react/hooks/usePageOrInfinite.types.ts (1)
UsePagesOrInfiniteSignature(71-101)packages/shared/src/react/clerk-rq/useQuery.ts (1)
useClerkQuery(40-42)packages/shared/src/react/clerk-rq/useInfiniteQuery.ts (1)
useClerkInfiniteQuery(42-44)
⏰ 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). (5)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Build Packages
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx (5)
21-22: LGTM: Clean refactor to object-based API.The signature change from positional arguments to a single params object improves clarity and aligns with the type definition in
usePageOrInfinite.types.ts.
24-28: LGTM: Correct config usage.Properly sources
initialPageandpageSizefrom theconfigobject consistent with the new API structure.
92-109: LGTM: Consistent infinite query implementation.Properly uses
configvalues for pagination parameters and correctly spreadsargsfrom the queryKey when calling the fetcher.
220-220: LGTM: Simplified error handling.Removing the type cast simplifies the code while maintaining correctness.
48-64: No type safety concern—destructuring is properly enforced by types.The
QueryKeyWithArgs<Params>type explicitly defines the first four elements as a readonly tuple with specific types:readonly [string, boolean, Record<string, unknown>, QueryArgs<Params>, ...Array<unknown>]. TypeScript enforces that these four positions exist and have the specified types, making the destructuringconst [stablePrefix, authenticated, tracked, untracked] = keys.queryKeytype-safe at compile time. No runtime checks are needed.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/shared/src/react/hooks/createBillingPaginatedHook.tsx (1)
39-48: Experimental__experimental_modeflag is wired consistently with behaviorThe new
__experimental_mode?: 'cache'option is documented to avoid triggering a request on mount and to read from cache, and it is correctly passed through tousePagesOrInfinitewhereconfig.__experimental_mode === 'cache'disables querying and keeps cache-only semantics. From this file’s perspective, the type and docs look consistent with the underlying behavior.Minor nit: if you touch this again, consider rephrasing the comment to “In
cachemode …” for slightly clearer wording, but this is non-blocking.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.changeset/lemon-facts-stare.md(1 hunks)packages/shared/src/react/hooks/createBillingPaginatedHook.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/shared/src/react/hooks/createBillingPaginatedHook.tsx (2)
packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx (1)
usePagesOrInfinite(21-294)packages/shared/src/react/hooks/createCacheKeys.ts (1)
createCacheKeys(4-23)
⏰ 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). (5)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (2)
.changeset/lemon-facts-stare.md (1)
1-5: Changeset accurately reflects the scoped changeThe changeset targets
@clerk/sharedwith a patch and concisely describes the cache key updates for SWR/RQ hooks; this aligns with the implementation changes in the PR.packages/shared/src/react/hooks/createBillingPaginatedHook.tsx (1)
10-10: Cache key construction viacreateCacheKeysand newusePagesOrInfinitecall look correct; confirm args shapeThe new integration with
usePagesOrInfiniteandcreateCacheKeyslooks coherent:
fetcher: fetchFnmatches the expected signature, andenabled: isEnabledkeeps the previous gating (clerk loaded, billing enabled, externalenabledflag).config.isSignedInis only set whenoptions?.unauthenticatedis falsy, which aligns with the use of theauthenticateddimension for sign‑out‑driven invalidation.createCacheKeysis fed:
stablePrefix: resourceTypeto namespace billing resources,authenticated: !options?.unauthenticatedso billing queries are treated as authenticated by default and get cleared on sign‑out,trackedthat differentiates:
- unauthenticated usage by
{ for: safeFor },- authenticated usage by
{ userId, orgId/_orgId? }, with the computed org key respecting the__CLERK_USE_RQ__build flag,untracked: { args: hookParams as TParams }so the full argument payload participates in the key but is not part of the invalidation key.This matches the
usePagesOrInfiniteexpectations in the shared hook layer and should keep cache invalidation and scoping behavior consistent.One thing worth double‑checking (outside this file) is that
hookParamsindeed contains all request‑relevant parameters expected by the underlyinguseFetcherimplementation, not justinitialPage,pageSize, and optionalorgId. If additional filters were ever added to billing hooks, they should be included inhookParamsso they contribute both to the fetcher arguments and to the cache key.Also applies to: 124-148
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx (1)
237-244: Bug inhasNextPage/hasPreviousPagewheninitialPage > 1
offsetCountis already in “items” units:(initialPageRef.current - 1) * pageSizeRef.current. The current checks multiply it bypageSizeRef.currentagain:count - offsetCount * pageSizeRef.current > page * pageSizeRef.current; (page - 1) * pageSizeRef.current > offsetCount * pageSizeRef.current;This double-multiplication makes
hasNextPageandhasPreviousPageincorrect wheneverinitialPageRef.currentis not1(and generally breaks dimensional consistency).You can fix this by comparing against
offsetCountdirectly:const offsetCount = (initialPageRef.current - 1) * pageSizeRef.current; const pageCount = Math.ceil((count - offsetCount) / pageSizeRef.current); const hasNextPage = triggerInfinite ? Boolean(infiniteQuery.hasNextPage) - : count - offsetCount * pageSizeRef.current > page * pageSizeRef.current; + : count - offsetCount > page * pageSizeRef.current; const hasPreviousPage = triggerInfinite ? Boolean(infiniteQuery.hasPreviousPage) - : (page - 1) * pageSizeRef.current > offsetCount * pageSizeRef.current; + : (page - 1) * pageSizeRef.current > offsetCount;This preserves existing semantics for
initialPage = 1and fixes the logic for non‑defaultinitialPage.
🧹 Nitpick comments (2)
packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx (2)
11-12: Optional: simplifyuseWithSafeValuesre-export
useWithSafeValuesis only used for re-export. You can reduce boilerplate by exporting it directly from the import source:-import { useWithSafeValues } from './usePagesOrInfinite.shared'; -… -export { useWithSafeValues }; +export { useWithSafeValues } from './usePagesOrInfinite.shared';This avoids an extra import binding and keeps the module boundary clearer.
Also applies to: 294-294
112-134: Sign‑out cache purge is now properly scoped to this hookThe updated
removeQueriespredicate that destructures[stablePrefix, authenticated]fromquery.queryKeyand checks:
authenticated === true, andstablePrefixequals eitherkeys.queryKey[0]orkeys.queryKey[0] + '-inf'ensures only this hook’s authenticated pagination queries (both single and infinite) are cleared on sign-out. This addresses the previous over-broad invalidation and keeps unrelated authenticated queries intact.
If you want additional defensive safety against unexpected key shapes, you could optionally add an
Array.isArray(query.queryKey)guard before destructuring, but it’s not strictly necessary given the current usage.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/shared/src/react/hooks/createCacheKeys.ts(1 hunks)packages/shared/src/react/hooks/usePageOrInfinite.types.ts(1 hunks)packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/shared/src/react/hooks/createCacheKeys.ts
🧰 Additional context used
🧬 Code graph analysis (2)
packages/shared/src/react/hooks/usePageOrInfinite.types.ts (1)
packages/shared/src/react/types.ts (3)
PagesOrInfiniteConfig(104-128)PagesOrInfiniteOptions(133-146)PaginatedResources(13-79)
packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx (2)
packages/shared/src/react/hooks/usePagesOrInfinite.swr.tsx (1)
usePagesOrInfinite(37-237)packages/shared/src/react/hooks/usePageOrInfinite.types.ts (1)
UsePagesOrInfiniteSignature(38-51)
⏰ 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). (5)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (7)
packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx (4)
21-35: Params-object refactor and config defaults look consistentThe move to a single
paramsobject with{ fetcher, config, keys }plus derivingpaginatedPage,initialPageRef, andpageSizeReffromconfigis internally consistent and matches the shared signature. Usingenabled,__experimental_mode === 'cache', andisSignedIn !== falsetogether forqueriesEnabledis a reasonable gate for network work.
48-64: Cache key construction and single-page fetcher correctly align withQueryKeyWithArgsThe
pagesQueryKeyderivation
- Unpacks
[stablePrefix, authenticated, tracked, untracked]fromkeys.queryKey.- Rebuilds the fourth element with an updated
argsobject that injectsinitialPageandpageSizewhile preserving any existing params.- Uses
queryKey[3].argsinqueryFnto callfetcher(args).This matches the tuple layout enforced by
QueryKeyWithArgsand cleanly ties the current page/pageSize into the key without extragetDifferentKeysindirection.Also applies to: 69-76
84-107: Infinite query key and paging logic are coherent with the new key shapeUsing an
infiniteQueryKeyof[stablePrefix + '-inf', authenticated, tracked, untracked]cleanly separates infinite-mode cache entries from single-page ones while reusing the same base metadata. TheinitialPageParam,consumedcomputation, andqueryFnthat spreadsargsand overridesinitialPage/pageSizewith{ pageParam, pageSizeRef.current }are consistent with the intended pagination behavior and the shared args structure.
216-220: Error normalization is straightforwardNormalizing to
const error = (triggerInfinite ? infiniteQuery.error : singlePageQuery.error) ?? null;simplifies downstream checks and avoids casts while keeping the external contract (ClerkAPIResponseError | null) intact.packages/shared/src/react/hooks/usePageOrInfinite.types.ts (3)
7-25: Config alias andAnyQueryKeyscaffolding align with usageDefining
type Config = PagesOrInfiniteConfig & PagesOrInfiniteOptions;and deriving
AnyQueryKeyfrom a mergeableRegisterinterface gives you a single internal config shape and a flexible invalidation key type. WhenRegister.queryKeyis present,AnyQueryKeyrespects its array shape; otherwise it safely falls back toReadonlyArray<unknown>. This matches howinvalidationKeyis used in the updated signature.
26-36:QueryKeyWithArgstuple matches the runtime key layoutThe
QueryArgs<Params>wrapper andtype QueryKeyWithArgs<Params> = readonly [ string, boolean, Record<string, unknown>, QueryArgs<Params>, ...Array<unknown>, ];precisely describe the
[stablePrefix, authenticated, tracked, { args: Params }, …]structure consumed inusePagesOrInfinite.rq.tsx. This keeps query key construction and consumption type-safe while leaving room for additional suffix segments if needed.
38-51: UpdatedUsePagesOrInfiniteSignaturekeeps the public API typed and coherentThe new signature:
- Switches to a single
paramsobject with{ fetcher, config, keys }.- Ties
keys.queryKeytoQueryKeyWithArgs<Params>andinvalidationKeytoAnyQueryKey.- Uses
TConfig extends Configso thatTConfig['infinite']cleanly drives thePaginatedResources<…, Infinite>generic.This matches the refactored hook implementation and should give consumers better inference around both pagination mode and return types.
# Conflicts: # packages/shared/src/react/hooks/useAPIKeys.ts
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/shared/src/react/hooks/useAPIKeys.ts (1)
87-105: Consider clarifying the duplication ofinitialPageandpageSize.Both
initialPageandpageSizeappear in two places:
- In
hookParams(lines 88-89), which becomesuntracked.args- In
config(lines 103-104)Based on the
usePagesOrInfiniteimplementation, the values inuntracked.argsare overridden when constructing query keys—the hook usesconfig.initialPageandconfig.pageSizefor internal pagination state and replaces the values inargsbefore passing them to the fetcher.This duplication may be intentional to ensure initial alignment, but it could be clarified with a comment or refactored to avoid redundancy if the values in
hookParamsserve no purpose.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/shared/src/react/hooks/useAPIKeys.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/shared/src/react/hooks/useAPIKeys.ts (2)
packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx (1)
usePagesOrInfinite(21-292)packages/shared/src/react/hooks/createCacheKeys.ts (1)
createCacheKeys(4-21)
⏰ 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). (5)
- GitHub Check: Build Packages
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (2)
packages/shared/src/react/hooks/useAPIKeys.ts (2)
4-4: LGTM! Imports support the new cache key architecture.The addition of
GetAPIKeysParamsandcreateCacheKeysproperly supports the refactored cache key generation pattern.Also applies to: 7-7
106-116: LGTM! Cache key structure is well-designed.The
createCacheKeysusage properly separates concerns:
tracked: { subject }affects invalidation, appropriate for stable identifiersuntracked: { args: hookParams }includes transient parameters likequeryauthenticatedconsistently matchesisSignedIn(both usingBoolean(clerk.user))This structure ensures efficient cache invalidation while supporting query-specific cache entries.
Description
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit