-
Notifications
You must be signed in to change notification settings - Fork 397
chore(shared): Adds unit tests for usePagesOrInfinite
, createBillingPaginatedHook
and others
#6992
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
🦋 Changeset detectedLatest commit: bd0b347 The changes in this PR will be included in the next version bump. This PR includes changesets to release 19 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.
|
WalkthroughTracks prior sign-in state and gates SWR fetching in billing/pagination hooks, filters cache-only keys from request params, adds a primitive Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Component
participant Hook as usePagesOrInfinite
participant Prev as usePreviousValue
participant SWR as useSWR/Infinite
participant Fetcher
Component->>Hook: call hook(props)
Hook->>Prev: read previous isSignedIn
Hook->>Hook: compute shouldFetch & swrKey (uses previous/current isSignedIn)
alt swrKey == null (signed-out or !shouldFetch)
Hook-->>SWR: null key → no fetch
SWR-->>Component: idle (no network, data cleared/kept per config)
else
Hook->>SWR: provide per-page key
SWR->>Hook: request page params
Hook->>Hook: strip cache-only keys from params
SWR->>Fetcher: fetch(filteredParams)
Fetcher-->>SWR: data / error
SWR-->>Component: data, isLoading/isFetching
end
rect rgba(220,240,255,0.5)
note right of Hook: sign-in transitions trigger key recompute to avoid stale/unauthorized fetches
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
@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: |
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.
Actionable comments posted: 0
🧹 Nitpick comments (9)
packages/shared/package.json (1)
151-156
: Make coverage script cross‑platform; avoid chaining Jest and Vitest under a single "test" task
open
is macOS‑only; will fail on Linux/Windows/CI. Prefer a pure CLI target and an optional viewer script.- Running
jest && vitest
under"test"
can be slow and brittle; keep them separate and let CI/dev choose.Suggested adjustments:
- "test": "jest && vitest", + "test": "jest", "test:ci": "jest --maxWorkers=70%", - "test:coverage": "jest --collectCoverage && open coverage/lcov-report/index.html", + "test:coverage": "jest --coverage", + "test:coverage:view": "node -e \"const {execSync}=require('node:child_process'); try{execSync(process.platform==='darwin'?'open coverage/lcov-report/index.html':process.platform==='win32'?'start \"\" coverage/lcov-report/index.html':'xdg-open coverage/lcov-report/index.html',{stdio:'ignore'});}catch{}}\"", "test:vitest": "vitest"packages/shared/src/react/hooks/__tests__/useSafeValues.spec.ts (1)
18-23
: Removeany
in tests; usesatisfies
/inferred param types for safetyAvoid
as any
by inferring the options type from the hook signature.Example changes:
- const { result: r3, rerender } = renderHook( - ({ page }) => - useWithSafeValues({ initialPage: page, pageSize: 5, infinite: true, keepPreviousData: true }, defaults as any), - { initialProps: { page: 2 } }, - ); + type Defaults = Parameters<typeof useWithSafeValues>[1]; + const defaultsTyped: Defaults = defaults; + const { result: r3, rerender } = renderHook( + ({ page }) => + useWithSafeValues({ initialPage: page, pageSize: 5, infinite: true, keepPreviousData: true }, defaultsTyped), + { initialProps: { page: 2 } }, + ); - const { result } = renderHook(() => useWithSafeValues(user as any, defaults as any)); + const defaults2: Defaults = { initialPage: 1, pageSize: 10, infinite: false, keepPreviousData: false }; + const user = { initialPage: 2, pageSize: 20, infinite: true } as const satisfies Partial<Defaults>; + const { result } = renderHook(() => useWithSafeValues(user, defaults2)); - const { result } = renderHook(() => useWithSafeValues(user as any, defaults as any)); + const defaults3: Defaults = { initialPage: 1, pageSize: 10, infinite: false, keepPreviousData: true }; + const user2 = { pageSize: 50 } as const satisfies Partial<Defaults>; + const { result } = renderHook(() => useWithSafeValues(user2, defaults3));As per coding guidelines
Also applies to: 38-46, 52-60
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.ts (1)
62-71
: Isolate SWR cache to prevent cross‑test pollutionAdd an SWR wrapper with an in‑memory Map provider so each test runs against a fresh cache.
+ import React from 'react'; + import { SWRConfig } from 'swr'; import { createBillingPaginatedHook } from '../createBillingPaginatedHook'; + const wrapper = ({ children }: { children: React.ReactNode }) => ( + <SWRConfig value={{ provider: () => new Map() }}>{children}</SWRConfig> + );Then pass
{ wrapper }
torenderHook
calls:- const { result } = renderHook(() => useDummyAuth()); + const { result } = renderHook(() => useDummyAuth(), { wrapper });Repeat for other
renderHook
usages. This prevents flaky assertions due to shared SWR state.packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx (1)
111-116
: Fix minor typos in comments; consider waiting only for the settled state
- Typos: “Asser” -> “Assert”.
- Optional: Instead of first awaiting
isLoading === true
thenfalse
, you can await only the settledisLoading === false
to reduce timing flakiness.Example:
- // Asser that SWR will flip to fetching ... + // Assert that SWR will flip to fetching ...Also applies to: 135-142
packages/shared/src/react/hooks/__tests__/usePlans.spec.ts (1)
50-58
: Add SWR wrapper to avoid cache bleed across testsLike in the subscription tests, wrap renders with an SWR provider using a fresh Map:
+ import React from 'react'; + import { SWRConfig } from 'swr'; import { usePlans } from '../usePlans'; + const wrapper = ({ children }: { children: React.ReactNode }) => ( + <SWRConfig value={{ provider: () => new Map() }}>{children}</SWRConfig> + ); - const { result } = renderHook(() => usePlans({ initialPage: 1, pageSize: 5 })); + const { result } = renderHook(() => usePlans({ initialPage: 1, pageSize: 5 }), { wrapper });Repeat for the other
renderHook
calls to prevent flakiness.packages/shared/src/react/hooks/usePagesOrInfinite.ts (3)
84-84
: Consider proper typing instead of ts-ignore.The
@ts-ignore
suppresses the type error for dynamic key access. While the runtime safety is ensured by iterating over keys ofdefaultValues
, consider using a type-safe approach such as:newObj[key as keyof T] = shouldUseDefaults ? defaultValues[key] : (params?.[key] ?? defaultValues[key]);Or use a type assertion on the loop:
for (const key of Object.keys(defaultValues) as Array<keyof T>) { newObj[key] = shouldUseDefaults ? defaultValues[key] : (params?.[key] ?? defaultValues[key]); }As per coding guidelines
165-174
: Simplify redundant parameter merge.The merge
{ ...params, ...requestParams }
on Line 172 is redundant. SincegetDifferentKeys
extracts all keys fromcacheKeyParams
that are not incacheKeys
, andcacheKeyParams
includesparams
, the resultingrequestParams
already contains all keys fromparams
. Spreadingparams
first adds no additional keys.Additionally, there's an inconsistency: the pagination mode fetcher (Line 172) merges params, while the infinite mode fetcher (Line 209) only passes
requestParams
directly. Consider aligning both for consistency.Apply this diff to simplify the pagination fetcher:
- return fetcher({ ...params, ...requestParams }); + return fetcher(requestParams as Params);This makes the pagination and infinite modes consistent, as both now pass only
requestParams
.
206-210
: Address ts-ignore comments with proper typing.Two
@ts-ignore
comments suppress type errors:
- Line 206:
getDifferentKeys
call- Line 208:
fetcher
call withrequestParams
Consider replacing these with proper type assertions or narrowing. For example:
const requestParams = getDifferentKeys(cacheKeyParams, cacheKeys) as Partial<Params>; return fetcher?.(requestParams as Params);Alternatively, refine the
getDifferentKeys
return type to preserve the relationship between input and output types.As per coding guidelines
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts (1)
1-669
: LGTM: Comprehensive test coverage with room for type safety improvements.The test suite thoroughly covers all major features and edge cases:
- ✅ Pagination and infinite modes
- ✅ Parameter handling and cache key stripping
- ✅
isSignedIn
gating and sign-out scenarios- ✅
keepPreviousData
, cache mode, and disabled state- ✅ Revalidation behavior in both modes
- ✅ Error propagation and loading states
The tests correctly use
waitFor
for async operations and verify both data flow and state transitions.Optional improvement: Consider reducing the extensive use of
as any
type assertions (lines 26, 62, 82, 126, 163, 183, 201, 233, 270, 308, 346, 380, 412, 449, 492, 532, 577, 622, 681, 697). Per coding guidelines for**/__tests__/**/*.{ts,tsx}
, you could create type-safe test builders/factories:type TestParams = { initialPage: number; pageSize: number; [key: string]: unknown }; type TestConfig = { infinite: boolean; enabled?: boolean; isSignedIn?: boolean; [key: string]: unknown }; type TestCacheKeys = Record<string, unknown>; function createTestHook<T>( params: TestParams, fetcher: any, config: TestConfig, cacheKeys: TestCacheKeys ) { return renderHook(() => usePagesOrInfinite(params, fetcher, config, cacheKeys) ); }This would eliminate most
as any
casts while maintaining test readability.As per coding guidelines
📜 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 (7)
packages/shared/package.json
(1 hunks)packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.ts
(1 hunks)packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
(1 hunks)packages/shared/src/react/hooks/__tests__/usePlans.spec.ts
(1 hunks)packages/shared/src/react/hooks/__tests__/useSafeValues.spec.ts
(1 hunks)packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx
(1 hunks)packages/shared/src/react/hooks/usePagesOrInfinite.ts
(5 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}
: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.ts
packages/shared/src/react/hooks/__tests__/useSafeValues.spec.ts
packages/shared/src/react/hooks/__tests__/usePlans.spec.ts
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx
packages/shared/src/react/hooks/usePagesOrInfinite.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.ts
packages/shared/src/react/hooks/__tests__/useSafeValues.spec.ts
packages/shared/src/react/hooks/__tests__/usePlans.spec.ts
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx
packages/shared/package.json
packages/shared/src/react/hooks/usePagesOrInfinite.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.ts
packages/shared/src/react/hooks/__tests__/useSafeValues.spec.ts
packages/shared/src/react/hooks/__tests__/usePlans.spec.ts
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx
packages/shared/src/react/hooks/usePagesOrInfinite.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.ts
packages/shared/src/react/hooks/__tests__/useSafeValues.spec.ts
packages/shared/src/react/hooks/__tests__/usePlans.spec.ts
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx
packages/shared/src/react/hooks/usePagesOrInfinite.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}
: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidany
type - preferunknown
when type is uncertain, then narrow with type guards
Useinterface
for object shapes that might be extended
Usetype
for unions, primitives, and computed types
Preferreadonly
properties for immutable data structures
Useprivate
for internal implementation details
Useprotected
for inheritance hierarchies
Usepublic
explicitly for clarity in public APIs
Preferreadonly
for properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertions
for literal types:as const
Usesatisfies
operator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noany
types without justification
Proper error handling with typed errors
Consistent use ofreadonly
for immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.ts
packages/shared/src/react/hooks/__tests__/useSafeValues.spec.ts
packages/shared/src/react/hooks/__tests__/usePlans.spec.ts
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx
packages/shared/src/react/hooks/usePagesOrInfinite.ts
packages/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Unit tests should use Jest or Vitest as the test runner.
Files:
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.ts
packages/shared/src/react/hooks/__tests__/useSafeValues.spec.ts
packages/shared/src/react/hooks/__tests__/usePlans.spec.ts
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.ts
packages/shared/src/react/hooks/__tests__/useSafeValues.spec.ts
packages/shared/src/react/hooks/__tests__/usePlans.spec.ts
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx
packages/shared/src/react/hooks/usePagesOrInfinite.ts
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/__tests__/**/*.{ts,tsx}
: Create type-safe test builders/factories
Use branded types for test isolation
Implement proper mock types that match interfaces
Files:
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.ts
packages/shared/src/react/hooks/__tests__/useSafeValues.spec.ts
packages/shared/src/react/hooks/__tests__/usePlans.spec.ts
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{jsx,tsx}
: Use error boundaries in React components
Minimize re-renders in React components
**/*.{jsx,tsx}
: Always use functional components with hooks instead of class components
Follow PascalCase naming for components:UserProfile
,NavigationMenu
Keep components focused on a single responsibility - split large components
Limit component size to 150-200 lines; extract logic into custom hooks
Use composition over inheritance - prefer smaller, composable components
Export components as named exports for better tree-shaking
One component per file with matching filename and component name
Use useState for simple state management
Use useReducer for complex state logic
Implement proper state initialization
Use proper state updates with callbacks
Implement proper state cleanup
Use Context API for theme/authentication
Implement proper state selectors
Use proper state normalization
Implement proper state persistence
Use React.memo for expensive components
Implement proper useCallback for handlers
Use proper useMemo for expensive computations
Implement proper virtualization for lists
Use proper code splitting with React.lazy
Implement proper cleanup in useEffect
Use proper refs for DOM access
Implement proper event listener cleanup
Use proper abort controllers for fetch
Implement proper subscription cleanup
Use proper HTML elements
Implement proper ARIA attributes
Use proper heading hierarchy
Implement proper form labels
Use proper button types
Implement proper focus management
Use proper keyboard shortcuts
Implement proper tab order
Use proper skip links
Implement proper focus traps
Implement proper error boundaries
Use proper error logging
Implement proper error recovery
Use proper error messages
Implement proper error fallbacks
Use proper form validation
Implement proper error states
Use proper error messages
Implement proper form submission
Use proper form reset
Use proper component naming
Implement proper file naming
Use proper prop naming
Implement proper...
Files:
packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.tsx
: Use proper type definitions for props and state
Leverage TypeScript's type inference where possible
Use proper event types for handlers
Implement proper generic types for reusable components
Use proper type guards for conditional rendering
Files:
packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx
packages/*/package.json
📄 CodeRabbit inference engine (.cursor/rules/global.mdc)
All publishable packages should be placed under the packages/ directory
packages/*/package.json
: All publishable packages must be located in the 'packages/' directory.
All packages must be published under the @clerk namespace on npm.
Semantic versioning must be used across all packages.
Files:
packages/shared/package.json
🧬 Code graph analysis (3)
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.ts (2)
packages/types/src/resource.ts (1)
ClerkResource
(8-21)packages/shared/src/react/hooks/createBillingPaginatedHook.tsx (1)
createBillingPaginatedHook
(41-112)
packages/shared/src/react/hooks/__tests__/useSafeValues.spec.ts (1)
packages/shared/src/react/hooks/usePagesOrInfinite.ts (1)
useWithSafeValues
(73-93)
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts (2)
packages/shared/src/react/hooks/usePagesOrInfinite.ts (1)
usePagesOrInfinite
(140-298)packages/shared/src/utils/createDeferredPromise.ts (1)
createDeferredPromise
(11-19)
⏰ 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 (6)
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.ts (1)
73-86
: Clarify expectation:useFetcher
called eagerly, fetcher gated byenabled
Good assertions on eager
useFetcher
vs. gated fetcher. Consider also assertingresult.current.data
default when disabled to fully capture behavior (you already do for some cases).packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx (1)
19-27
: Nice use of SWRConfig to isolate cacheWrapper ensures per‑test SWR cache; reduces interference across tests. LGTM.
packages/shared/src/react/hooks/__tests__/usePlans.spec.ts (1)
73-84
: Good: verifies org mode doesn’t leak orgId to getPlansThe assertions match the expected contract of
usePlans
(forwardfor
, notorgId
). LGTM.packages/shared/src/react/hooks/usePagesOrInfinite.ts (3)
163-163
: LGTM: Proper gating for signed-out users.The check
isSignedIn === false
correctly prevents SWR from fetching when the user is signed out by setting the key tonull
. This aligns with the test coverage that verifies data is cleared on sign-out.
194-196
: LGTM: Proper gating in infinite mode.The guard
isSignedIn === false
correctly prevents infinite loading for signed-out users by returningnull
as the SWR key.
221-230
: LGTM: Correct dependency for fetchPage.Adding
triggerInfinite
to the dependency array ensuresfetchPage
recomputes when the infinite mode toggles, which is necessary since the callback behavior branches ontriggerInfinite
.
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.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/shared/src/react/hooks/__tests__/wrapper.tsx (1)
4-12
: Harden SWR test wrapper to avoid flakiness (disable retries/refetch, dedupe=0).Prevents surprise network retries/revalidations during tests.
export const wrapper = ({ children }: { children: React.ReactNode }) => ( <SWRConfig value={{ provider: () => new Map(), + revalidateOnFocus: false, + revalidateOnReconnect: false, + shouldRetryOnError: false, + dedupingInterval: 0, + errorRetryCount: 0, + errorRetryInterval: 0, }} > {children} </SWRConfig> );packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx (2)
36-37
: Make fetcher async to stabilize loading-state assertions.A sync mock (returning undefined) can make isLoading flip too fast and cause flakiness.
-const fetcherMock = vi.fn(); +const fetcherMock = vi.fn().mockResolvedValue({ data: [], total_count: 0 }); const useFetcherMock = vi.fn(() => fetcherMock);
108-118
: Avoid asserting transientisLoading === true
; assert on fetcher call or final state.
isLoading
can be very brief with fast/async mocks. Prefer waiting forfetcherMock
call and/or finalisLoading === false
.Example change:
- await waitFor(() => expect(result.current.isLoading).toBe(true)); + await waitFor(() => expect(fetcherMock).toHaveBeenCalled());Also applies to: 138-144, 150-157, 166-170, 172-183
packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx (1)
100-107
: Fix typos in comments (“Asser” → “Assert”).Keeps test narratives clear; current assertions are correct.
- // Asser that SWR will flip to fetching because the fetcherFN runs, but it forces `null` when userId is falsy. + // Assert that SWR flips to fetching because the fetcherFN runs, but it forces `null` when userId is falsy.Also applies to: 124-131
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts (1)
375-407
: Add hasNext/hasPrevious assertions for initialPage>1 to catch regressions.Strengthens coverage for offset scenarios; will surface boundary bugs.
await waitFor(() => expect(result.current.isLoading).toBe(false)); // offset = (3-1)*5 = 10; remaining = 37-10 = 27; pageCount = ceil(27/5) = 6 expect(result.current.pageCount).toBe(6); + // With initialPage=3 and totalCount=37, previous should exist; next should likely exist as well + expect(result.current.hasPreviousPage).toBe(true); + expect(result.current.hasNextPage).toBe(true); act(() => { result.current.fetchPrevious(); }); expect(result.current.page).toBe(2); + expect(result.current.hasPreviousPage).toBe(true); act(() => { result.current.fetchNext(); result.current.fetchNext(); }); expect(result.current.page).toBe(4); + expect(result.current.hasPreviousPage).toBe(true); + expect(result.current.hasNextPage).toBe(true);If these fail, we likely need to adjust the
hasNextPage
/hasPreviousPage
formulas in usePagesOrInfinite to account for the initialPage offset correctly. Based on learnings
📜 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 (5)
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx
(1 hunks)packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
(1 hunks)packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx
(1 hunks)packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx
(1 hunks)packages/shared/src/react/hooks/__tests__/wrapper.tsx
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}
: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx
packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx
packages/shared/src/react/hooks/__tests__/wrapper.tsx
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx
packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx
packages/shared/src/react/hooks/__tests__/wrapper.tsx
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx
packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx
packages/shared/src/react/hooks/__tests__/wrapper.tsx
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx
packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx
packages/shared/src/react/hooks/__tests__/wrapper.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}
: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidany
type - preferunknown
when type is uncertain, then narrow with type guards
Useinterface
for object shapes that might be extended
Usetype
for unions, primitives, and computed types
Preferreadonly
properties for immutable data structures
Useprivate
for internal implementation details
Useprotected
for inheritance hierarchies
Usepublic
explicitly for clarity in public APIs
Preferreadonly
for properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertions
for literal types:as const
Usesatisfies
operator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noany
types without justification
Proper error handling with typed errors
Consistent use ofreadonly
for immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx
packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx
packages/shared/src/react/hooks/__tests__/wrapper.tsx
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{jsx,tsx}
: Use error boundaries in React components
Minimize re-renders in React components
**/*.{jsx,tsx}
: Always use functional components with hooks instead of class components
Follow PascalCase naming for components:UserProfile
,NavigationMenu
Keep components focused on a single responsibility - split large components
Limit component size to 150-200 lines; extract logic into custom hooks
Use composition over inheritance - prefer smaller, composable components
Export components as named exports for better tree-shaking
One component per file with matching filename and component name
Use useState for simple state management
Use useReducer for complex state logic
Implement proper state initialization
Use proper state updates with callbacks
Implement proper state cleanup
Use Context API for theme/authentication
Implement proper state selectors
Use proper state normalization
Implement proper state persistence
Use React.memo for expensive components
Implement proper useCallback for handlers
Use proper useMemo for expensive computations
Implement proper virtualization for lists
Use proper code splitting with React.lazy
Implement proper cleanup in useEffect
Use proper refs for DOM access
Implement proper event listener cleanup
Use proper abort controllers for fetch
Implement proper subscription cleanup
Use proper HTML elements
Implement proper ARIA attributes
Use proper heading hierarchy
Implement proper form labels
Use proper button types
Implement proper focus management
Use proper keyboard shortcuts
Implement proper tab order
Use proper skip links
Implement proper focus traps
Implement proper error boundaries
Use proper error logging
Implement proper error recovery
Use proper error messages
Implement proper error fallbacks
Use proper form validation
Implement proper error states
Use proper error messages
Implement proper form submission
Use proper form reset
Use proper component naming
Implement proper file naming
Use proper prop naming
Implement proper...
Files:
packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx
packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx
packages/shared/src/react/hooks/__tests__/wrapper.tsx
packages/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Unit tests should use Jest or Vitest as the test runner.
Files:
packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx
packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx
packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx
packages/shared/src/react/hooks/__tests__/wrapper.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.tsx
: Use proper type definitions for props and state
Leverage TypeScript's type inference where possible
Use proper event types for handlers
Implement proper generic types for reusable components
Use proper type guards for conditional rendering
Files:
packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx
packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx
packages/shared/src/react/hooks/__tests__/wrapper.tsx
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/__tests__/**/*.{ts,tsx}
: Create type-safe test builders/factories
Use branded types for test isolation
Implement proper mock types that match interfaces
Files:
packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx
packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx
packages/shared/src/react/hooks/__tests__/wrapper.tsx
🧬 Code graph analysis (2)
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts (2)
packages/shared/src/react/hooks/usePagesOrInfinite.ts (1)
usePagesOrInfinite
(140-298)packages/shared/src/utils/createDeferredPromise.ts (1)
createDeferredPromise
(11-19)
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx (2)
packages/types/src/resource.ts (1)
ClerkResource
(8-21)packages/shared/src/react/hooks/createBillingPaginatedHook.tsx (1)
createBillingPaginatedHook
(41-112)
⏰ 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/__tests__/usePlans.spec.tsx (1)
88-101
: No changes needed Theunauthenticated: true
option inusePlans
already disables theisSignedIn
filter, so this test validly covers the unauthenticated scenario.packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx (1)
62-71
: Remove param-gating concern: default pagination is always applied
Destructuringparams || {}
makespaginationParams
an empty object, sohookParams
is neverundefined
and defaults ({initialPage:1,pageSize:10}
) are injected as expected.Likely an incorrect or invalid review comment.
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.
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)
.changeset/seven-turkeys-sin.md
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
.changeset/**
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Automated releases must use Changesets.
Files:
.changeset/seven-turkeys-sin.md
🪛 LanguageTool
.changeset/seven-turkeys-sin.md
[grammar] ~5-~5: There might be a mistake here.
Context: ... requests while the user was signed out.
(QB_NEW_EN)
⏰ 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
…d orgId changes as it returns unauthenticated data.
usePagesOrInfinite
, createBillingPaginatedHook
and othersusePagesOrInfinite
, createBillingPaginatedHook
and others
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.
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 (2)
packages/shared/src/react/hooks/usePagesOrInfinite.ts (2)
3-3
: Fix: usePrevious updates state during render and lacks explicit return typeCalling setState inside render is unsafe and can cause warnings/re-renders. Implement with useRef + useEffect and declare return type.
As per coding guidelines
-import { useCallback, useMemo, useRef, useState } from 'react'; +import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; @@ -/** - * - */ -export function usePrevious<T>(value: T) { - const [current, setCurrent] = useState(value); - const [previous, setPrevious] = useState<T | null>(null); - - if (value !== current) { - setPrevious(current); - setCurrent(value); - } - - return previous; -} +/** + * Returns the previous value of `value` across renders. + */ +export function usePrevious<T>(value: T): T | null { + const prevRef = useRef<T | null>(null); + useEffect(() => { + prevRef.current = value; + }, [value]); + return prevRef.current; +}Also applies to: 124-137
318-321
: Bug: hasNextPage/hasPreviousPage arithmetic is incorrect (double-multiplies offset)Using
offsetCount * pageSize
is wrong. This yields incorrect results when initialPage > 1.As per coding guidelines
- const hasNextPage = count - offsetCount * pageSizeRef.current > page * pageSizeRef.current; - const hasPreviousPage = (page - 1) * pageSizeRef.current > offsetCount * pageSizeRef.current; + const hasNextPage = page * pageSizeRef.current < count; + const hasPreviousPage = page > 1;
🧹 Nitpick comments (3)
packages/shared/src/react/hooks/usePagesOrInfinite.ts (1)
82-86
: Type-safety cleanups: remove ts-ignore; avoidany
in generics
- Replace
@ts-ignore
dynamic indexing with a safe merge to drop cache-only keys.- Tighten
FetcherReturnData
to avoidany
.As per coding guidelines
@@ - const newObj: Record<string, unknown> = {}; - for (const key of Object.keys(defaultValues)) { - // @ts-ignore - defaultValues and params share shape; dynamic index access is safe here - newObj[key] = shouldUseDefaults ? defaultValues[key] : (params?.[key] ?? defaultValues[key]); - } + const newObj = { + ...defaultValues, + ...(shouldUseDefaults || typeof params !== 'object' ? {} : params), + } as T; @@ - // @ts-ignore - remove cache-only keys from request params const requestParams = getDifferentKeys(cacheKeyParams, cacheKeys); - // @ts-ignore - fetcher expects Params subset; narrowing at call-site - return fetcher?.(requestParams); + return fetcher?.(requestParams as unknown as Params);And update the generic:
-type UsePagesOrInfinite = < - Params extends PagesOrInfiniteOptions, - FetcherReturnData extends Record<string, any>, +type UsePagesOrInfinite = < + Params extends PagesOrInfiniteOptions, + FetcherReturnData extends { data: unknown; total_count?: number },Also applies to: 261-265, 104-108
packages/shared/src/react/hooks/createBillingPaginatedHook.tsx (1)
77-85
: Simplify hookParams and isEnabled; remove dead check
typeof paginationParams === 'undefined'
is never true (always{}
). Also!!hookParams
is always true. Build params directly and computeisEnabled
from Clerk state.As per coding guidelines
- const hookParams = - typeof paginationParams === 'undefined' - ? undefined - : ({ - initialPage: safeValues.initialPage, - pageSize: safeValues.pageSize, - ...(options?.unauthenticated ? {} : _for === 'organization' ? { orgId: organization?.id } : {}), - } as TParams); + const hookParams = { + initialPage: safeValues.initialPage, + pageSize: safeValues.pageSize, + ...(options?.unauthenticated ? {} : _for === 'organization' ? { orgId: organization?.id } : {}), + } as TParams; @@ - const isEnabled = !!hookParams && clerk.loaded && !!billingEnabled; + const isEnabled = clerk.loaded && !!billingEnabled;Also applies to: 91-92
packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx (1)
44-49
: Reset mutated mock IDs between tests to avoid leakageTests set user/organization ids to undefined; restore them in beforeEach.
beforeEach(() => { vi.clearAllMocks(); mockClerk.loaded = true; mockClerk.__unstable__environment.commerceSettings.billing.user.enabled = true; mockClerk.__unstable__environment.commerceSettings.billing.organization.enabled = true; + // reset mutated ids + (mockUser as any).id = 'user_1'; + (mockOrganization as any).id = 'org_1'; });Also applies to: 88-89, 101-103
📜 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 (6)
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx
(1 hunks)packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
(1 hunks)packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx
(1 hunks)packages/shared/src/react/hooks/createBillingPaginatedHook.tsx
(2 hunks)packages/shared/src/react/hooks/usePagesOrInfinite.ts
(7 hunks)packages/shared/src/react/hooks/usePlans.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/shared/src/react/hooks/tests/createBillingPaginatedHook.spec.tsx
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}
: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/shared/src/react/hooks/createBillingPaginatedHook.tsx
packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
packages/shared/src/react/hooks/usePagesOrInfinite.ts
packages/shared/src/react/hooks/usePlans.tsx
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/shared/src/react/hooks/createBillingPaginatedHook.tsx
packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
packages/shared/src/react/hooks/usePagesOrInfinite.ts
packages/shared/src/react/hooks/usePlans.tsx
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/shared/src/react/hooks/createBillingPaginatedHook.tsx
packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
packages/shared/src/react/hooks/usePagesOrInfinite.ts
packages/shared/src/react/hooks/usePlans.tsx
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/shared/src/react/hooks/createBillingPaginatedHook.tsx
packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
packages/shared/src/react/hooks/usePagesOrInfinite.ts
packages/shared/src/react/hooks/usePlans.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}
: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidany
type - preferunknown
when type is uncertain, then narrow with type guards
Useinterface
for object shapes that might be extended
Usetype
for unions, primitives, and computed types
Preferreadonly
properties for immutable data structures
Useprivate
for internal implementation details
Useprotected
for inheritance hierarchies
Usepublic
explicitly for clarity in public APIs
Preferreadonly
for properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertions
for literal types:as const
Usesatisfies
operator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noany
types without justification
Proper error handling with typed errors
Consistent use ofreadonly
for immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/shared/src/react/hooks/createBillingPaginatedHook.tsx
packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
packages/shared/src/react/hooks/usePagesOrInfinite.ts
packages/shared/src/react/hooks/usePlans.tsx
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{jsx,tsx}
: Use error boundaries in React components
Minimize re-renders in React components
**/*.{jsx,tsx}
: Always use functional components with hooks instead of class components
Follow PascalCase naming for components:UserProfile
,NavigationMenu
Keep components focused on a single responsibility - split large components
Limit component size to 150-200 lines; extract logic into custom hooks
Use composition over inheritance - prefer smaller, composable components
Export components as named exports for better tree-shaking
One component per file with matching filename and component name
Use useState for simple state management
Use useReducer for complex state logic
Implement proper state initialization
Use proper state updates with callbacks
Implement proper state cleanup
Use Context API for theme/authentication
Implement proper state selectors
Use proper state normalization
Implement proper state persistence
Use React.memo for expensive components
Implement proper useCallback for handlers
Use proper useMemo for expensive computations
Implement proper virtualization for lists
Use proper code splitting with React.lazy
Implement proper cleanup in useEffect
Use proper refs for DOM access
Implement proper event listener cleanup
Use proper abort controllers for fetch
Implement proper subscription cleanup
Use proper HTML elements
Implement proper ARIA attributes
Use proper heading hierarchy
Implement proper form labels
Use proper button types
Implement proper focus management
Use proper keyboard shortcuts
Implement proper tab order
Use proper skip links
Implement proper focus traps
Implement proper error boundaries
Use proper error logging
Implement proper error recovery
Use proper error messages
Implement proper error fallbacks
Use proper form validation
Implement proper error states
Use proper error messages
Implement proper form submission
Use proper form reset
Use proper component naming
Implement proper file naming
Use proper prop naming
Implement proper...
Files:
packages/shared/src/react/hooks/createBillingPaginatedHook.tsx
packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx
packages/shared/src/react/hooks/usePlans.tsx
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/shared/src/react/hooks/createBillingPaginatedHook.tsx
packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
packages/shared/src/react/hooks/usePagesOrInfinite.ts
packages/shared/src/react/hooks/usePlans.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.tsx
: Use proper type definitions for props and state
Leverage TypeScript's type inference where possible
Use proper event types for handlers
Implement proper generic types for reusable components
Use proper type guards for conditional rendering
Files:
packages/shared/src/react/hooks/createBillingPaginatedHook.tsx
packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx
packages/shared/src/react/hooks/usePlans.tsx
packages/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Unit tests should use Jest or Vitest as the test runner.
Files:
packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/__tests__/**/*.{ts,tsx}
: Create type-safe test builders/factories
Use branded types for test isolation
Implement proper mock types that match interfaces
Files:
packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
🧬 Code graph analysis (3)
packages/shared/src/react/hooks/createBillingPaginatedHook.tsx (1)
packages/react/src/isomorphicClerk.ts (1)
organization
(698-704)
packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx (1)
packages/shared/src/react/hooks/usePlans.tsx (1)
usePlans
(9-22)
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts (2)
packages/shared/src/react/hooks/usePagesOrInfinite.ts (1)
usePagesOrInfinite
(155-353)packages/shared/src/utils/createDeferredPromise.ts (1)
createDeferredPromise
(11-19)
⏰ 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 (1)
packages/shared/src/react/hooks/usePlans.tsx (1)
17-18
: LGTM: pass-through params with explicitfor
is correctMatches factory expectations and tests; unauthenticated mode is appropriate.
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.
Actionable comments posted: 1
🧹 Nitpick comments (9)
packages/shared/src/react/hooks/usePlans.tsx (1)
17-17
: Defensive param filtering: avoid leaking orgId/userId to getPlansForwarding all fields works with current call sites, but it’s safer to explicitly omit orgId/userId at this boundary to prevent accidental leakage if upstream changes later include them. This also reduces risk of 422s on strict APIs.
- return params => clerk.billing.getPlans({ ...params, for: _for }); + return params => { + const { orgId: _omitOrgId, userId: _omitUserId, ...rest } = (params || {}) as Record<string, unknown>; + return clerk.billing.getPlans({ ...(rest as any), for: _for }); + };.changeset/seven-turkeys-sin.md (1)
7-7
: Fix grammar and hook name in release noteUse “usePlans” and remove the redundant phrasing.
-Improves the `usePlan` hook has been updated to not fire requests when switching organizations or when users sign in/out. +The `usePlans` hook has been updated to not fire requests when switching organizations or when users sign in/out.packages/shared/src/react/hooks/createBillingPaginatedHook.tsx (2)
81-85
: Only include orgId when defined to avoid sending undefined valuesThis prevents passing
{ orgId: undefined }
downstream.- ...(options?.unauthenticated ? {} : _for === 'organization' ? { orgId: organization?.id } : {}), + ...(options?.unauthenticated + ? {} + : _for === 'organization' && organization?.id + ? { orgId: organization.id } + : {}),
104-112
: Cache key and payload hygiene: include userId/orgId only when they existAvoids undefined-valued keys/props and keeps cache keys more stable.
- // userId: user?.id, - ...(options?.unauthenticated - ? {} - : { - userId: user?.id, - ...(_for === 'organization' ? { orgId: organization?.id } : {}), - }), + ...(options?.unauthenticated + ? {} + : { + ...(user?.id ? { userId: user.id } : {}), + ...(_for === 'organization' && organization?.id ? { orgId: organization.id } : {}), + }),packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx (2)
44-49
: Reset mock identities in beforeEach to avoid cross-test leakageOne test mutates mockUser.id / mockOrganization.id; reset them for isolation.
beforeEach(() => { vi.clearAllMocks(); mockClerk.loaded = true; mockClerk.__unstable__environment.commerceSettings.billing.user.enabled = true; mockClerk.__unstable__environment.commerceSettings.billing.organization.enabled = true; + // Ensure isolation across tests + mockUser.id = 'user_1'; + mockOrganization.id = 'org_1'; });
92-96
: Reduce flakiness: wait on fetch call instead of transient isLoading=trueWaiting for isLoading=true can race; assert the spy was called instead.
- await waitFor(() => expect(result.current.isLoading).toBe(true)); - - expect(getPlansSpy).toHaveBeenCalledTimes(1); + await waitFor(() => expect(getPlansSpy).toHaveBeenCalledTimes(1));packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx (1)
115-120
: Prefer waiting on fetcher invocation over isLoading=trueMinor flakiness guard: assert fetcherMock call instead of transient loading state.
- await waitFor(() => expect(result.current.isLoading).toBe(true)); - expect(fetcherMock).toHaveBeenCalled(); + await waitFor(() => expect(fetcherMock).toHaveBeenCalled());packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts (2)
88-96
: Avoid waiting for isLoading=true; wait on fetch call or stable stateSwitching to spy-based waits reduces race conditions in CI.
- await waitFor(() => expect(result.current.isLoading).toBe(true)); - - // First call: should include provided params, not include cache keys - expect(fetcher).toHaveBeenCalledTimes(1); + await waitFor(() => expect(fetcher).toHaveBeenCalledTimes(1)); + // First call: should include provided params, not include cache keys
232-248
: Cache mode test: assert fetcher not called without relying on timingYou can assert zero calls directly; no need to wait on isFetching=false first.
- // Should never be fetching in cache mode - expect(result.current.isFetching).toBe(false); - await waitFor(() => expect(result.current.isFetching).toBe(false)); - // Should not have called fetcher - expect(fetcher).toHaveBeenCalledTimes(0); + // Cache mode: fetcher must never be called + expect(fetcher).toHaveBeenCalledTimes(0);
📜 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 (7)
.changeset/seven-turkeys-sin.md
(1 hunks)packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx
(1 hunks)packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
(1 hunks)packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx
(1 hunks)packages/shared/src/react/hooks/createBillingPaginatedHook.tsx
(2 hunks)packages/shared/src/react/hooks/usePagesOrInfinite.ts
(7 hunks)packages/shared/src/react/hooks/usePlans.tsx
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}
: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/shared/src/react/hooks/usePlans.tsx
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx
packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx
packages/shared/src/react/hooks/createBillingPaginatedHook.tsx
packages/shared/src/react/hooks/usePagesOrInfinite.ts
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/shared/src/react/hooks/usePlans.tsx
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx
packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx
packages/shared/src/react/hooks/createBillingPaginatedHook.tsx
packages/shared/src/react/hooks/usePagesOrInfinite.ts
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/shared/src/react/hooks/usePlans.tsx
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx
packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx
packages/shared/src/react/hooks/createBillingPaginatedHook.tsx
packages/shared/src/react/hooks/usePagesOrInfinite.ts
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/shared/src/react/hooks/usePlans.tsx
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx
packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx
packages/shared/src/react/hooks/createBillingPaginatedHook.tsx
packages/shared/src/react/hooks/usePagesOrInfinite.ts
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}
: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidany
type - preferunknown
when type is uncertain, then narrow with type guards
Useinterface
for object shapes that might be extended
Usetype
for unions, primitives, and computed types
Preferreadonly
properties for immutable data structures
Useprivate
for internal implementation details
Useprotected
for inheritance hierarchies
Usepublic
explicitly for clarity in public APIs
Preferreadonly
for properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertions
for literal types:as const
Usesatisfies
operator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noany
types without justification
Proper error handling with typed errors
Consistent use ofreadonly
for immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/shared/src/react/hooks/usePlans.tsx
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx
packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx
packages/shared/src/react/hooks/createBillingPaginatedHook.tsx
packages/shared/src/react/hooks/usePagesOrInfinite.ts
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{jsx,tsx}
: Use error boundaries in React components
Minimize re-renders in React components
**/*.{jsx,tsx}
: Always use functional components with hooks instead of class components
Follow PascalCase naming for components:UserProfile
,NavigationMenu
Keep components focused on a single responsibility - split large components
Limit component size to 150-200 lines; extract logic into custom hooks
Use composition over inheritance - prefer smaller, composable components
Export components as named exports for better tree-shaking
One component per file with matching filename and component name
Use useState for simple state management
Use useReducer for complex state logic
Implement proper state initialization
Use proper state updates with callbacks
Implement proper state cleanup
Use Context API for theme/authentication
Implement proper state selectors
Use proper state normalization
Implement proper state persistence
Use React.memo for expensive components
Implement proper useCallback for handlers
Use proper useMemo for expensive computations
Implement proper virtualization for lists
Use proper code splitting with React.lazy
Implement proper cleanup in useEffect
Use proper refs for DOM access
Implement proper event listener cleanup
Use proper abort controllers for fetch
Implement proper subscription cleanup
Use proper HTML elements
Implement proper ARIA attributes
Use proper heading hierarchy
Implement proper form labels
Use proper button types
Implement proper focus management
Use proper keyboard shortcuts
Implement proper tab order
Use proper skip links
Implement proper focus traps
Implement proper error boundaries
Use proper error logging
Implement proper error recovery
Use proper error messages
Implement proper error fallbacks
Use proper form validation
Implement proper error states
Use proper error messages
Implement proper form submission
Use proper form reset
Use proper component naming
Implement proper file naming
Use proper prop naming
Implement proper...
Files:
packages/shared/src/react/hooks/usePlans.tsx
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx
packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx
packages/shared/src/react/hooks/createBillingPaginatedHook.tsx
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/shared/src/react/hooks/usePlans.tsx
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx
packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx
packages/shared/src/react/hooks/createBillingPaginatedHook.tsx
packages/shared/src/react/hooks/usePagesOrInfinite.ts
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.tsx
: Use proper type definitions for props and state
Leverage TypeScript's type inference where possible
Use proper event types for handlers
Implement proper generic types for reusable components
Use proper type guards for conditional rendering
Files:
packages/shared/src/react/hooks/usePlans.tsx
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx
packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx
packages/shared/src/react/hooks/createBillingPaginatedHook.tsx
packages/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Unit tests should use Jest or Vitest as the test runner.
Files:
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx
packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/__tests__/**/*.{ts,tsx}
: Create type-safe test builders/factories
Use branded types for test isolation
Implement proper mock types that match interfaces
Files:
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx
packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
.changeset/**
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Automated releases must use Changesets.
Files:
.changeset/seven-turkeys-sin.md
🧬 Code graph analysis (3)
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx (2)
packages/types/src/resource.ts (1)
ClerkResource
(8-21)packages/shared/src/react/hooks/createBillingPaginatedHook.tsx (1)
createBillingPaginatedHook
(41-117)
packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx (1)
packages/shared/src/react/hooks/usePlans.tsx (1)
usePlans
(9-22)
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts (2)
packages/shared/src/react/hooks/usePagesOrInfinite.ts (1)
usePagesOrInfinite
(155-353)packages/shared/src/utils/createDeferredPromise.ts (1)
createDeferredPromise
(11-19)
🪛 LanguageTool
.changeset/seven-turkeys-sin.md
[grammar] ~7-~7: There might be a mistake here.
Context: ...organizations or when users sign in/out.
(QB_NEW_EN)
⏰ 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: semgrep-cloud-platform/scan
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/shared/src/react/hooks/usePagesOrInfinite.ts (2)
167-188
: Complex but correct sign-out transition logic.The nested ternary correctly implements the sign-out behavior described in the comments. However, consider extracting this into a helper function for improved readability.
Optional refactor to improve readability:
function getSwrKey( isSignedIn: boolean | undefined, previousIsSignedIn: boolean | null, shouldFetch: boolean, pagesCacheKey: Record<string, unknown> ) { // When isSignedIn is undefined, fall back to simple shouldFetch logic if (typeof isSignedIn !== 'boolean') { return shouldFetch ? pagesCacheKey : null; } // Transitioning from signed in to signed out: use cache key to trigger null fetch if (previousIsSignedIn === true && isSignedIn === false) { return pagesCacheKey; } // Currently signed in: fetch if shouldFetch is true if (isSignedIn) { return shouldFetch ? pagesCacheKey : null; } // Currently signed out (not a transition): don't fetch return null; } const swrKey = getSwrKey(isSignedIn, previousIsSignedIn, shouldFetch, pagesCacheKey);
246-252
: Consider improving type safety for filtered params.The
@ts-ignore
on line 249 suppresses type checking when passing filtered params to the fetcher. While the filtering logic is correct, this could hide genuine type mismatches between the filtered params and the fetcher's expected type.Consider one of these approaches:
- Add a runtime assertion that the filtered params contain all required fields
- Use a more specific type for
requestParams
that the fetcher can accept- Create a separate type that represents the fetcher's minimal required params
Example approach with type assertion:
cacheKeyParams => { const requestParams = getDifferentKeys(cacheKeyParams, cacheKeys) as Partial<typeof params>; return fetcher?.(requestParams as typeof params); }However, if the current approach is validated by tests and works correctly in practice, this refactor can be deferred.
📜 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/__tests__/usePreviousValue.spec.ts
(1 hunks)packages/shared/src/react/hooks/usePagesOrInfinite.ts
(7 hunks)packages/shared/src/react/hooks/usePreviousValue.ts
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}
: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/shared/src/react/hooks/usePreviousValue.ts
packages/shared/src/react/hooks/usePagesOrInfinite.ts
packages/shared/src/react/hooks/__tests__/usePreviousValue.spec.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/shared/src/react/hooks/usePreviousValue.ts
packages/shared/src/react/hooks/usePagesOrInfinite.ts
packages/shared/src/react/hooks/__tests__/usePreviousValue.spec.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/shared/src/react/hooks/usePreviousValue.ts
packages/shared/src/react/hooks/usePagesOrInfinite.ts
packages/shared/src/react/hooks/__tests__/usePreviousValue.spec.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/shared/src/react/hooks/usePreviousValue.ts
packages/shared/src/react/hooks/usePagesOrInfinite.ts
packages/shared/src/react/hooks/__tests__/usePreviousValue.spec.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}
: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidany
type - preferunknown
when type is uncertain, then narrow with type guards
Useinterface
for object shapes that might be extended
Usetype
for unions, primitives, and computed types
Preferreadonly
properties for immutable data structures
Useprivate
for internal implementation details
Useprotected
for inheritance hierarchies
Usepublic
explicitly for clarity in public APIs
Preferreadonly
for properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertions
for literal types:as const
Usesatisfies
operator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noany
types without justification
Proper error handling with typed errors
Consistent use ofreadonly
for immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/shared/src/react/hooks/usePreviousValue.ts
packages/shared/src/react/hooks/usePagesOrInfinite.ts
packages/shared/src/react/hooks/__tests__/usePreviousValue.spec.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/shared/src/react/hooks/usePreviousValue.ts
packages/shared/src/react/hooks/usePagesOrInfinite.ts
packages/shared/src/react/hooks/__tests__/usePreviousValue.spec.ts
packages/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Unit tests should use Jest or Vitest as the test runner.
Files:
packages/shared/src/react/hooks/__tests__/usePreviousValue.spec.ts
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/__tests__/**/*.{ts,tsx}
: Create type-safe test builders/factories
Use branded types for test isolation
Implement proper mock types that match interfaces
Files:
packages/shared/src/react/hooks/__tests__/usePreviousValue.spec.ts
🧬 Code graph analysis (2)
packages/shared/src/react/hooks/usePagesOrInfinite.ts (1)
packages/shared/src/react/hooks/usePreviousValue.ts (1)
usePreviousValue
(20-30)
packages/shared/src/react/hooks/__tests__/usePreviousValue.spec.ts (1)
packages/shared/src/react/hooks/usePreviousValue.ts (1)
usePreviousValue
(20-30)
⏰ 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). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (13)
packages/shared/src/react/hooks/__tests__/usePreviousValue.spec.ts (4)
7-10
: LGTM!The test correctly verifies that
usePreviousValue
returnsnull
on the first render.
12-19
: LGTM!The test correctly verifies string value tracking across multiple renders.
21-30
: LGTM!The test correctly verifies number tracking and also tests the important case where rerendering with the same value (line 26) keeps the previous value stable.
32-39
: LGTM!The test correctly verifies boolean value tracking.
packages/shared/src/react/hooks/usePreviousValue.ts (2)
3-3
: LGTM!The
Primitive
type definition is comprehensive and correctly includes all JavaScript primitive types.
20-30
: Excellent implementation - past review concern addressed.This implementation correctly uses refs instead of state, avoiding the render-phase setState issue flagged in the previous review. The ref-based approach is safe because:
- Ref updates don't trigger re-renders
- Reading/writing refs during render is allowed for this pattern
- The logic is synchronous and deterministic
This is the recommended React pattern for tracking previous values.
packages/shared/src/react/hooks/usePagesOrInfinite.ts (7)
13-13
: LGTM!The import of
usePreviousValue
is correctly placed.
161-161
: LGTM!The
usePreviousValue
hook is correctly used to track the previousisSignedIn
state, which is essential for the sign-out transition logic.
190-199
: LGTM!The fetcher correctly guards against fetching when signed out and properly filters cache-only keys from the request parameters.
209-224
: Excellent documentation!The comments clearly explain why
useSWRInfinite
has simpler sign-out semantics compared touseSWR
, which will help future maintainers understand the design decisions.
234-244
: LGTM!The key getter correctly returns
null
when signed out, preventing page fetches in infinite mode. This aligns with the behavior documented in the comments above.
262-271
: LGTM - Dependency array correctly updated!The
fetchPage
callback now correctly includestriggerInfinite
in its dependency array (line 270), which is necessary since the callback's behavior depends on this value. This prevents potential stale closure issues.
85-86
: Acceptable use of@ts-ignore
.The
@ts-ignore
is justified here becausedefaultValues
andparams
share the same shape, making dynamic key access safe. The comment clearly explains the reasoning.
# Conflicts: # packages/shared/package.json
Description
This PR
usePlan
that fetch unauthenticated data do not fire requests when orgId or userId changes.Test added for
usePagesOrInfinite
createBillingPaginatedHook
which is a factory for hooks likeuseStatements
usePlans
which is a special case ofcreateBillingPaginatedHook
useSubscription
which is not a paginated list of data but uses SWR for data fetching.Checklist
pnpm test
runs as expected.pnpm build
runs as expected.Type of change
Summary by CodeRabbit
Bug Fixes
Tests
Chores