[WIP] chore(share): Add RQ variant of usePageOrInfinite
#7143
Conversation
This reverts commit 7fc1248.
# Conflicts: # .changeset/tricky-badgers-post.md # packages/clerk-js/bundlewatch.config.json # packages/clerk-js/package.json # packages/clerk-js/src/core/clerk.ts # packages/shared/src/react/clerk-rq/types.ts # packages/shared/src/react/clerk-rq/useBaseQuery.ts # packages/shared/src/react/contexts.tsx # packages/shared/src/react/hooks/usePagesOrInfinite.ts # packages/shared/src/react/hooks/useSubscription.rq.tsx # packages/shared/src/react/hooks/useSubscription.swr.tsx # packages/shared/src/react/hooks/useSubscription.types.ts # packages/shared/src/types/virtual-data-hooks.d.ts # packages/shared/tsconfig.json # packages/shared/tsup.config.ts # pnpm-lock.yaml
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 35f4e83 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 |
WalkthroughThis PR introduces React Query (RQ) integration for infinite pagination, adds type utilities for infinite queries, implements a dual-mode Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant usePagesOrInfinite as usePagesOrInfinite Hook
participant RQ as React Query<br/>(or SWR)
participant Fetcher as User Fetcher
Note over App,Fetcher: Single-Page Mode (keepPreviousData=false)
App->>usePagesOrInfinite: mount with config (infinite=false)
usePagesOrInfinite->>RQ: useClerkQuery(queryKey, fetcher)
RQ->>Fetcher: call fetcher(params)
Fetcher-->>RQ: return {data, total_count}
RQ-->>usePagesOrInfinite: {data, isLoading}
usePagesOrInfinite-->>App: {data, isLoading, fetchPage, ...}
App->>usePagesOrInfinite: fetchPage(2)
usePagesOrInfinite->>RQ: useClerkQuery with new page
RQ->>Fetcher: call fetcher with pageParam=2
Fetcher-->>RQ: return page 2 data
RQ-->>usePagesOrInfinite: updated data
usePagesOrInfinite-->>App: {data: pageData, isLoading}
Note over App,Fetcher: Infinite Mode (infinite=true)
App->>usePagesOrInfinite: mount with config (infinite=true)
usePagesOrInfinite->>RQ: useClerkInfiniteQuery(infiniteQueryKey, fetcher)
RQ->>Fetcher: call fetcher(pageParam, params)
Fetcher-->>RQ: return {data, total_count}
RQ-->>usePagesOrInfinite: {pages, isLoading}
usePagesOrInfinite-->>App: {data: [merged pages], isLoading, fetchNext, ...}
App->>usePagesOrInfinite: fetchNext()
usePagesOrInfinite->>RQ: fetchNextPage()
RQ->>Fetcher: call fetcher with next pageParam
Fetcher-->>RQ: return next page data
RQ-->>usePagesOrInfinite: accumulated pages
usePagesOrInfinite-->>App: {data: [page1, page2, ...], isFetching}
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1).github/workflows/*.yml📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Files:
⏰ 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). (33)
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: |
b7bb779 to
751dd31
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
packages/shared/src/react/hooks/useSubscription.rq.tsx (1)
59-59: Update TODO to reference current React Query v5 API.React Query v5 replaced
keepPreviousDatawithplaceholderData. Since this project uses @tanstack/query-core version 5.87.4, the TODO should reference the current API.Apply this diff:
- // TODO: Add support for keepPreviousData + // TODO: Add support for placeholderData (replaces keepPreviousData in React Query v5)packages/shared/src/react/hooks/usePagesOrInfinite.shared.ts (1)
34-54: Consider improving type safety and clarifying behavior.The implementation has a few areas for improvement:
Type assertion at line 53: The
as Tassertion may not be type-safe since the function constructs a new object shape. The returned object might not match all properties ofTifparamshas additional properties beyond those indefaultValues.Generic indexing at lines 45-46: The
@ts-ignoresuppresses type checking when accessing properties. Consider using a more type-safe approach:type DefaultKeys = keyof typeof defaultValues; const key = k as DefaultKeys; newObj[key] = shouldUseDefaults ? defaultValues[key] : (params?.[key] ?? defaultValues[key]);
- Edge case: When
paramsis an object with a property explicitly set toundefined, the nullish coalescing operator??will use the default value. This may or may not be the intended behavior - consider documenting this.Consider this alternative implementation:
export const useWithSafeValues = <T extends PagesOrInfiniteOptions>(params: T | true | undefined, defaultValues: T) => { const shouldUseDefaults = typeof params === 'boolean' && params; // Cache initialPage and initialPageSize until unmount const initialPageRef = useRef( shouldUseDefaults ? defaultValues.initialPage : (params?.initialPage ?? defaultValues.initialPage), ); const pageSizeRef = useRef(shouldUseDefaults ? defaultValues.pageSize : (params?.pageSize ?? defaultValues.pageSize)); - const newObj: Record<string, unknown> = {}; + const newObj = {} as Record<keyof T, any>; for (const key of Object.keys(defaultValues)) { - // @ts-ignore - indexing into generic param to preserve unknown keys from defaults/params - newObj[key] = shouldUseDefaults ? defaultValues[key] : (params?.[key] ?? defaultValues[key]); + const typedKey = key as keyof T; + newObj[typedKey] = shouldUseDefaults ? defaultValues[typedKey] : (params?.[typedKey] ?? defaultValues[typedKey]); } return { ...newObj, initialPage: initialPageRef.current, pageSize: pageSizeRef.current, } as T; };packages/shared/src/react/hooks/usePageOrInfinite.types.ts (1)
7-26: Consider constraining FetcherReturnData more strictly.The generic parameter
FetcherReturnData extends Record<string, any>on line 9 usesany, which weakens type safety. Since the return data should have adataproperty (based on theExtractDatausage), consider a more specific constraint:export type UsePagesOrInfiniteSignature = < Params extends PagesOrInfiniteOptions, - FetcherReturnData extends Record<string, any>, + FetcherReturnData extends { data?: unknown[] } | unknown, CacheKeys extends Record<string, unknown> = Record<string, unknown>, TConfig extends PagesOrInfiniteConfig = PagesOrInfiniteConfig, >(This would provide better type checking while still being flexible enough for various return shapes.
packages/shared/src/react/clerk-rq/useInfiniteQuery.ts (1)
39-44: Remove empty JSDoc or add documentation.The JSDoc comment block on lines 39-41 is empty. Either remove it or add meaningful documentation explaining the hook's purpose and usage.
Apply this diff:
-/** - * - */ export function useClerkInfiniteQuery(options: UseInfiniteQueryOptions) { return useBaseQuery(options, InfiniteQueryObserver as unknown as typeof QueryObserver); }Or add proper documentation:
/** - * + * Hook for infinite/paginated queries using React Query with Clerk integration. + * Delegates to useBaseQuery with InfiniteQueryObserver for managing cursor-based pagination. */ export function useClerkInfiniteQuery(options: UseInfiniteQueryOptions) { return useBaseQuery(options, InfiniteQueryObserver as unknown as typeof QueryObserver); }packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx (2)
22-23: Track TODO for keepPreviousData support.The React Query implementation lacks support for
keepPreviousDataconfiguration. This feature is available in the SWR variant and should be implemented for feature parity.Do you want me to open a new issue to track this enhancement, or will this be addressed before the PR is merged?
52-53: Document or resolve type assertions.The
@ts-ignorecomments on lines 52 and 82 suppress TypeScript errors for type compatibility. Consider:
- Adding detailed comments explaining why the types differ and why the assertion is safe
- Refining the types to eliminate the need for suppression
- Using
@ts-expect-errorwith specific error codes instead of@ts-ignorefor better type safetyAlso applies to: 82-83
packages/shared/src/react/clerk-rq/infiniteQueryOptions.ts (1)
89-94: Remove empty JSDoc or add documentation.The JSDoc comment block on lines 89-91 is empty. Either remove it or add meaningful documentation explaining that this is a type-level helper that returns options unchanged for type inference purposes.
Apply this diff:
-/** - * - */ export function infiniteQueryOptions(options: unknown) { return options; }Or add proper documentation:
/** - * + * Type-level helper for infinite query options that adds proper type inference. + * Returns the options unchanged at runtime but provides enhanced TypeScript types. */ export function infiniteQueryOptions(options: unknown) { return options; }
📜 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 (15)
.changeset/fuzzy-keys-smell.md(1 hunks)packages/clerk-js/src/test/mock-helpers.ts(3 hunks)packages/clerk-js/src/ui/components/OrganizationList/__tests__/OrganizationList.test.tsx(2 hunks)packages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsx(2 hunks)packages/shared/src/react/clerk-rq/infiniteQueryOptions.ts(1 hunks)packages/shared/src/react/clerk-rq/types.ts(2 hunks)packages/shared/src/react/clerk-rq/useInfiniteQuery.ts(1 hunks)packages/shared/src/react/hooks/usePageOrInfinite.types.ts(1 hunks)packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx(1 hunks)packages/shared/src/react/hooks/usePagesOrInfinite.shared.ts(1 hunks)packages/shared/src/react/hooks/usePagesOrInfinite.swr.tsx(4 hunks)packages/shared/src/react/hooks/usePagesOrInfinite.tsx(1 hunks)packages/shared/src/react/hooks/useSubscription.rq.tsx(1 hunks)packages/shared/src/types/virtual-data-hooks.d.ts(1 hunks)packages/shared/tsconfig.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (15)
**/*.{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/usePagesOrInfinite.tsxpackages/clerk-js/src/ui/components/OrganizationList/__tests__/OrganizationList.test.tsxpackages/shared/src/react/hooks/usePagesOrInfinite.shared.tspackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsxpackages/clerk-js/src/test/mock-helpers.tspackages/shared/src/types/virtual-data-hooks.d.tspackages/shared/src/react/clerk-rq/types.tspackages/shared/src/react/clerk-rq/useInfiniteQuery.tspackages/shared/src/react/hooks/usePagesOrInfinite.rq.tsxpackages/shared/src/react/hooks/useSubscription.rq.tsxpackages/shared/src/react/hooks/usePageOrInfinite.types.tspackages/shared/src/react/clerk-rq/infiniteQueryOptions.tspackages/shared/src/react/hooks/usePagesOrInfinite.swr.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/usePagesOrInfinite.tsxpackages/clerk-js/src/ui/components/OrganizationList/__tests__/OrganizationList.test.tsxpackages/shared/src/react/hooks/usePagesOrInfinite.shared.tspackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsxpackages/clerk-js/src/test/mock-helpers.tspackages/shared/src/types/virtual-data-hooks.d.tspackages/shared/src/react/clerk-rq/types.tspackages/shared/src/react/clerk-rq/useInfiniteQuery.tspackages/shared/src/react/hooks/usePagesOrInfinite.rq.tsxpackages/shared/src/react/hooks/useSubscription.rq.tsxpackages/shared/src/react/hooks/usePageOrInfinite.types.tspackages/shared/src/react/clerk-rq/infiniteQueryOptions.tspackages/shared/tsconfig.jsonpackages/shared/src/react/hooks/usePagesOrInfinite.swr.tsx
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/shared/src/react/hooks/usePagesOrInfinite.tsxpackages/clerk-js/src/ui/components/OrganizationList/__tests__/OrganizationList.test.tsxpackages/shared/src/react/hooks/usePagesOrInfinite.shared.tspackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsxpackages/clerk-js/src/test/mock-helpers.tspackages/shared/src/types/virtual-data-hooks.d.tspackages/shared/src/react/clerk-rq/types.tspackages/shared/src/react/clerk-rq/useInfiniteQuery.tspackages/shared/src/react/hooks/usePagesOrInfinite.rq.tsxpackages/shared/src/react/hooks/useSubscription.rq.tsxpackages/shared/src/react/hooks/usePageOrInfinite.types.tspackages/shared/src/react/clerk-rq/infiniteQueryOptions.tspackages/shared/src/react/hooks/usePagesOrInfinite.swr.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/usePagesOrInfinite.tsxpackages/clerk-js/src/ui/components/OrganizationList/__tests__/OrganizationList.test.tsxpackages/shared/src/react/hooks/usePagesOrInfinite.shared.tspackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsxpackages/clerk-js/src/test/mock-helpers.tspackages/shared/src/types/virtual-data-hooks.d.tspackages/shared/src/react/clerk-rq/types.tspackages/shared/src/react/clerk-rq/useInfiniteQuery.tspackages/shared/src/react/hooks/usePagesOrInfinite.rq.tsxpackages/shared/src/react/hooks/useSubscription.rq.tsxpackages/shared/src/react/hooks/usePageOrInfinite.types.tspackages/shared/src/react/clerk-rq/infiniteQueryOptions.tspackages/shared/src/react/hooks/usePagesOrInfinite.swr.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
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor 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 assertionsfor literal types:as const
Usesatisfiesoperator 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 ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor 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/usePagesOrInfinite.tsxpackages/clerk-js/src/ui/components/OrganizationList/__tests__/OrganizationList.test.tsxpackages/shared/src/react/hooks/usePagesOrInfinite.shared.tspackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsxpackages/clerk-js/src/test/mock-helpers.tspackages/shared/src/types/virtual-data-hooks.d.tspackages/shared/src/react/clerk-rq/types.tspackages/shared/src/react/clerk-rq/useInfiniteQuery.tspackages/shared/src/react/hooks/usePagesOrInfinite.rq.tsxpackages/shared/src/react/hooks/useSubscription.rq.tsxpackages/shared/src/react/hooks/usePageOrInfinite.types.tspackages/shared/src/react/clerk-rq/infiniteQueryOptions.tspackages/shared/src/react/hooks/usePagesOrInfinite.swr.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/usePagesOrInfinite.tsxpackages/clerk-js/src/ui/components/OrganizationList/__tests__/OrganizationList.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsxpackages/shared/src/react/hooks/usePagesOrInfinite.rq.tsxpackages/shared/src/react/hooks/useSubscription.rq.tsxpackages/shared/src/react/hooks/usePagesOrInfinite.swr.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/usePagesOrInfinite.tsxpackages/clerk-js/src/ui/components/OrganizationList/__tests__/OrganizationList.test.tsxpackages/shared/src/react/hooks/usePagesOrInfinite.shared.tspackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsxpackages/clerk-js/src/test/mock-helpers.tspackages/shared/src/types/virtual-data-hooks.d.tspackages/shared/src/react/clerk-rq/types.tspackages/shared/src/react/clerk-rq/useInfiniteQuery.tspackages/shared/src/react/hooks/usePagesOrInfinite.rq.tsxpackages/shared/src/react/hooks/useSubscription.rq.tsxpackages/shared/src/react/hooks/usePageOrInfinite.types.tspackages/shared/src/react/clerk-rq/infiniteQueryOptions.tspackages/shared/src/react/hooks/usePagesOrInfinite.swr.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/usePagesOrInfinite.tsxpackages/clerk-js/src/ui/components/OrganizationList/__tests__/OrganizationList.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsxpackages/shared/src/react/hooks/usePagesOrInfinite.rq.tsxpackages/shared/src/react/hooks/useSubscription.rq.tsxpackages/shared/src/react/hooks/usePagesOrInfinite.swr.tsx
packages/clerk-js/src/ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/clerk-js-ui.mdc)
packages/clerk-js/src/ui/**/*.{ts,tsx}: Element descriptors should always be camelCase
Use element descriptors in UI components to enable consistent theming and styling via appearance.elements
Element descriptors should generate unique, stable CSS classes for theming
Element descriptors should handle state classes (e.g., cl-loading, cl-active, cl-error, cl-open) automatically based on component state
Do not render hard-coded values; all user-facing strings must be localized using provided localization methods
Use the useLocalizations hook and localizationKeys utility for all text and error messages
Use the styled system (sx prop, theme tokens, responsive values) for custom component styling
Use useCardState for card-level state, useFormState for form-level state, and useLoadingStatus for loading states
Always use handleError utility for API errors and use translateError for localized error messages
Use useFormControl for form field state, implement proper validation, and handle loading and error states in forms
Use localization keys for all form labels and placeholders
Use element descriptors for consistent styling and follow the theme token system
Use the Card and FormContainer patterns for consistent UI structure
Files:
packages/clerk-js/src/ui/components/OrganizationList/__tests__/OrganizationList.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.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/clerk-js/src/ui/components/OrganizationList/__tests__/OrganizationList.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsx
packages/{clerk-js,elements,themes}/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Visual regression testing should be performed for UI components.
Files:
packages/clerk-js/src/ui/components/OrganizationList/__tests__/OrganizationList.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsx
**/*.test.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.test.{jsx,tsx}: Use React Testing Library
Test component behavior, not implementation
Use proper test queries
Implement proper test isolation
Use proper test coverage
Test component interactions
Use proper test data
Implement proper test setup
Use proper test cleanup
Implement proper test assertions
Use proper test structure
Files:
packages/clerk-js/src/ui/components/OrganizationList/__tests__/OrganizationList.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.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/clerk-js/src/ui/components/OrganizationList/__tests__/OrganizationList.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsx
.changeset/**
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Automated releases must use Changesets.
Files:
.changeset/fuzzy-keys-smell.md
packages/*/tsconfig.json
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Type checking must be performed with TypeScript and publint.
Files:
packages/shared/tsconfig.json
🧬 Code graph analysis (8)
packages/shared/src/react/hooks/usePagesOrInfinite.shared.ts (1)
packages/shared/src/react/types.ts (1)
PagesOrInfiniteOptions(133-156)
packages/clerk-js/src/test/mock-helpers.ts (1)
packages/clerk-js/src/core/query-core.ts (1)
QueryClient(3-3)
packages/shared/src/types/virtual-data-hooks.d.ts (2)
packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx (1)
usePagesOrInfinite(13-191)packages/shared/src/react/hooks/usePagesOrInfinite.swr.tsx (1)
usePagesOrInfinite(32-228)
packages/shared/src/react/clerk-rq/useInfiniteQuery.ts (3)
packages/shared/src/react/clerk-rq/infiniteQueryOptions.ts (2)
DefinedInitialDataInfiniteOptions(40-51)UndefinedInitialDataInfiniteOptions(14-25)packages/shared/src/react/clerk-rq/types.ts (3)
DefinedUseInfiniteQueryResult(60-63)UseInfiniteQueryResult(58-58)UseInfiniteQueryOptions(38-50)packages/shared/src/react/clerk-rq/useBaseQuery.ts (1)
useBaseQuery(23-79)
packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx (7)
packages/shared/src/react/hooks/usePagesOrInfinite.swr.tsx (1)
usePagesOrInfinite(32-228)packages/shared/src/react/hooks/usePageOrInfinite.types.ts (1)
UsePagesOrInfiniteSignature(7-26)packages/shared/src/react/clerk-rq/use-clerk-query-client.ts (1)
useClerkQueryClient(84-84)packages/shared/src/react/clerk-rq/useQuery.ts (1)
useClerkQuery(40-42)packages/shared/src/react/hooks/usePagesOrInfinite.shared.ts (1)
getDifferentKeys(75-89)packages/shared/src/react/clerk-rq/useInfiniteQuery.ts (1)
useClerkInfiniteQuery(42-44)packages/shared/src/react/types.ts (2)
ValueOrSetter(4-4)CacheSetter(6-8)
packages/shared/src/react/hooks/usePageOrInfinite.types.ts (1)
packages/shared/src/react/types.ts (3)
PagesOrInfiniteOptions(133-156)PagesOrInfiniteConfig(104-128)PaginatedResources(13-79)
packages/shared/src/react/clerk-rq/infiniteQueryOptions.ts (1)
packages/shared/src/react/clerk-rq/types.ts (1)
UseInfiniteQueryOptions(38-50)
packages/shared/src/react/hooks/usePagesOrInfinite.swr.tsx (2)
packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx (1)
usePagesOrInfinite(13-191)packages/shared/src/react/hooks/usePageOrInfinite.types.ts (1)
UsePagesOrInfiniteSignature(7-26)
🪛 LanguageTool
.changeset/fuzzy-keys-smell.md
[grammar] ~1-~1: Dit kan een fout zijn.
Context: --- '@clerk/shared': patch --- wip
(QB_NEW_NL)
[grammar] ~5-~5: Dit kan een fout zijn.
Context: --- '@clerk/shared': patch --- wip
(QB_NEW_NL)
⏰ 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). (27)
- GitHub Check: Integration Tests (expo-web, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (quickstart, chrome, 15)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (quickstart, chrome, 16)
- GitHub Check: Integration Tests (nextjs, chrome, 16)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (elements, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Publish with pkg-pr-new
- GitHub Check: Static analysis
- GitHub Check: Unit Tests (22, **)
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (18)
.changeset/fuzzy-keys-smell.md (1)
1-5: Update changeset description before merging.The changeset currently contains only "wip" as the description. Before merging this PR, replace this placeholder with a clear description of the changes (e.g., "Add React Query variant of usePagesOrInfinite hook with infinite query support").
packages/clerk-js/src/test/mock-helpers.ts (1)
49-97: LGTM! Clean test infrastructure for React Query.The addition of
defaultQueryClientwith test-appropriate configuration (no retries, zero stale time) and the__internal_queryClientgetter provides a solid foundation for testing React Query-based hooks. The__internal_prefix appropriately signals this is for internal/test use.packages/clerk-js/src/ui/components/OrganizationList/__tests__/OrganizationList.test.tsx (1)
120-138: Verify test isolation with persistent mocks.The change from
mockReturnValueOncetomockReturnValuemakes these mocks persist across multiple invocations. This aligns with React Query's behavior where queries may be invoked multiple times (initial fetch, refetch, etc.). Ensure the persistent mock values correctly represent the expected behavior for all invocations within the test.Also applies to: 159-164
packages/shared/src/react/hooks/usePagesOrInfinite.shared.ts (1)
75-89: LGTM! Clean implementation with good examples.The
getDifferentKeysfunction is well-implemented with efficient O(n+m) time complexity using a Set for lookups. The JSDoc examples clearly demonstrate the intended use cases.packages/shared/src/react/hooks/usePageOrInfinite.types.ts (1)
3-5: LGTM! Clean type utilities for data extraction.The
ArrayTypeandExtractDatatype helpers correctly use conditional types andinferto extract element types from arrays and unwrap data properties. These will be useful for maintaining type safety across the pagination hooks.packages/shared/src/types/virtual-data-hooks.d.ts (1)
5-5: LGTM! Consistent with existing virtual module pattern.The addition of
usePagesOrInfiniteexport follows the same pattern as other virtual module exports. Usinganyis appropriate here since the actual implementation is resolved at build time through the virtual module system.packages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsx (1)
222-227: Test isolation is properly maintained—no actionable changes needed.The change from
mockReturnValueOncetomockReturnValueis correct. Test isolation is ensured through multiple layers:
clearFetchCache()inbeforeEachclears the fetch cache before each test- Each test creates fresh fixtures via
await createFixtures()- Mocks are configured independently within each test (not in
beforeEach)- Each test specifies distinct mock values (e.g.,
total_count: 14vs.total_count: 5)The persistent mock pattern is appropriate for React Query's potential multiple fetches within a single test execution, while cache clearing prevents state leakage between tests.
packages/shared/tsconfig.json (1)
27-28: LGTM!The new path mapping for
virtual:data-hooks/usePagesOrInfinitecorrectly follows the established pattern for virtual module resolution, pointing to the SWR implementation file.packages/shared/src/react/hooks/usePagesOrInfinite.tsx (1)
1-2: LGTM!Clean re-export pattern that establishes the public API surface. The virtual module import will resolve to the appropriate implementation (SWR or RQ) based on build configuration.
packages/shared/src/react/clerk-rq/types.ts (2)
3-6: LGTM!Imports the necessary observer result types from TanStack Query Core to support infinite query operations.
58-63: LGTM!The new type aliases mirror the existing pattern for standard queries and provide proper TypeScript support for infinite query results with appropriate generic parameters.
packages/shared/src/react/hooks/usePagesOrInfinite.swr.tsx (3)
6-8: LGTM!Good refactoring to import shared types and utilities, promoting code reuse between SWR and RQ implementations.
32-32: LGTM!Using the shared
UsePagesOrInfiniteSignaturetype ensures consistency across different implementation variants.
230-230: LGTM!Re-exporting
useWithSafeValuesmakes this utility available to consumers of the module.packages/shared/src/react/clerk-rq/useInfiniteQuery.ts (1)
10-38: LGTM!The three function overloads correctly provide type inference based on
initialDatapresence, following TanStack Query's typing patterns. This ensures proper return types for defined vs undefined data scenarios.packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx (1)
193-193: LGTM!Re-exporting
useWithSafeValuesmaintains API parity with the SWR implementation.packages/shared/src/react/clerk-rq/infiniteQueryOptions.ts (2)
14-51: LGTM!The three option type aliases provide precise typing for different infinite query scenarios (undefined initial data, unused skip token, defined initial data), following TanStack Query conventions.
53-87: LGTM!The function overloads correctly map each option type to a return type that includes the
DataTagtype brand, enabling better type inference for query keys.
| getNextPageParam: (lastPage, allPages, lastPageParam) => { | ||
| const total = lastPage?.total_count ?? 0; | ||
| const consumed = (allPages.length + (params.initialPage ? params.initialPage - 1 : 0)) * (params.pageSize ?? 10); | ||
| return consumed < total ? (lastPageParam as number) + 1 : undefined; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify getNextPageParam calculation for edge cases.
The calculation (allPages.length + (params.initialPage ? params.initialPage - 1 : 0)) assumes a specific page indexing pattern. Ensure this works correctly when:
initialPageis not 1- Pages have variable sizes
- Total count changes between requests
Run the following verification to check usage patterns of initialPage:
🏁 Script executed:
#!/bin/bash
# Description: Find usages of initialPage to understand expected values and edge cases
# Search for initialPage assignments and usage
rg -n -C3 "initialPage\s*[:=]" --type=ts --type=tsxLength of output: 27
I need to search for initialPage usage with a corrected approach. Let me retry the verification:
🏁 Script executed:
#!/bin/bash
# Description: Find usages of initialPage to understand expected values and edge cases
# Search for initialPage without file type restriction
rg -n -C3 "initialPage" packages/shared/src/react/hooks/ -g "*.ts" -g "*.tsx"
# Also search the broader codebase
echo "=== Broader search for initialPage ==="
rg -n "initialPage\s*=" --type typescript || rg -n "initialPage\s*=" packages/Length of output: 85488
🏁 Script executed:
#!/bin/bash
# Check the complete getNextPageParam implementation and related test coverage
# Show the complete usePagesOrInfinite.rq.tsx around the problematic lines
echo "=== Complete getNextPageParam implementation ==="
sed -n '70,85p' packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx
# Find and show relevant test case that validates initialPage behavior
echo ""
echo "=== Test case validating initialPage > 1 ==="
sed -n '299,320p' packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.tsLength of output: 1680
Refactor the getNextPageParam calculation to use the defaulted initialPageParam value consistently.
The calculation at line 75 should reference the initialPageParam value (set at line 72 with a default of 1) rather than rechecking params.initialPage directly. Replace:
const consumed = (allPages.length + (params.initialPage ? params.initialPage - 1 : 0)) * (params.pageSize ?? 10);with:
const consumed = (allPages.length + (initialPageParam - 1)) * (params.pageSize ?? 10);This eliminates the unnecessary ternary, improves clarity, and ensures the calculation uses the actual starting page value that was already computed. The current code works because undefined being falsy accidentally produces the correct offset (0), but it's fragile and inconsistent with the initialPageParam default set on line 72.
🤖 Prompt for AI Agents
In packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx around lines 73 to
76, the getNextPageParam calculation uses params.initialPage directly causing
inconsistency with the defaulted initialPageParam set at line 72; replace the
ternary that references params.initialPage with a calculation that uses
initialPageParam (i.e., compute consumed using (allPages.length +
(initialPageParam - 1)) * (params.pageSize ?? 10)) so the offset consistently
respects the defaulted starting page and removes the fragile conditional.
| const fetchPage: ValueOrSetter<number> = useCallback( | ||
| numberOrgFn => { | ||
| if (triggerInfinite) { | ||
| const next = typeof numberOrgFn === 'function' ? (numberOrgFn as (n: number) => number)(page) : numberOrgFn; | ||
| const targetCount = Math.max(0, next); | ||
| const currentCount = infiniteQuery.data?.pages?.length ?? 0; | ||
| const toFetch = targetCount - currentCount; | ||
| if (toFetch > 0) { | ||
| void infiniteQuery.fetchNextPage({ cancelRefetch: false }); | ||
| } | ||
| return; | ||
| } | ||
| return setPaginatedPage(numberOrgFn); | ||
| }, | ||
| [infiniteQuery, page, triggerInfinite], | ||
| ); |
There was a problem hiding this comment.
fetchPage infinite mode only supports forward pagination.
In infinite mode (lines 98-106), fetchPage can only fetch forward - it calls fetchNextPage when toFetch > 0 but silently no-ops when trying to navigate backward. This differs from the paginated mode behavior and may confuse users.
Consider either:
- Adding a warning when attempting backward navigation in infinite mode
- Documenting this limitation clearly in JSDoc
- Implementing backward navigation support if feasible
🤖 Prompt for AI Agents
In packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx around lines 96 to
111, the infinite-mode branch only handles forward pagination (calls
fetchNextPage when toFetch > 0) and silently no-ops on backward navigation;
update the handler to detect toFetch < 0 and give clear feedback instead of
silently doing nothing — add a warning log (e.g. console.warn or a logger) that
backward navigation isn’t supported in infinite mode and return, and add a short
JSDoc note above the hook explaining this limitation (or alternatively implement
backward support if you prefer); ensure tests or callers relying on fetchPage
are adjusted to expect the warning behavior.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/clerk-js/src/ui/hooks/__tests__/useCoreOrganization.test.tsx (1)
570-590: Mock configuration issue: SecondmockReturnValueoverwrites the first.The second
mockReturnValuecall on line 590 completely overwrites the first one on line 570. UnlikemockReturnValueOnce,mockReturnValuepersists for all subsequent calls.This means:
- The initial fetch (before line 570) won't get the expected data
- When
fetchNextis called on line 610, it will return items with ids '3' and '4'- But items '1' and '2' were never returned, breaking the infinite fetch expectation
This is likely why the test is failing.
Apply this diff to fix the mock setup:
-fixtures.clerk.organization?.getInvitations.mockReturnValue( +fixtures.clerk.organization?.getInvitations.mockReturnValueOnce( Promise.resolve({ data: [ createFakeOrganizationInvitation({ id: '1', emailAddress: 'admin1@clerk.com', organizationId: '1', createdAt: new Date('2022-01-01'), }), createFakeOrganizationInvitation({ id: '2', emailAddress: 'member2@clerk.com', organizationId: '1', createdAt: new Date('2022-01-01'), }), ], total_count: 4, }), ); -fixtures.clerk.organization?.getInvitations.mockReturnValue( +fixtures.clerk.organization?.getInvitations.mockReturnValueOnce( Promise.resolve({ data: [ createFakeOrganizationInvitation({ id: '3', emailAddress: 'admin3@clerk.com', organizationId: '1', createdAt: new Date('2022-01-01'), }), createFakeOrganizationInvitation({ id: '4', emailAddress: 'member4@clerk.com', organizationId: '1', createdAt: new Date('2022-01-01'), }), ], total_count: 4, }), );After fixing this, you should be able to unskip the test on line 526.
packages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationProfile.test.tsx (1)
73-84: Remove redundantmockRejectedValuecall.Lines 73-74 call
mockRejectedValue(null), but lines 76-84 immediately callmockResolvedValue(...)on the same mocks. The second call overrides the first, making the rejection setup unnecessary and potentially confusing.- fixtures.clerk.billing.getSubscription.mockRejectedValue(null); - fixtures.clerk.billing.getStatements.mockRejectedValue(null); - fixtures.clerk.billing.getSubscription.mockResolvedValue({ id: 'sub_top', subscriptionItems: [
🧹 Nitpick comments (6)
packages/clerk-js/src/ui/hooks/__tests__/useCoreOrganization.test.tsx (1)
105-107: Consider using proper Error objects instead of null for mock rejections.The pattern
mockRejectedValue(null)is repeated throughout the test file (lines 105-107, 232-234, 332-334, 440-442) to prevent unrelated resources from being fetched. However, rejecting withnulldoesn't realistically simulate error scenarios—real API failures typically reject with Error objects.Consider this approach:
-fixtures.clerk.organization?.getDomains.mockRejectedValue(null); -fixtures.clerk.organization?.getMembershipRequests.mockRejectedValue(null); -fixtures.clerk.organization?.getInvitations.mockRejectedValue(null); +fixtures.clerk.organization?.getDomains.mockRejectedValue(new Error('Not testing domains')); +fixtures.clerk.organization?.getMembershipRequests.mockRejectedValue(new Error('Not testing membership requests')); +fixtures.clerk.organization?.getInvitations.mockRejectedValue(new Error('Not testing invitations'));This ensures error-handling paths are tested more realistically.
packages/clerk-js/src/ui/components/PricingTable/__tests__/PricingTable.test.tsx (1)
58-59: Pass explicit error values tomockRejectedValue().Throughout the tests,
mockRejectedValue()is called without arguments, which will reject withundefined. For better test clarity and realism, consider passing explicit error objects.Example:
-fixtures.clerk.billing.getStatements.mockRejectedValue(); -fixtures.clerk.user.getPaymentMethods.mockRejectedValue(); +fixtures.clerk.billing.getStatements.mockRejectedValue(new Error('Not available')); +fixtures.clerk.user.getPaymentMethods.mockRejectedValue(new Error('Not available'));Also applies to: 119-120, 159-159, 191-191, 220-221, 314-315, 341-342, 390-390, 412-413, 435-436, 464-465, 526-527, 581-582, 635-636
packages/clerk-js/src/ui/components/SubscriptionDetails/__tests__/SubscriptionDetails.test.tsx (1)
41-44: Consider extracting repeated mock setup to reduce duplication.The same four billing method mocks are repeated across 11 tests. This pattern could be extracted into a shared test helper function or moved to a common setup block to improve maintainability and adhere to the DRY principle.
Example refactor:
// Add a helper function at the top of the describe block const setupBillingMocks = (fixtures: ReturnType<typeof createFixtures>['fixtures']) => { fixtures.clerk.billing.getPlans.mockResolvedValue([]); fixtures.clerk.billing.getStatements.mockResolvedValue([]); fixtures.clerk.billing.getPaymentAttempts.mockResolvedValue([]); fixtures.clerk.user.getPaymentMethods.mockResolvedValue([]); };Then replace each occurrence with:
- fixtures.clerk.billing.getPlans.mockResolvedValue([]); - fixtures.clerk.billing.getStatements.mockResolvedValue([]); - fixtures.clerk.billing.getPaymentAttempts.mockResolvedValue([]); - fixtures.clerk.user.getPaymentMethods.mockResolvedValue([]); + setupBillingMocks(fixtures);Also applies to: 148-151, 255-258, 342-345, 505-508, 647-650, 754-757, 861-864, 971-974, 1065-1068, 1178-1181
packages/clerk-js/src/ui/components/OrganizationProfile/__tests__/InviteMembersPage.test.tsx (1)
33-34: Consider usingmockResolvedValuewith empty data instead ofmockRejectedValue(null).Throughout this file,
getInvitationsis mocked to reject withnull. While this prevents unintended API calls, it's more idiomatic to resolve with empty data to simulate the absence of invitations:-fixtures.clerk.organization?.getInvitations.mockRejectedValue(null); +fixtures.clerk.organization?.getInvitations.mockResolvedValue({ data: [], total_count: 0 });This pattern better represents the actual API behavior and avoids the unusual practice of rejecting with
null(which isn't a proper Error object).packages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsx (1)
33-34: Consider usingmockResolvedValuewith empty data instead ofmockRejectedValue(null).Similar to the InviteMembersPage test file, rejecting with
nullis unusual. Consider resolving with empty data instead:-fixtures.clerk.organization?.getInvitations.mockRejectedValue(null); -fixtures.clerk.organization?.getMemberships.mockRejectedValue(null); +fixtures.clerk.organization?.getInvitations.mockResolvedValue({ data: [], total_count: 0 }); +fixtures.clerk.organization?.getMemberships.mockResolvedValue({ data: [], total_count: 0 });packages/clerk-js/src/ui/hooks/__tests__/useCoreOrganizationList.test.tsx (1)
68-69: Consider usingmockResolvedValuewith empty data.Similar to other test files, rejecting with
nullis unusual. Consider resolving with empty data instead:-fixtures.clerk.user?.getOrganizationInvitations.mockRejectedValue(null); -fixtures.clerk.user?.getOrganizationSuggestions.mockRejectedValue(null); +fixtures.clerk.user?.getOrganizationInvitations.mockResolvedValue({ data: [], total_count: 0 }); +fixtures.clerk.user?.getOrganizationSuggestions.mockResolvedValue({ data: [], total_count: 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 (12)
packages/clerk-js/src/test/mock-helpers.ts(3 hunks)packages/clerk-js/src/ui/components/Checkout/__tests__/Checkout.test.tsx(1 hunks)packages/clerk-js/src/ui/components/OrganizationList/__tests__/OrganizationList.test.tsx(7 hunks)packages/clerk-js/src/ui/components/OrganizationProfile/__tests__/InviteMembersPage.test.tsx(12 hunks)packages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsx(14 hunks)packages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationProfile.test.tsx(5 hunks)packages/clerk-js/src/ui/components/PricingTable/__tests__/PricingTable.test.tsx(15 hunks)packages/clerk-js/src/ui/components/SubscriptionDetails/__tests__/SubscriptionDetails.test.tsx(12 hunks)packages/clerk-js/src/ui/components/Subscriptions/__tests__/SubscriptionsList.test.tsx(7 hunks)packages/clerk-js/src/ui/components/UserProfile/__tests__/UserProfile.test.tsx(2 hunks)packages/clerk-js/src/ui/hooks/__tests__/useCoreOrganization.test.tsx(10 hunks)packages/clerk-js/src/ui/hooks/__tests__/useCoreOrganizationList.test.tsx(13 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/clerk-js/src/test/mock-helpers.ts
🧰 Additional context used
📓 Path-based instructions (13)
packages/clerk-js/src/ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/clerk-js-ui.mdc)
packages/clerk-js/src/ui/**/*.{ts,tsx}: Element descriptors should always be camelCase
Use element descriptors in UI components to enable consistent theming and styling via appearance.elements
Element descriptors should generate unique, stable CSS classes for theming
Element descriptors should handle state classes (e.g., cl-loading, cl-active, cl-error, cl-open) automatically based on component state
Do not render hard-coded values; all user-facing strings must be localized using provided localization methods
Use the useLocalizations hook and localizationKeys utility for all text and error messages
Use the styled system (sx prop, theme tokens, responsive values) for custom component styling
Use useCardState for card-level state, useFormState for form-level state, and useLoadingStatus for loading states
Always use handleError utility for API errors and use translateError for localized error messages
Use useFormControl for form field state, implement proper validation, and handle loading and error states in forms
Use localization keys for all form labels and placeholders
Use element descriptors for consistent styling and follow the theme token system
Use the Card and FormContainer patterns for consistent UI structure
Files:
packages/clerk-js/src/ui/components/OrganizationList/__tests__/OrganizationList.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsxpackages/clerk-js/src/ui/components/SubscriptionDetails/__tests__/SubscriptionDetails.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/InviteMembersPage.test.tsxpackages/clerk-js/src/ui/components/Checkout/__tests__/Checkout.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useCoreOrganization.test.tsxpackages/clerk-js/src/ui/components/PricingTable/__tests__/PricingTable.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useCoreOrganizationList.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationProfile.test.tsxpackages/clerk-js/src/ui/components/UserProfile/__tests__/UserProfile.test.tsxpackages/clerk-js/src/ui/components/Subscriptions/__tests__/SubscriptionsList.test.tsx
**/*.{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/clerk-js/src/ui/components/OrganizationList/__tests__/OrganizationList.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsxpackages/clerk-js/src/ui/components/SubscriptionDetails/__tests__/SubscriptionDetails.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/InviteMembersPage.test.tsxpackages/clerk-js/src/ui/components/Checkout/__tests__/Checkout.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useCoreOrganization.test.tsxpackages/clerk-js/src/ui/components/PricingTable/__tests__/PricingTable.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useCoreOrganizationList.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationProfile.test.tsxpackages/clerk-js/src/ui/components/UserProfile/__tests__/UserProfile.test.tsxpackages/clerk-js/src/ui/components/Subscriptions/__tests__/SubscriptionsList.test.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/clerk-js/src/ui/components/OrganizationList/__tests__/OrganizationList.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsxpackages/clerk-js/src/ui/components/SubscriptionDetails/__tests__/SubscriptionDetails.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/InviteMembersPage.test.tsxpackages/clerk-js/src/ui/components/Checkout/__tests__/Checkout.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useCoreOrganization.test.tsxpackages/clerk-js/src/ui/components/PricingTable/__tests__/PricingTable.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useCoreOrganizationList.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationProfile.test.tsxpackages/clerk-js/src/ui/components/UserProfile/__tests__/UserProfile.test.tsxpackages/clerk-js/src/ui/components/Subscriptions/__tests__/SubscriptionsList.test.tsx
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/ui/components/OrganizationList/__tests__/OrganizationList.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsxpackages/clerk-js/src/ui/components/SubscriptionDetails/__tests__/SubscriptionDetails.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/InviteMembersPage.test.tsxpackages/clerk-js/src/ui/components/Checkout/__tests__/Checkout.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useCoreOrganization.test.tsxpackages/clerk-js/src/ui/components/PricingTable/__tests__/PricingTable.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useCoreOrganizationList.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationProfile.test.tsxpackages/clerk-js/src/ui/components/UserProfile/__tests__/UserProfile.test.tsxpackages/clerk-js/src/ui/components/Subscriptions/__tests__/SubscriptionsList.test.tsx
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/clerk-js/src/ui/components/OrganizationList/__tests__/OrganizationList.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsxpackages/clerk-js/src/ui/components/SubscriptionDetails/__tests__/SubscriptionDetails.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/InviteMembersPage.test.tsxpackages/clerk-js/src/ui/components/Checkout/__tests__/Checkout.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useCoreOrganization.test.tsxpackages/clerk-js/src/ui/components/PricingTable/__tests__/PricingTable.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useCoreOrganizationList.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationProfile.test.tsxpackages/clerk-js/src/ui/components/UserProfile/__tests__/UserProfile.test.tsxpackages/clerk-js/src/ui/components/Subscriptions/__tests__/SubscriptionsList.test.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
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor 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 assertionsfor literal types:as const
Usesatisfiesoperator 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 ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor 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/clerk-js/src/ui/components/OrganizationList/__tests__/OrganizationList.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsxpackages/clerk-js/src/ui/components/SubscriptionDetails/__tests__/SubscriptionDetails.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/InviteMembersPage.test.tsxpackages/clerk-js/src/ui/components/Checkout/__tests__/Checkout.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useCoreOrganization.test.tsxpackages/clerk-js/src/ui/components/PricingTable/__tests__/PricingTable.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useCoreOrganizationList.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationProfile.test.tsxpackages/clerk-js/src/ui/components/UserProfile/__tests__/UserProfile.test.tsxpackages/clerk-js/src/ui/components/Subscriptions/__tests__/SubscriptionsList.test.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/clerk-js/src/ui/components/OrganizationList/__tests__/OrganizationList.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsxpackages/clerk-js/src/ui/components/SubscriptionDetails/__tests__/SubscriptionDetails.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/InviteMembersPage.test.tsxpackages/clerk-js/src/ui/components/Checkout/__tests__/Checkout.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useCoreOrganization.test.tsxpackages/clerk-js/src/ui/components/PricingTable/__tests__/PricingTable.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useCoreOrganizationList.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationProfile.test.tsxpackages/clerk-js/src/ui/components/UserProfile/__tests__/UserProfile.test.tsxpackages/clerk-js/src/ui/components/Subscriptions/__tests__/SubscriptionsList.test.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/clerk-js/src/ui/components/OrganizationList/__tests__/OrganizationList.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsxpackages/clerk-js/src/ui/components/SubscriptionDetails/__tests__/SubscriptionDetails.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/InviteMembersPage.test.tsxpackages/clerk-js/src/ui/components/Checkout/__tests__/Checkout.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useCoreOrganization.test.tsxpackages/clerk-js/src/ui/components/PricingTable/__tests__/PricingTable.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useCoreOrganizationList.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationProfile.test.tsxpackages/clerk-js/src/ui/components/UserProfile/__tests__/UserProfile.test.tsxpackages/clerk-js/src/ui/components/Subscriptions/__tests__/SubscriptionsList.test.tsx
packages/{clerk-js,elements,themes}/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Visual regression testing should be performed for UI components.
Files:
packages/clerk-js/src/ui/components/OrganizationList/__tests__/OrganizationList.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsxpackages/clerk-js/src/ui/components/SubscriptionDetails/__tests__/SubscriptionDetails.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/InviteMembersPage.test.tsxpackages/clerk-js/src/ui/components/Checkout/__tests__/Checkout.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useCoreOrganization.test.tsxpackages/clerk-js/src/ui/components/PricingTable/__tests__/PricingTable.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useCoreOrganizationList.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationProfile.test.tsxpackages/clerk-js/src/ui/components/UserProfile/__tests__/UserProfile.test.tsxpackages/clerk-js/src/ui/components/Subscriptions/__tests__/SubscriptionsList.test.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/clerk-js/src/ui/components/OrganizationList/__tests__/OrganizationList.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsxpackages/clerk-js/src/ui/components/SubscriptionDetails/__tests__/SubscriptionDetails.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/InviteMembersPage.test.tsxpackages/clerk-js/src/ui/components/Checkout/__tests__/Checkout.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useCoreOrganization.test.tsxpackages/clerk-js/src/ui/components/PricingTable/__tests__/PricingTable.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useCoreOrganizationList.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationProfile.test.tsxpackages/clerk-js/src/ui/components/UserProfile/__tests__/UserProfile.test.tsxpackages/clerk-js/src/ui/components/Subscriptions/__tests__/SubscriptionsList.test.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/clerk-js/src/ui/components/OrganizationList/__tests__/OrganizationList.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsxpackages/clerk-js/src/ui/components/SubscriptionDetails/__tests__/SubscriptionDetails.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/InviteMembersPage.test.tsxpackages/clerk-js/src/ui/components/Checkout/__tests__/Checkout.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useCoreOrganization.test.tsxpackages/clerk-js/src/ui/components/PricingTable/__tests__/PricingTable.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useCoreOrganizationList.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationProfile.test.tsxpackages/clerk-js/src/ui/components/UserProfile/__tests__/UserProfile.test.tsxpackages/clerk-js/src/ui/components/Subscriptions/__tests__/SubscriptionsList.test.tsx
**/*.test.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.test.{jsx,tsx}: Use React Testing Library
Test component behavior, not implementation
Use proper test queries
Implement proper test isolation
Use proper test coverage
Test component interactions
Use proper test data
Implement proper test setup
Use proper test cleanup
Implement proper test assertions
Use proper test structure
Files:
packages/clerk-js/src/ui/components/OrganizationList/__tests__/OrganizationList.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsxpackages/clerk-js/src/ui/components/SubscriptionDetails/__tests__/SubscriptionDetails.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/InviteMembersPage.test.tsxpackages/clerk-js/src/ui/components/Checkout/__tests__/Checkout.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useCoreOrganization.test.tsxpackages/clerk-js/src/ui/components/PricingTable/__tests__/PricingTable.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useCoreOrganizationList.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationProfile.test.tsxpackages/clerk-js/src/ui/components/UserProfile/__tests__/UserProfile.test.tsxpackages/clerk-js/src/ui/components/Subscriptions/__tests__/SubscriptionsList.test.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/clerk-js/src/ui/components/OrganizationList/__tests__/OrganizationList.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsxpackages/clerk-js/src/ui/components/SubscriptionDetails/__tests__/SubscriptionDetails.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/InviteMembersPage.test.tsxpackages/clerk-js/src/ui/components/Checkout/__tests__/Checkout.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useCoreOrganization.test.tsxpackages/clerk-js/src/ui/components/PricingTable/__tests__/PricingTable.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useCoreOrganizationList.test.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationProfile.test.tsxpackages/clerk-js/src/ui/components/UserProfile/__tests__/UserProfile.test.tsxpackages/clerk-js/src/ui/components/Subscriptions/__tests__/SubscriptionsList.test.tsx
🧬 Code graph analysis (1)
packages/clerk-js/src/ui/components/PricingTable/__tests__/PricingTable.test.tsx (1)
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). (29)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (quickstart, chrome, 16)
- GitHub Check: Integration Tests (quickstart, chrome, 15)
- GitHub Check: Integration Tests (nextjs, chrome, 16)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (expo-web, chrome)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Integration Tests (elements, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Static analysis
- GitHub Check: Unit Tests (22, **)
- GitHub Check: Publish with pkg-pr-new
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (15)
packages/clerk-js/src/ui/components/UserProfile/__tests__/UserProfile.test.tsx (1)
116-117: LGTM!The assertions correctly verify that the billing API methods are called even when
hasPaidPlansis true, ensuring the component doesn't short-circuit the API calls based solely on configuration flags.packages/clerk-js/src/ui/hooks/__tests__/useCoreOrganization.test.tsx (1)
45-72: LGTM! Good test coverage improvement.Destructuring
fixturesand asserting that API methods are not invoked in the default state properly validates that resources are fetched lazily only when requested.packages/clerk-js/src/ui/components/PricingTable/__tests__/PricingTable.test.tsx (1)
1-1: LGTM!The import is correctly added and properly used in the test at line 438.
packages/clerk-js/src/ui/components/Checkout/__tests__/Checkout.test.tsx (1)
312-313: Remove the skip or provide clear tracking before merge; this test cannot be deferred without documentation.This is the only skipped test in the file. While the PR enables RQ variant support, test skips require either immediate fixes or formal tracking (issue link, detailed failure description in TODO). The vague TODO "Why is this failing?" doesn't help future developers.
The comparable
status: 'completed'test at line 396 passes with identical mock structure, suggesting the issue is specific to theneeds_confirmationstate handling under the RQ variant.Options:
- Fix it now: Investigate whether confirmation stage component logic or mock setup needs adjustment for RQ variant
- Track it formally: Replace TODO with issue link and specific failure symptom (e.g., "timeout waiting for element X" or "React Query retry causing assertion to fail")
- Remove entirely: If confirmation stage isn't required for this PR, remove the incomplete test and file a separate task
packages/clerk-js/src/ui/components/SubscriptionDetails/__tests__/SubscriptionDetails.test.tsx (2)
1-1283: Well-structured test suite with comprehensive coverage.The test suite demonstrates good practices:
- Proper use of React Testing Library queries and user interactions
- Well-isolated tests with explicit mocking
- Comprehensive coverage of different subscription states (active, past due, free trial, etc.)
- Clear test naming and assertions
12-34: ****The first test is correctly set up without the billing method mocks. The SubscriptionDetails component only calls
useSubscription(), which fetches subscription data via__experimental_useSubscription. The additional mocks forgetPlans,getStatements,getPaymentAttempts, andgetPaymentMethodsare not invoked by the SubscriptionDetails component during initialization or rendering, so their absence in the first test is not a problem. The four mocks present in the other tests appear to be either unnecessary for this component or copied from a different test context.Likely an incorrect or invalid review comment.
packages/clerk-js/src/ui/components/Subscriptions/__tests__/SubscriptionsList.test.tsx (2)
54-57: Good addition: Explicitly verifying data-fetching calls.These expectations ensure the component properly attempts to fetch billing data even when subscriptions are empty. This improves test coverage and makes the test intent clearer.
24-25: Clarify the skipped test and resolve before merging.The TODO comment is confusing—it states the test "passes with RQ and fails with SWR," suggesting RQ works correctly, yet the test remains skipped. The RQ implementation already includes proper
enabledflags (inuseSubscription.rq.tsxline 58:enabled: Boolean(user?.id && billingEnabled) && ...), so the concern about unwanted requests appears to be addressed.Before merging, verify one of the following:
- If RQ is working: Unskip the test and confirm it passes.
- If the test still fails: Document the specific failure reason in the TODO comment and track resolution separately.
Update the TODO to be explicit: clarify whether this is a known limitation of the test setup, a remaining RQ issue, or a case where the test expectations need adjustment.
packages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsx (1)
162-174: Verify the mock strategy change from one-time to persistent.The mock has been changed from
mockReturnValueOncetomockReturnValue, which means it will persist across multiple invocations instead of being consumed after the first call. Ensure this aligns with the component's actual behavior—ifgetMembershipsis called multiple times during the test lifecycle, this change is correct.packages/clerk-js/src/ui/components/OrganizationList/__tests__/OrganizationList.test.tsx (2)
48-66: Mock strategy change looks correct.The shift from
mockReturnValueOncetomockReturnValueforgetOrganizationMembershipsandgetOrganizationInvitationsaligns with the pattern used across other test files in this PR. This allows mocks to persist across multiple invocations, which is appropriate if these methods are called multiple times during the test lifecycle.Also applies to: 120-138, 159-164
345-345:setActivemock change is consistent.Changing
setActive.mockReturnValueOncetosetActive.mockReturnValueis consistent with the broader mock strategy update in this PR.Also applies to: 395-395, 426-426
packages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationProfile.test.tsx (2)
183-184: Good addition of explicit API call assertions.Adding explicit assertions to verify that
getSubscriptionandgetStatementsare called improves test clarity and ensures the billing APIs are invoked as expected when billing is enabled.
92-115: Re-enable test after implementing permission guard for billing API calls.The test correctly identifies a real issue: billing APIs (getSubscription, getStatements) are called unconditionally in useOrganizationProfileContext before any permission checks. The wrapper only controls UI visibility, not API execution.
To re-enable this test, add a permission check that prevents useSubscription() and useStatements() from being called when the user lacks
org:sys_billing:readororg:sys_billing:managepermissions. This ensures billing data is not fetched for unauthorized users.packages/clerk-js/src/ui/hooks/__tests__/useCoreOrganizationList.test.tsx (2)
108-109: Good addition of explicit API call verification.Adding
waitForexpectations to verify thatgetOrganizationInvitationsandgetOrganizationSuggestionsare called improves test reliability by ensuring these APIs are invoked during the test flow.
173-174: Investigate and re-enable skipped infinite fetch tests.The three "infinite fetch" tests in
packages/clerk-js/src/ui/hooks/__tests__/useCoreOrganizationList.test.tsx(at lines 173-174, 402-403, 582-583) are valid test cases that should be investigated and re-enabled.Verification confirms that:
- The
infiniteparameter is fully supported and properly implemented in the shared library'susePagesOrInfinitehook- Production code actively uses
infinite: truethroughorganizationListParamsfor all three collection types- The shared package has comprehensive passing tests for infinite mode, including data accumulation behavior during
fetchNext()calls- The clerk-js test structure mirrors the working tests in the shared package, suggesting the issue is test-specific rather than feature-related
Re-enabling these tests is important for maintaining test coverage of infinite scrolling behavior in organization list components, which is actively used in production.
packages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationProfile.test.tsx
Show resolved
Hide resolved
packages/clerk-js/src/ui/components/PricingTable/__tests__/PricingTable.test.tsx
Show resolved
Hide resolved
| fixtures.clerk.billing.getPlans.mockRejectedValue(null); | ||
| fixtures.clerk.billing.getStatements.mockRejectedValue(null); | ||
| fixtures.clerk.user.getPaymentMethods.mockRejectedValue(null); |
There was a problem hiding this comment.
Verify rejection with null is appropriate and consider extracting repeated setup.
Two concerns:
-
Non-standard rejection pattern:
mockRejectedValue(null)is unusual. Typically, Promise rejections use Error objects. If the component's error handling expects structured error objects, rejecting withnullcould mask bugs or lead to runtime errors when the code tries to access error properties. -
Code duplication: This three-line mock pattern is repeated in 6 different tests. Consider extracting it into a helper function or
beforeEachblock to improve maintainability.
Verification needed:
#!/bin/bash
# Check how SubscriptionsList handles errors from these methods
rg -A 10 -B 2 'getPlans|getStatements|getPaymentMethods' --type=tsx --type=ts -g '**/SubscriptionsList.tsx' -g '!**/*.test.*'Suggested refactor:
// Add a helper function at the top of the describe block
const mockBillingDataAsUnavailable = (fixtures: any) => {
fixtures.clerk.billing.getPlans.mockRejectedValue(null);
fixtures.clerk.billing.getStatements.mockRejectedValue(null);
fixtures.clerk.user.getPaymentMethods.mockRejectedValue(null);
};
// Then use it in each test:
mockBillingDataAsUnavailable(fixtures);Also applies to: 66-68, 178-180, 244-246, 275-277, 412-414
🤖 Prompt for AI Agents
In
packages/clerk-js/src/ui/components/Subscriptions/__tests__/SubscriptionsList.test.tsx
around lines 31-33, the tests call mockRejectedValue(null) for three fixtures
which is non-standard and may hide issues when the component expects Error
objects; replace null rejections with Error instances (e.g., new Error('billing
unavailable')) to match realistic rejection shapes, and eliminate duplication by
extracting the three mockRejectedValue calls into a shared helper function or a
beforeEach that accepts the fixtures so the pattern is reused across the six
tests mentioned (lines 31-33, 66-68, 178-180, 244-246, 275-277, 412-414).
packages/clerk-js/src/ui/components/UserProfile/__tests__/UserProfile.test.tsx
Show resolved
Hide resolved
packages/clerk-js/src/ui/hooks/__tests__/useCoreOrganization.test.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx (1)
1-1: Well-structured test infrastructure for dual-mode testing.The QueryClient mock setup is appropriate for deterministic tests (disabled retries, infinite staleTime), and clearing state in
beforeEachensures proper test isolation.Optional: Simplify the getter definition if verification isn't needed.
Line 53 wraps the getter in
vi.fn(), but there are no assertions verifying this spy. If you don't plan to assert that__internal_queryClientwas accessed, you can simplify it:Object.defineProperty(mockClerk, '__internal_queryClient', { - get: vi.fn(() => defaultQueryClient), + get: () => defaultQueryClient, configurable: true, });Similarly, the
onandoffmocks (lines 33-34) are not verified in this test file—confirm whether they're needed for other test files or if coverage is missing here.Also applies to: 33-55, 76-76
📜 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(6 hunks)packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts(4 hunks)packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx(6 hunks)packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx(5 hunks)packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx(1 hunks)packages/shared/src/react/hooks/usePagesOrInfinite.swr.tsx(5 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.tsxpackages/shared/src/react/hooks/__tests__/useSubscription.spec.tsxpackages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsxpackages/shared/src/react/hooks/usePagesOrInfinite.rq.tsxpackages/shared/src/react/hooks/usePagesOrInfinite.swr.tsxpackages/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/__tests__/usePlans.spec.tsxpackages/shared/src/react/hooks/__tests__/useSubscription.spec.tsxpackages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsxpackages/shared/src/react/hooks/usePagesOrInfinite.rq.tsxpackages/shared/src/react/hooks/usePagesOrInfinite.swr.tsxpackages/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/__tests__/usePlans.spec.tsxpackages/shared/src/react/hooks/__tests__/useSubscription.spec.tsxpackages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsxpackages/shared/src/react/hooks/usePagesOrInfinite.rq.tsxpackages/shared/src/react/hooks/usePagesOrInfinite.swr.tsxpackages/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/__tests__/usePlans.spec.tsxpackages/shared/src/react/hooks/__tests__/useSubscription.spec.tsxpackages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsxpackages/shared/src/react/hooks/usePagesOrInfinite.rq.tsxpackages/shared/src/react/hooks/usePagesOrInfinite.swr.tsxpackages/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
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor 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 assertionsfor literal types:as const
Usesatisfiesoperator 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 ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor 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.tsxpackages/shared/src/react/hooks/__tests__/useSubscription.spec.tsxpackages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsxpackages/shared/src/react/hooks/usePagesOrInfinite.rq.tsxpackages/shared/src/react/hooks/usePagesOrInfinite.swr.tsxpackages/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/__tests__/usePlans.spec.tsxpackages/shared/src/react/hooks/__tests__/useSubscription.spec.tsxpackages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsxpackages/shared/src/react/hooks/usePagesOrInfinite.rq.tsxpackages/shared/src/react/hooks/usePagesOrInfinite.swr.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.tsxpackages/shared/src/react/hooks/__tests__/useSubscription.spec.tsxpackages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsxpackages/shared/src/react/hooks/__tests__/usePagesOrInfinite.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/__tests__/usePlans.spec.tsxpackages/shared/src/react/hooks/__tests__/useSubscription.spec.tsxpackages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsxpackages/shared/src/react/hooks/usePagesOrInfinite.rq.tsxpackages/shared/src/react/hooks/usePagesOrInfinite.swr.tsxpackages/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/__tests__/usePlans.spec.tsxpackages/shared/src/react/hooks/__tests__/useSubscription.spec.tsxpackages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsxpackages/shared/src/react/hooks/usePagesOrInfinite.rq.tsxpackages/shared/src/react/hooks/usePagesOrInfinite.swr.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.tsxpackages/shared/src/react/hooks/__tests__/useSubscription.spec.tsxpackages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsxpackages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
| numberOrgFn => { | ||
| if (triggerInfinite) { | ||
| const next = typeof numberOrgFn === 'function' ? (numberOrgFn as (n: number) => number)(page) : numberOrgFn; | ||
| const targetCount = Math.max(0, next); | ||
| const cachedData = queryClient.getQueryData<{ pages?: Array<ClerkPaginatedResponse<any>> }>(infiniteQueryKey); | ||
| const pages = infiniteQuery.data?.pages ?? cachedData?.pages ?? []; | ||
| const currentCount = pages.length; | ||
| const toFetch = targetCount - currentCount; | ||
| if (toFetch > 0) { | ||
| void infiniteQuery.fetchNextPage({ cancelRefetch: false }); | ||
| } | ||
| return; | ||
| } |
There was a problem hiding this comment.
Restore multi-page fetch support in infinite mode.
fetchPage computes toFetch but always calls fetchNextPage just once. Passing a numeric target greater than the current page count (e.g. fetchPage(5)) should load four additional pages, but this implementation only loads one, leaving page stuck at 2. The SWR version used setSize to jump directly to the requested size, so this is a regression for consumers that rely on arbitrary jumps. Please loop (or otherwise chain) fetchNextPage until the requested count is reached, and also handle shrinking when toFetch < 0, so the React Query version matches the existing contract.
🤖 Prompt for AI Agents
In packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx around lines 143 to
155, the infinite-mode fetchPage computes how many pages to reach targetCount
but only calls fetchNextPage once; change this to repeatedly fetch until current
pages length equals targetCount (loop or sequentially await/then fetchNextPage
while toFetch > 0, using cancelRefetch: false) so calling fetchPage(5) loads
enough pages; also handle shrinking when toFetch < 0 by using
queryClient.setQueryData(infiniteQueryKey, prev => ({ ...prev, pages:
(prev?.pages ?? []).slice(0, targetCount) })) to truncate pages to targetCount,
and ensure you read cachedData/pages after each update to recompute remaining
toFetch to avoid infinite loops.
This comment has been minimized.
This comment has been minimized.
# Conflicts: # packages/shared/src/react/hooks/usePagesOrInfinite.swr.tsx
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 (2)
packages/clerk-js/src/ui/components/Checkout/__tests__/Checkout.test.tsx(2 hunks)packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (13)
packages/clerk-js/src/ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/clerk-js-ui.mdc)
packages/clerk-js/src/ui/**/*.{ts,tsx}: Element descriptors should always be camelCase
Use element descriptors in UI components to enable consistent theming and styling via appearance.elements
Element descriptors should generate unique, stable CSS classes for theming
Element descriptors should handle state classes (e.g., cl-loading, cl-active, cl-error, cl-open) automatically based on component state
Do not render hard-coded values; all user-facing strings must be localized using provided localization methods
Use the useLocalizations hook and localizationKeys utility for all text and error messages
Use the styled system (sx prop, theme tokens, responsive values) for custom component styling
Use useCardState for card-level state, useFormState for form-level state, and useLoadingStatus for loading states
Always use handleError utility for API errors and use translateError for localized error messages
Use useFormControl for form field state, implement proper validation, and handle loading and error states in forms
Use localization keys for all form labels and placeholders
Use element descriptors for consistent styling and follow the theme token system
Use the Card and FormContainer patterns for consistent UI structure
Files:
packages/clerk-js/src/ui/components/Checkout/__tests__/Checkout.test.tsx
**/*.{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/clerk-js/src/ui/components/Checkout/__tests__/Checkout.test.tsxpackages/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/clerk-js/src/ui/components/Checkout/__tests__/Checkout.test.tsxpackages/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/clerk-js/src/ui/components/Checkout/__tests__/Checkout.test.tsxpackages/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/clerk-js/src/ui/components/Checkout/__tests__/Checkout.test.tsxpackages/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
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor 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 assertionsfor literal types:as const
Usesatisfiesoperator 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 ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor 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/clerk-js/src/ui/components/Checkout/__tests__/Checkout.test.tsxpackages/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/clerk-js/src/ui/components/Checkout/__tests__/Checkout.test.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/clerk-js/src/ui/components/Checkout/__tests__/Checkout.test.tsxpackages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
packages/{clerk-js,elements,themes}/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Visual regression testing should be performed for UI components.
Files:
packages/clerk-js/src/ui/components/Checkout/__tests__/Checkout.test.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/clerk-js/src/ui/components/Checkout/__tests__/Checkout.test.tsxpackages/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/clerk-js/src/ui/components/Checkout/__tests__/Checkout.test.tsx
**/*.test.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.test.{jsx,tsx}: Use React Testing Library
Test component behavior, not implementation
Use proper test queries
Implement proper test isolation
Use proper test coverage
Test component interactions
Use proper test data
Implement proper test setup
Use proper test cleanup
Implement proper test assertions
Use proper test structure
Files:
packages/clerk-js/src/ui/components/Checkout/__tests__/Checkout.test.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/clerk-js/src/ui/components/Checkout/__tests__/Checkout.test.tsxpackages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
🧬 Code graph analysis (1)
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts (2)
packages/shared/src/utils/createDeferredPromise.ts (1)
createDeferredPromise(11-19)packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx (1)
usePagesOrInfinite(14-287)
⏰ 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)
packages/clerk-js/src/ui/components/Checkout/__tests__/Checkout.test.tsx (2)
320-323: LGTM - Improves test isolation.The explicit mock for
getPaymentMethodsensures predictable test behavior and prevents potential timing issues with unmocked API calls. Returning an empty array is appropriate for this free trial scenario.
1042-1052: LGTM - Enhances test reliability.The restructured test flow explicitly waits for the "Add payment method" button to be visible before interacting with it, reducing potential flakiness. The two-stage approach (waiting for the heading, then the button) makes the test's intent clearer and aligns with React Testing Library best practices.
| const capturedStates: Array<{ isLoading: boolean; isFetching: boolean }> = []; | ||
|
|
||
| // Capture states during transition | ||
| let iterations = 0; | ||
| while (iterations < 10 && result.current.data[0]?.id !== 'item-B') { | ||
| capturedStates.push({ | ||
| isLoading: result.current.isLoading, | ||
| isFetching: result.current.isFetching, | ||
| }); | ||
| await new Promise(resolve => setTimeout(resolve, 10)); | ||
| iterations++; | ||
| } | ||
|
|
||
| // Wait for new data to settle | ||
| await waitFor(() => expect(result.current.data).toEqual([{ id: 'item-B' }])); | ||
| expect(result.current.isLoading).toBe(false); | ||
|
|
||
| // Document that during transition, we may see loading/fetching states | ||
| // This is expected RQ behavior and tests must account for it | ||
| expect(fetcher).toHaveBeenCalledTimes(2); | ||
| expect(fetcher).toHaveBeenCalledWith(expect.objectContaining({ filter: 'B' })); | ||
| }); |
There was a problem hiding this comment.
Add assertions to validate the recorded loading/fetching states.
The test name promises to prove that isLoading may briefly flip during a key change, but the body never asserts anything about capturedStates. As written, the test would still pass if isLoading stayed false the entire time, so it won’t catch regressions. Please assert against the collected states (e.g., ensure at least one entry has isLoading === true or isFetching === true) or drop the unused tracking. Otherwise this scenario provides zero signal.
@@
- while (iterations < 10 && result.current.data[0]?.id !== 'item-B') {
- capturedStates.push({
- isLoading: result.current.isLoading,
- isFetching: result.current.isFetching,
- });
+ while (iterations < 10 && result.current.data[0]?.id !== 'item-B') {
+ capturedStates.push({
+ isLoading: result.current.isLoading,
+ isFetching: result.current.isFetching,
+ });
@@
- // Document that during transition, we may see loading/fetching states
- // This is expected RQ behavior and tests must account for it
+ expect(capturedStates.some(state => state.isLoading)).toBe(true);
+ expect(capturedStates.some(state => state.isFetching)).toBe(true);
+ // Document that during transition, we may see loading/fetching states
+ // This is expected RQ behavior and tests must account for it🤖 Prompt for AI Agents
In packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts around
lines 755 to 776, the test records capturedStates during a transition but never
asserts on them; add assertions to validate the recorded loading/fetching
behavior (or remove the tracking). Specifically, assert that
capturedStates.length > 0 and that at least one entry has isLoading === true or
isFetching === true (or both) to ensure the test actually verifies the transient
state flip promised by the test name; keep the existing waits and fetcher
expectations unchanged.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts (1)
756-777: Add assertions to validate the recorded loading/fetching states.The test collects
capturedStatesduring the transition but never asserts on them. Without assertions, the test provides no verification thatisLoadingorisFetchingactually flip during the key change, making it ineffective at catching regressions.Apply this diff to add assertions:
// Wait for new data to settle await waitFor(() => expect(result.current.data).toEqual([{ id: 'item-B' }])); expect(result.current.isLoading).toBe(false); + // Verify that we captured states and that loading/fetching occurred + expect(capturedStates.length).toBeGreaterThan(0); + expect(capturedStates.some(state => state.isLoading || state.isFetching)).toBe(true); + // Document that during transition, we may see loading/fetching states // This is expected RQ behavior and tests must account for it
🧹 Nitpick comments (2)
packages/shared/src/react/hooks/useSubscription.rq.tsx (1)
60-60: TODO is appropriately documented for future enhancement.The
keepPreviousDataoption (orplaceholderData: keepPreviousDatain React Query v5) prevents UI flicker during refetches by keeping the previous data visible while new data loads. This is good to track for improving UX, especially for subscription data that updates infrequently.If you'd like help implementing this, I can provide a code snippet that adds the appropriate option based on your React Query version.
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts (1)
328-328: Remove or document the commented-out assertions.Both lines contain the same commented assertion
expect(result.current.isLoading).toBe(false);. If these assertions are intentionally skipped due to uncertain React Query behavior during transitions, consider:
- Adding an inline comment explaining why the assertion is skipped
- Removing them entirely if they're not needed
Otherwise, uncomment them if the behavior is now deterministic.
Apply this diff if removing is preferred:
- // expect(result.current.isLoading).toBe(false);Also applies to: 364-364
📜 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__/usePagesOrInfinite.spec.ts(5 hunks)packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx(5 hunks)packages/shared/src/react/hooks/useSubscription.rq.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__/useSubscription.spec.tsxpackages/shared/src/react/hooks/useSubscription.rq.tsxpackages/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/__tests__/useSubscription.spec.tsxpackages/shared/src/react/hooks/useSubscription.rq.tsxpackages/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/__tests__/useSubscription.spec.tsxpackages/shared/src/react/hooks/useSubscription.rq.tsxpackages/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/__tests__/useSubscription.spec.tsxpackages/shared/src/react/hooks/useSubscription.rq.tsxpackages/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
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor 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 assertionsfor literal types:as const
Usesatisfiesoperator 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 ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor 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__/useSubscription.spec.tsxpackages/shared/src/react/hooks/useSubscription.rq.tsxpackages/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/__tests__/useSubscription.spec.tsxpackages/shared/src/react/hooks/useSubscription.rq.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__/useSubscription.spec.tsxpackages/shared/src/react/hooks/__tests__/usePagesOrInfinite.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/__tests__/useSubscription.spec.tsxpackages/shared/src/react/hooks/useSubscription.rq.tsxpackages/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/__tests__/useSubscription.spec.tsxpackages/shared/src/react/hooks/useSubscription.rq.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__/useSubscription.spec.tsxpackages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts
🧬 Code graph analysis (2)
packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx (1)
packages/clerk-js/src/core/query-core.ts (1)
QueryClient(3-3)
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts (2)
packages/shared/src/utils/createDeferredPromise.ts (1)
createDeferredPromise(11-19)packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx (1)
usePagesOrInfinite(14-287)
⏰ 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). (29)
- GitHub Check: Integration Tests (expo-web, chrome)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (quickstart, chrome, 16)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (quickstart, chrome, 15)
- GitHub Check: Integration Tests (nextjs, chrome, 16)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Integration Tests (elements, chrome)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Publish with pkg-pr-new
- GitHub Check: Unit Tests (22, **)
- GitHub Check: Static analysis
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (10)
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts (4)
1-49: Well-structured test infrastructure for React Query integration.The QueryClient configuration is appropriate for deterministic testing (disabled retries, infinite staleTime, disabled auto-refetches), and the mock setup properly isolates the tests from external dependencies. The
beforeEachhook ensures proper test isolation by clearing mocks and the query client cache.
338-372: Good test coverage forkeepPreviousData: falsebehavior.This test appropriately validates that data is cleared (empty array) during page transitions when
keepPreviousDatais false, complementing thekeepPreviousData: truetest above.
779-842: Excellent test coverage for render stability.These tests appropriately validate that
isLoadingremains stable (false) after initial data loads when re-rendering with unchanged parameters, covering both pagination and infinite modes. This is critical for preventing unnecessary loading indicators in production.
844-899: Outstanding documentation test forisLoadingvsisFetchingsemantics.This test clearly documents the critical distinction between
isLoading(true only on first fetch without data) andisFetching(true whenever query is running). The inline comments at lines 864-887 provide valuable guidance for test authors and future maintainers.packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx (6)
1-1: LGTM!The QueryClient import is correctly added to support React Query integration in tests.
33-34: LGTM!Event listener mocks are properly set up for test isolation.
76-76: LGTM!Clearing the query client state in
beforeEachensures proper test isolation and prevents cache state from leaking between tests.
127-138: LGTM!The conditional test paths for RQ and SWR are correctly implemented. The RQ path asserts data becomes undefined on sign-out, while the SWR path asserts the fetching state transition and null data result, which accurately reflects the different behaviors of each library.
155-167: LGTM!The conditional test assertions for the keepPreviousData scenario are correctly implemented and mirror the sign-out test pattern above, ensuring consistent behavior verification across both RQ and SWR paths.
37-55: The verification confirms that thedefaultQueryClientstructure is correct. The expected type for__internal_queryClientis{ __tag: 'clerk-rq-client'; client: QueryClient } | undefined, which matches exactly what the test setup creates. The test mock properly mirrors the actual implementation, and no changes are needed.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts (1)
735-756: Add assertions on captured states as previously requested.The past review comment correctly identifies that
capturedStatesis collected but never asserted. Without assertions, the test provides no signal if the transient loading behavior changes.Apply this diff to add the missing assertions:
// Wait for new data to settle await waitFor(() => expect(result.current.data).toEqual([{ id: 'item-B' }])); expect(result.current.isLoading).toBe(false); + // Verify we captured some transitional states + expect(capturedStates.length).toBeGreaterThan(0); + // At least one state during transition should show loading or fetching + const hadTransition = capturedStates.some(s => s.isLoading || s.isFetching); + expect(hadTransition).toBe(true); + // Document that during transition, we may see loading/fetching states // This is expected RQ behavior and tests must account for it expect(fetcher).toHaveBeenCalledTimes(2);
🧹 Nitpick comments (7)
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx (2)
59-61: Recreating mocks in beforeEach may hide mutation issues.While this ensures test isolation, recreating
mockUserandmockOrganizationon each test may hide issues where tests inadvertently mutate shared state. Consider whether tests should explicitly recreate mocks only when needed.
163-186: Consider consolidating with existing orgId forwarding test.This test is nearly identical to the test at lines 149-161, except it uses
infinite: true. The differences are:
- Line 172: adds
infinite: true- Line 164-169: mocks fetcher to return data
- Line 185: asserts data length
Consider parameterizing the existing test or adding a focused assertion for the infinite-mode-specific behavior rather than duplicating the entire test.
Apply this approach to reduce duplication:
it.each([ { infinite: false }, { infinite: true }, ])('when for=organization orgId should be forwarded to fetcher (infinite: $infinite)', async ({ infinite }) => { if (infinite) { fetcherMock.mockImplementation((params: any) => Promise.resolve({ data: Array.from({ length: params.pageSize }, (_, i) => ({ id: `item-${params.initialPage}-${i}` })), total_count: 20, }), ); } const { result } = renderHook( () => useDummyAuth({ initialPage: 1, pageSize: 4, for: 'organization', infinite } as any), { wrapper }, ); await waitFor(() => expect(result.current.isLoading).toBe(false)); expect(useFetcherMock).toHaveBeenCalledWith('organization'); expect(fetcherMock.mock.calls[0][0]).toStrictEqual({ initialPage: 1, pageSize: 4, orgId: 'org_1', }); if (infinite) { expect(result.current.data.length).toBe(4); } });packages/shared/src/react/hooks/__tests__/mocks/clerk.ts (2)
27-55: Add type constraints to mock factories.The extensive use of
anyreduces type safety. Consider defining interfaces for the mock shapes:interface MockClerkConfig { billing?: any; queryClient?: ReturnType<typeof createMockQueryClient>; loaded?: boolean; __unstable__environment?: any; [key: string]: any; } export function createMockClerk(overrides: MockClerkConfig = {}) { const queryClient = overrides.queryClient || createMockQueryClient(); // ... rest of implementation }This provides better IntelliSense and catches misuse while maintaining flexibility for test-specific overrides.
As per coding guidelines
57-63: Add JSDoc comments to test utility functions.These factories are reused across multiple test files. Add JSDoc comments documenting:
- Purpose of each factory
- Available override options
- Return type structure
/** * Creates a mock user object for testing. * @param overrides - Properties to override on the default mock user * @returns Mock user with id 'user_1' by default */ export function createMockUser(overrides: any = {}) { return { id: 'user_1', ...overrides }; }As per coding guidelines
packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx (2)
51-55: Recreating mocks in beforeEach may hide mutation issues.Similar to the pattern in
createBillingPaginatedHook.spec.tsx, recreatingmockUserandmockOrganizationin every test may mask issues where tests inadvertently mutate shared state. Consider whether tests should only recreate mocks when explicitly needed for their scenario.
106-113: Conditional test logic adds maintenance complexity.The
if (__CLERK_USE_RQ__)branching creates dual execution paths. As noted increateBillingPaginatedHook.spec.tsx, consider:
- Separating RQ and SWR test suites
- Using parameterized tests
- Extracting conditional expectations into helper functions
This pattern appears in multiple test files (lines 106-113, 134-142), suggesting it could benefit from a shared abstraction.
Based on learnings
.github/workflows/ci.yml (1)
289-289: Artifact naming now includes CLERK_USE_RQ context for better traceability.Line 289 uses
matrix.clerk-use-rq || 'default'to label artifacts with the flag state; line 490 uses conditional formatting to append-rq{value}suffix. This enhances debuggability. However, consider consistent formatting: line 290 produces names like-node-22-false,-node-22-true, or-node-22-default, while line 490 produces-rqtrueor-rqfalse(no hyphen between prefix and value). Minor nitpick, but-rq-true/-rq-falsewould be more readable.If more readable naming is preferred, update line 490 format:
-name: playwright-traces-${{ github.run_id }}-${{ github.run_attempt }}-${{ matrix.test-name }}${{ matrix.next-version && format('-next{0}', matrix.next-version) || '' }}${{ matrix.clerk-use-rq && format('-rq{0}', matrix.clerk-use-rq) || '' }} +name: playwright-traces-${{ github.run_id }}-${{ github.run_attempt }}-${{ matrix.test-name }}${{ matrix.next-version && format('-next{0}', matrix.next-version) || '' }}${{ matrix.clerk-use-rq && format('-rq-{0}', matrix.clerk-use-rq) || '' }}Also applies to: 490-490
📜 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)
.github/workflows/ci.yml(7 hunks)packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx(4 hunks)packages/shared/src/react/hooks/__tests__/mocks/clerk.ts(1 hunks)packages/shared/src/react/hooks/__tests__/mocks/index.ts(1 hunks)packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts(5 hunks)packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx(5 hunks)packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx(5 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/shared/src/react/hooks/tests/mocks/index.ts
🧰 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__/mocks/clerk.tspackages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.tspackages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsxpackages/shared/src/react/hooks/__tests__/useSubscription.spec.tsxpackages/shared/src/react/hooks/__tests__/usePlans.spec.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__/mocks/clerk.tspackages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.tspackages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsxpackages/shared/src/react/hooks/__tests__/useSubscription.spec.tsxpackages/shared/src/react/hooks/__tests__/usePlans.spec.tsx
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/shared/src/react/hooks/__tests__/mocks/clerk.tspackages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.tspackages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsxpackages/shared/src/react/hooks/__tests__/useSubscription.spec.tsxpackages/shared/src/react/hooks/__tests__/usePlans.spec.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__/mocks/clerk.tspackages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.tspackages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsxpackages/shared/src/react/hooks/__tests__/useSubscription.spec.tsxpackages/shared/src/react/hooks/__tests__/usePlans.spec.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
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor 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 assertionsfor literal types:as const
Usesatisfiesoperator 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 ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor 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__/mocks/clerk.tspackages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.tspackages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsxpackages/shared/src/react/hooks/__tests__/useSubscription.spec.tsxpackages/shared/src/react/hooks/__tests__/usePlans.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__/mocks/clerk.tspackages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.tspackages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsxpackages/shared/src/react/hooks/__tests__/useSubscription.spec.tsxpackages/shared/src/react/hooks/__tests__/usePlans.spec.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__/mocks/clerk.tspackages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.tspackages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsxpackages/shared/src/react/hooks/__tests__/useSubscription.spec.tsxpackages/shared/src/react/hooks/__tests__/usePlans.spec.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__/usePagesOrInfinite.spec.tspackages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsxpackages/shared/src/react/hooks/__tests__/useSubscription.spec.tsxpackages/shared/src/react/hooks/__tests__/usePlans.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__/createBillingPaginatedHook.spec.tsxpackages/shared/src/react/hooks/__tests__/useSubscription.spec.tsxpackages/shared/src/react/hooks/__tests__/usePlans.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__/createBillingPaginatedHook.spec.tsxpackages/shared/src/react/hooks/__tests__/useSubscription.spec.tsxpackages/shared/src/react/hooks/__tests__/usePlans.spec.tsx
.github/workflows/*.yml
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
CI/CD must be configured with GitHub Actions.
Files:
.github/workflows/ci.yml
🧬 Code graph analysis (4)
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts (2)
packages/shared/src/react/hooks/__tests__/mocks/clerk.ts (2)
createMockQueryClient(7-22)createMockClerk(27-55)packages/shared/src/utils/createDeferredPromise.ts (1)
createDeferredPromise(11-19)
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx (1)
packages/shared/src/react/hooks/__tests__/mocks/clerk.ts (4)
createMockUser(57-59)createMockOrganization(61-63)createMockQueryClient(7-22)createMockClerk(27-55)
packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx (1)
packages/shared/src/react/hooks/__tests__/mocks/clerk.ts (4)
createMockUser(57-59)createMockOrganization(61-63)createMockQueryClient(7-22)createMockClerk(27-55)
packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx (2)
packages/shared/src/react/hooks/__tests__/mocks/clerk.ts (4)
createMockUser(57-59)createMockOrganization(61-63)createMockQueryClient(7-22)createMockClerk(27-55)packages/shared/src/react/hooks/usePlans.tsx (1)
usePlans(8-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 (16)
packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx (3)
5-8: LGTM! Mock factory imports are clean.The migration to centralized mock factories improves test maintainability.
21-28: LGTM! Shared query client setup enables test isolation.The
defaultQueryClientis properly wired tomockClerkand will be cleared between tests (line 49), ensuring clean state.
183-204: LGTM! Test correctly verifies data persistence for unauthenticated hooks.The test properly captures initial data, simulates sign-out by nulling
mockUser.id, and asserts that data persists. This aligns with the expected behavior for hooks withunauthenticated: trueoption.packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx (2)
50-55: Good addition of default mock implementation.Setting a default empty response in
beforeEachprevents undefined behavior when tests don't explicitly mock the fetcher.
277-286: Conditional test logic adds complexity.The
if (__CLERK_USE_RQ__)branching creates dual execution paths, making tests harder to maintain and reason about. Consider:
- Splitting into separate test files for RQ and SWR implementations
- Using parameterized tests with
it.each- Extracting conditional expectations into helper functions
Based on learnings
packages/shared/src/react/hooks/__tests__/mocks/clerk.ts (1)
7-22: LGTM! Query client configuration is appropriate for tests.The configuration disables retry, sets infinite staleTime, and prevents automatic refetches—all good defaults for deterministic testing.
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts (3)
9-28: LGTM! Consistent mock setup with other test files.The global mock setup with
defaultQueryClientandbeforeEachcleanup ensures test isolation.
317-351: LGTM! Test correctly verifies non-keepPreviousData behavior.The test properly asserts that data becomes empty (line 345) during page transitions when
keepPreviousData: false, contrasting with the existing test at lines 281-315 where data is retained.
823-878: LGTM! Excellent documentation of isLoading vs isFetching semantics.This test clearly demonstrates the difference between
isLoading(true only on first fetch) andisFetching(true on any fetch). The inline comments make this critical distinction explicit for future test authors.packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx (1)
19-34: LGTM! Consistent mock setup.The
defaultQueryClientis properly created and wired tomockClerk, and will be cleared inbeforeEach(line 55)..github/workflows/ci.yml (6)
213-230: Matrix expansion for CLERK_USE_RQ testing is well-structured.The unit-test matrix selectively introduces RQ variants for @clerk/shared and @clerk/clerk-js while running the full suite without the flag. The
run-namefield improves visibility in logs. The strategy of testing both modes for core packages while leaving other packages in the baseline run is reasonable.
273-273: CLERK_USE_RQ propagated consistently across test environments.Env var is set in both unit-tests and integration-tests steps. For matrix entries without
clerk-use-rqdefined, the interpolation yields an empty string; ensure downstream code and build scripts handle this gracefully as default (non-RQ) behavior.Does the codebase check CLERK_USE_RQ with a conditional like
if CLERK_USE_RQ == 'true'(which would safely ignore empty/undefined) or does it depend on the variable being set?Also applies to: 481-481
278-278: Typedoc gating refined to main test run only.The condition now checks both
node-version == 22andtest-filter == "**", ensuring Typedoc tests run only on the full suite run and not on per-package matrix variants. This aligns with the comment and reduces redundant checks.
330-346: Integration test matrix strategically includes CLERK_USE_RQ variants.The matrix adds RQ testing for billing and nextjs (Next.js 15 + 16), leaving other test suites (generic, express, astro, etc.) and Next.js 14 without RQ variants. This focused approach keeps CI time reasonable while covering critical paths. Each variant (false/true) is explicitly defined.
421-421: Verdaccio publish command propagates CLERK_USE_RQ.The command includes
CLERK_USE_RQ=${{ matrix.clerk-use-rq }}inline. For matrix entries withoutclerk-use-rqdefined, this interpolates to an empty string, which should resolve to default (non-RQ) behavior. Verify that the pnpm/turbo build invocation handles empty or unset CLERK_USE_RQ safely.For matrix entries that don't define
clerk-use-rq, confirm thatCLERK_USE_RQ=(empty) is the intended default and that build/publish scripts don't fail or produce unexpected behavior when the env var is empty.
250-260: All claims verified—code is correctly implemented.The filter syntax
--filter=@clerk/shared^...selects all dependencies of @clerk/shared but excludes @clerk/shared itself, which is the correct turbo pattern for this two-step rebuild approach. The conditions are properly applied (with additionalsteps.task-status.outputs.affected == '1'check in the integration test section), and positioning before test execution is correct.
| // Add query client as getter if not already set | ||
| if (!Object.getOwnPropertyDescriptor(mockClerk, '__internal_queryClient')) { | ||
| Object.defineProperty(mockClerk, '__internal_queryClient', { | ||
| get: vi.fn(() => queryClient), | ||
| configurable: true, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the need for conditional property definition.
The check if (!Object.getOwnPropertyDescriptor(mockClerk, '__internal_queryClient')) suggests overrides might already define this property. Is this intentional? If overrides should be able to provide a custom getter, document this behavior. Otherwise, always define the getter to simplify the logic.
🏁 Script executed:
#!/bin/bash
# Search for tests that pass __internal_queryClient in overrides
rg -n -C3 'createMockClerk.*__internal_queryClient' --type=tsLength of output: 0
---
🏁 Script executed:
```shell
#!/bin/bash
# Examine the file to understand createMockClerk function and how overrides works
cat packages/shared/src/react/hooks/__tests__/mocks/clerk.ts | head -100
Length of output: 1506
🏁 Script executed:
#!/bin/bash
# Search for any calls to createMockClerk in test files
rg -n 'createMockClerk\s*\(' --type=ts -A2 -B2Length of output: 2452
🏁 Script executed:
#!/bin/bash
# Search for any references to __internal_queryClient in the entire codebase
rg -n '__internal_queryClient' --type=tsLength of output: 1181
Remove the conditional check—always define the getter for consistency with the real Clerk API.
The check if (!Object.getOwnPropertyDescriptor(mockClerk, '__internal_queryClient')) allows overrides to provide a custom __internal_queryClient, but no tests use this capability. Since the real Clerk class (packages/clerk-js/src/core/clerk.ts:242) always exposes __internal_queryClient as a getter, the mock should do the same unconditionally. This simplifies the code and ensures predictable behavior across all test scenarios.
🤖 Prompt for AI Agents
In packages/shared/src/react/hooks/__tests__/mocks/clerk.ts around lines 46-52,
remove the surrounding if-check and always define the '__internal_queryClient'
getter on mockClerk to mirror the real Clerk API; replace the conditional block
with an unconditional Object.defineProperty call that sets get: vi.fn(() =>
queryClient) and configurable: true (redefining unconditionally to ensure
consistent behavior across tests).
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/clerk-rq/useBaseQuery.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{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/clerk-rq/useBaseQuery.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/clerk-rq/useBaseQuery.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/shared/src/react/clerk-rq/useBaseQuery.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/clerk-rq/useBaseQuery.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
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor 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 assertionsfor literal types:as const
Usesatisfiesoperator 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 ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor 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/clerk-rq/useBaseQuery.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/clerk-rq/useBaseQuery.ts
⏰ 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
| throw new Error('test'); | ||
|
|
There was a problem hiding this comment.
Remove the test throw statement before merging.
This unconditional throw new Error('test') breaks the entire useBaseQuery hook and makes all subsequent code unreachable. Every component using this hook (or any hook that depends on it) will throw an error at runtime, completely blocking the React Query integration.
While this may be intentional for a WIP branch, it must be removed before the PR is merged.
Apply this diff to remove the debug statement:
- throw new Error('test');
-
// note: this must be called before useSyncExternalStore
const result = observer.getOptimisticResult(defaultedOptions);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| throw new Error('test'); | |
| // note: this must be called before useSyncExternalStore | |
| const result = observer.getOptimisticResult(defaultedOptions); |
🤖 Prompt for AI Agents
In packages/shared/src/react/clerk-rq/useBaseQuery.ts around lines 39-40, there
is an unconditional debug throw ("throw new Error('test')") that breaks the
hook; remove that throw statement so execution continues normally, then run
lint/tests and a quick dev build to ensure no other leftover debug code remains
before merging.
This comment has been minimized.
This comment has been minimized.
d10f7d3 to
8475329
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/ci.yml (2)
210-225: Full test suite never runs withCLERK_USE_RQ=true.The unit-tests matrix runs individual packages (@clerk/shared, @clerk/clerk-js) with both
clerk-use-rq: "false"and"true", but the full suite (line 210-212) only runs withclerk-use-rq: "false". This means end-to-end cross-package compatibility is never validated when CLERK_USE_RQ is active.Consider adding a full-suite variant with
clerk-use-rq: "true"to ensure RQ support doesn't break inter-package integration, or document if this is intentional (e.g., to avoid CI bloat).
304-341: Integration tests matrix relies on implicitundefinedforclerk-use-rqin base matrix items.The base matrix items (lines 304–322) do not define
clerk-use-rq, leaving itundefinedfor most test combinations. While conditionals likematrix.clerk-use-rq == 'true'will correctly evaluate tofalsewhen undefined, this is implicit and fragile.Consider explicitly setting a default value like
clerk-use-rq: 'false'in the base matrix or using a more explicit fallback pattern to improve clarity and maintainability.
📜 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)
.github/workflows/ci.yml(8 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
.github/workflows/*.yml
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
CI/CD must be configured with GitHub Actions.
Files:
.github/workflows/ci.yml
⏰ 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). (27)
- GitHub Check: Integration Tests (CLERK_USE_RQ=true)
- GitHub Check: Integration Tests
- GitHub Check: Integration Tests
- GitHub Check: Integration Tests
- GitHub Check: Integration Tests
- GitHub Check: Integration Tests (CLERK_USE_RQ=true)
- GitHub Check: Integration Tests
- GitHub Check: Integration Tests
- GitHub Check: Integration Tests
- GitHub Check: Integration Tests
- GitHub Check: Integration Tests
- GitHub Check: Integration Tests
- GitHub Check: Integration Tests
- GitHub Check: Integration Tests
- GitHub Check: Integration Tests
- GitHub Check: Integration Tests
- GitHub Check: Unit Tests
- GitHub Check: Unit Tests (CLERK_USE_RQ=true)
- GitHub Check: Unit Tests (CLERK_USE_RQ=true)
- GitHub Check: Static analysis
- GitHub Check: Unit Tests
- GitHub Check: Unit Tests
- GitHub Check: Publish with pkg-pr-new
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (4)
.github/workflows/ci.yml (4)
245-256: Rebuild strategy forCLERK_USE_RQenvironment variable is sound.The conditional rebuilds with
--forcewhenclerk-use-rq: "true"ensure that environment-dependent builds don't rely on cached artifacts fromclerk-use-rq: "false"runs. This is the correct approach for environment-gated code paths.
257-287: Unit test environment variable and artifact naming are correctly configured.The
CLERK_USE_RQenvironment variable is properly propagated to test runs, and artifacts are appropriately named with-rqsuffix when applicable. The Typedoc gating (line 273) correctly restricts tests to the full suite run only, preventing duplicate runs across filtered matrix variants.
465-487: Integration test environment variable and artifact naming are correctly configured.The
CLERK_USE_RQenvironment variable is properly passed to integration test runs, and artifact naming correctly combinesnext-versionandclerk-use-rqindicators. The conditional formatting handles undefined matrix variables appropriately.
411-417: Verdaccio publish correctly propagatesCLERK_USE_RQto build step.The inline environment variable assignment ensures that packages are built with the appropriate RQ flag before publishing to Verdaccio. This maintains consistency between test and publish paths.
This comment has been minimized.
This comment has been minimized.
00c568a to
92b24d6
Compare
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)
.github/workflows/ci.yml(8 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
.github/workflows/*.yml
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
CI/CD must be configured with GitHub Actions.
Files:
.github/workflows/ci.yml
⏰ 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: check-major-bump
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (5)
.github/workflows/ci.yml (5)
318-343: Document asymmetry in RQ coverage across integration-test variants.The matrix configures RQ testing inconsistently:
billing: both'false'and'true'nextjs: version 15 gets both variants; versions 14 and 16 get neitherquickstart: neither variant (no explicitclerk-use-rq)This selective RQ coverage may be intentional (e.g., focusing on nextjs 15 for gradual rollout), but it's undocumented in the workflow or PR description. If RQ is a significant variant, ensure consistent coverage or document the rationale for skipping certain combinations.
238-248: Rebuild steps correctly target packages with RQ flag.The conditional rebuilds of
@clerk/sharedand its dependents are properly gated onmatrix.clerk-use-rq == 'true'and use--forceto bypass cache. TheCLERK_USE_RQ=trueenvironment variable is correctly passed during the build phase. This ensures that RQ-variant builds are distinct and not cached with non-RQ builds.Also applies to: 392-402
263-268: Typedoc tests correctly limited to main test run.The condition restricts Typedoc tests to Node 22 and the full test-filter (
"**"), which ensures they run only once per workflow on the non-RQ variant. This prevents unnecessary test duplication across matrix rows.
409-409: Verify environment variable expansion in Verdaccio publish command.The publish command passes
CLERK_USE_RQ=${{ matrix.clerk-use-rq }}. For matrix entries whereclerk-use-rqis undefined, this expands to an empty string (not the booleanfalse). Verify that the Verdaccio action or downstream build correctly interprets an emptyCLERK_USE_RQvariable as "disabled" (or explicitly default undefined values to'false'in the matrix, as noted above).
194-194: Matrix context correctly propagated to job names and artifacts.Job names and artifact naming now include RQ context (e.g., "Unit Tests (22, shared, clerk-js, RQ)" and suffix "-rq" on turbo summaries and playwright traces). This improves observability and traceability when debugging RQ-specific failures.
Also applies to: 284-284, 277-277, 478-478
| strategy: | ||
| fail-fast: true | ||
| fail-fast: false | ||
| matrix: | ||
| include: | ||
| - node-version: 22 | ||
| test-filter: "**" | ||
| clerk-use-rq: "false" | ||
| filter-label: "**" | ||
| - node-version: 22 | ||
| test-filter: "--filter=@clerk/shared --filter=@clerk/clerk-js" | ||
| clerk-use-rq: "true" | ||
| filter-label: "shared, clerk-js" |
There was a problem hiding this comment.
🧩 Analysis chain
Clarify matrix defaults for undefined clerk-use-rq values.
The integration-tests matrix includes entries where clerk-use-rq is not explicitly set (e.g., nextjs with versions 14 and 16, quickstart). When undefined, the matrix value expands to an empty string in template expressions, which may lead to subtle behavioral differences compared to an explicit 'false' value. This is particularly relevant for conditional logic in bash and environment variable assignments.
Recommend explicitly setting clerk-use-rq: 'false' for all matrix entries that should disable RQ testing, for clarity and to avoid relying on implicit falsy evaluation.
🏁 Script executed:
#!/bin/bash
# Description: Verify that all matrix entries have explicit clerk-use-rq values
# Check for any include entries missing clerk-use-rq
rg -A 3 'test-name:' .github/workflows/ci.yml | grep -A 3 'include:' | grep -v 'clerk-use-rq' | head -20Length of output: 0
🏁 Script executed:
#!/bin/bash
# Extract the full matrix configuration for integration-tests job
# This will show all include entries clearly
cat .github/workflows/ci.yml | sed -n '/integration-tests:/,/^ [^ ]/p' | sed -n '/matrix:/,/include:/p' -A 100 | head -80Length of output: 1822
🏁 Script executed:
#!/bin/bash
# Alternative: Use ast-grep or simple awk to find all 'include:' blocks with their entries
# Show each matrix entry with all its properties
rg -A 30 'integration-tests:' .github/workflows/ci.yml | rg -A 20 'matrix:'Length of output: 387
🏁 Script executed:
#!/bin/bash
# Check all matrix entries and look for missing clerk-use-rq
# Extract YAML and analyze structure
python3 << 'EOF'
import yaml
import sys
try:
with open('.github/workflows/ci.yml', 'r') as f:
content = yaml.safe_load(f)
jobs = content.get('jobs', {})
for job_name, job_config in jobs.items():
strategy = job_config.get('strategy', {})
matrix = strategy.get('matrix', {})
include = matrix.get('include', [])
if include:
print(f"\n=== Job: {job_name} ===")
for idx, entry in enumerate(include):
has_rq = 'clerk-use-rq' in entry
print(f"Entry {idx}: {entry}")
print(f" has clerk-use-rq: {has_rq}")
except Exception as e:
print(f"Error: {e}", file=sys.stderr)
EOFLength of output: 142
🏁 Script executed:
#!/bin/bash
# Show lines around integration-tests matrix to see include entries
rg -n -A 50 'integration-tests:' .github/workflows/ci.yml | grep -A 45 'matrix:'Length of output: 1131
🏁 Script executed:
#!/bin/bash
# Count how many matrix entries have clerk-use-rq vs don't
# Search for include: and then look ahead for clerk-use-rq
rg -B 2 -A 5 'clerk-use-rq' .github/workflows/ci.yml | head -60Length of output: 2436
🏁 Script executed:
#!/bin/bash
# Extract raw lines from the file for manual inspection
# Show lines 200-250 to capture the unit-tests matrix mentioned in review
sed -n '200,250p' .github/workflows/ci.ymlLength of output: 1669
🏁 Script executed:
#!/bin/bash
# Show integration-tests matrix entries more explicitly
# Look for matrix: include: pattern
sed -n '380,500p' .github/workflows/ci.ymlLength of output: 5082
🏁 Script executed:
#!/bin/bash
# Get the complete integration-tests matrix to see all entries
sed -n '317,360p' .github/workflows/ci.ymlLength of output: 1334
Add explicit clerk-use-rq: 'false' to integration-tests matrix entries lacking the field.
The integration-tests matrix has 4 entries without an explicit clerk-use-rq value:
- nextjs (next-version: '14') — lines 324-326
- nextjs (next-version: '16') — lines 332-334
- quickstart (next-version: '15') — lines 335-338
- quickstart (next-version: '16') — lines 339-341
When undefined, the matrix variable expands to an empty string in conditionals and environment variable assignments (CLERK_USE_RQ=${{ matrix.clerk-use-rq }}), causing behavioral inconsistency compared to an explicit 'false' value. Add clerk-use-rq: 'false' to all four entries to ensure consistent and predictable behavior.
🤖 Prompt for AI Agents
.github/workflows/ci.yml around lines 324-341: several integration-tests matrix
entries for nextjs (next-version: '14' at lines 324-326 and '16' at 332-334) and
quickstart (next-version: '15' at 335-338 and '16' at 339-341) are missing an
explicit clerk-use-rq field; add clerk-use-rq: 'false' to each of these four
matrix entries so the matrix variable expands to a predictable 'false' value
(preventing empty-string behavior in conditionals and env assignments like
CLERK_USE_RQ=${{ matrix.clerk-use-rq }}).
7be0dc3 to
35f4e83
Compare
Description
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Chores