-
Notifications
You must be signed in to change notification settings - Fork 381
chore(shared,clerk-js): Introduce experimental usePlans
#6327
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: 2a24131 The changes in this PR will be included in the next version bump. This PR includes changesets to release 22 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 WalkthroughWalkthroughThe changes refactor the plan retrieval logic in the commerce billing module and update related hooks and type definitions. The 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@clerk/upgrade
@clerk/vue
commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/clerk-js/src/ui/components/PricingTable/PricingTable.tsx (1)
16-16
: Remove debug console.log statement.This debugging statement should be removed before merging to production.
- console.log('plansss', plans, error);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/clerk-js/src/core/modules/commerce/CommerceBilling.ts
(1 hunks)packages/clerk-js/src/ui/components/PricingTable/PricingTable.tsx
(1 hunks)packages/clerk-js/src/ui/contexts/components/Plans.tsx
(4 hunks)packages/shared/src/react/hooks/index.ts
(1 hunks)packages/shared/src/react/hooks/usePlans.tsx
(1 hunks)packages/types/src/commerce.ts
(2 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{js,jsx,ts,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/development.mdc
packages/**/*.ts
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/development.mdc
packages/**/*.{ts,tsx,d.ts}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/development.mdc
packages/**/index.{js,ts}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/development.mdc
**/*.{ts,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/development.mdc
- .cursor/rules/typescript.mdc
**/index.ts
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/react.mdc
- .cursor/rules/typescript.mdc
**/*
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
**/*.{tsx,jsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/development.mdc
packages/{clerk-js,elements,themes}/**/*
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/monorepo.mdc
**/*.{jsx,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/react.mdc
**/*.tsx
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/react.mdc
🧠 Learnings (5)
📓 Common learnings
Learnt from: dstaley
PR: clerk/javascript#6116
File: .changeset/tangy-garlics-say.md:1-2
Timestamp: 2025-06-13T16:09:53.061Z
Learning: In the Clerk JavaScript repository, contributors create intentionally empty changeset files (containing only the YAML delimiters) when a PR touches only non-published parts of the codebase (e.g., sandbox assets). This signals that no package release is required, so such changesets should not be flagged as missing content.
Learnt from: panteliselef
PR: clerk/javascript#6285
File: packages/types/src/commerce.ts:1305-1305
Timestamp: 2025-07-11T18:08:14.697Z
Learning: In the Clerk JavaScript repository, when there's a conflict between naming consistency (camelCase) and avoiding breaking changes, the team prioritizes maintaining backward compatibility over enforcing naming conventions, even for experimental APIs.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/nextjs.mdc:0-0
Timestamp: 2025-06-30T10:31:43.578Z
Learning: Applies to pages/**/*.{js,ts,jsx,tsx} : Use SWR for client-side data fetching in Pages Router
packages/shared/src/react/hooks/index.ts (12)
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.{jsx,tsx} : Use proper hook structure
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.{jsx,tsx} : Export components as named exports for better tree-shaking
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Applies to packages/**/index.{js,ts} : Use tree-shaking friendly exports
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/*.{ts,tsx} : Proper tree-shaking friendly exports
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/*.{ts,tsx} : Use ES6 imports/exports consistently
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.{jsx,tsx} : Use useState for simple state management
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/*.{ts,tsx} : Use default exports sparingly, prefer named exports
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/index.ts : Use index.ts files for clean imports but avoid deep barrel exports
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/*.{ts,tsx} : Use `public` explicitly for clarity in public APIs
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/index.ts : Avoid barrel files (index.ts re-exports) as they can cause circular dependencies
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.{jsx,tsx} : Use useReducer for complex state logic
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Use dynamic imports for optional features
packages/clerk-js/src/ui/components/PricingTable/PricingTable.tsx (10)
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/clerk-react/**/*.{test,spec}.{js,ts,tsx} : Component testing should use React Testing Library.
Learnt from: panteliselef
PR: clerk/javascript#6097
File: packages/clerk-js/src/ui/elements/LineItems.tsx:89-89
Timestamp: 2025-06-10T09:38:56.214Z
Learning: In packages/clerk-js/src/ui/elements/LineItems.tsx, the Title component's React.forwardRef should use HTMLTableCellElement as the generic type parameter, even though it renders a Dt element. This is the correct implementation according to the codebase maintainer.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.tsx : Use proper type definitions for props and state
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.{jsx,tsx} : Use proper hook structure
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.{jsx,tsx} : Implement proper error states
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.{jsx,tsx} : Use proper error logging
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/*.{ts,tsx} : Use immutable update patterns (spread, etc.) for objects and arrays
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.{jsx,tsx} : Implement proper state naming
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.{jsx,tsx} : Implement proper state selectors
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/*.{ts,tsx} : Proper error handling with typed errors
packages/shared/src/react/hooks/usePlans.tsx (3)
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.{jsx,tsx} : Use proper hook structure
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/clerk-react/**/*.{test,spec}.{js,ts,tsx} : Component testing should use React Testing Library.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.{jsx,tsx} : Always use functional components with hooks instead of class components
packages/clerk-js/src/ui/contexts/components/Plans.tsx (15)
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/clerk-react/**/*.{test,spec}.{js,ts,tsx} : Component testing should use React Testing Library.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/nextjs.mdc:0-0
Timestamp: 2025-06-30T10:31:43.578Z
Learning: Applies to pages/**/*.{js,ts,jsx,tsx} : Use SWR for client-side data fetching in Pages Router
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.{jsx,tsx} : Use proper hook structure
Learnt from: panteliselef
PR: clerk/javascript#6159
File: packages/shared/src/react/hooks/usePagesOrInfinite.ts:100-111
Timestamp: 2025-06-23T08:31:52.513Z
Learning: In usePagesOrInfinite hook, when fetchOnMount is false, it's expected that shouldFetch can be true while swrFetcher is null. This creates a valid SWR pattern where a key is provided without a fetcher, allowing manual revalidation later via the mutate function while maintaining cache state management. This is intentional design for manual fetching scenarios.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.{jsx,tsx} : Use Context API for theme/authentication
Learnt from: LauraBeatris
PR: clerk/javascript#6117
File: packages/clerk-js/src/ui/components/SessionTasks/index.tsx:64-83
Timestamp: 2025-06-18T16:33:36.764Z
Learning: In SessionTasks component at packages/clerk-js/src/ui/components/SessionTasks/index.tsx, the useEffect that checks for pending tasks should only run on mount (not react to currentTask/status changes) to handle browser back navigation edge cases without interfering with normal task completion flow managed by nextTask().
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.{jsx,tsx} : Always use functional components with hooks instead of class components
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.{jsx,tsx} : Use proper state updates with callbacks
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/*.{ts,tsx} : Use immutable update patterns (spread, etc.) for objects and arrays
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.{jsx,tsx} : Use useReducer for complex state logic
Learnt from: panteliselef
PR: clerk/javascript#6285
File: packages/types/src/commerce.ts:1305-1305
Timestamp: 2025-07-11T18:08:14.697Z
Learning: In the Clerk JavaScript repository, when there's a conflict between naming consistency (camelCase) and avoiding breaking changes, the team prioritizes maintaining backward compatibility over enforcing naming conventions, even for experimental APIs.
Learnt from: LauraBeatris
PR: clerk/javascript#6117
File: packages/clerk-js/src/ui/components/SessionTasks/tasks/ForceOrganizationSelection.tsx:17-21
Timestamp: 2025-06-18T23:27:13.537Z
Learning: In Clerk's JavaScript codebase, query errors from hooks like useOrganizationList are not typically handled in AIO (All-in-One) components. Error handling may be connected with the Card context instead, suggesting a centralized error handling approach rather than component-level error handling.
Learnt from: dstaley
PR: clerk/javascript#6100
File: packages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsx:121-124
Timestamp: 2025-06-16T17:08:58.414Z
Learning: The @clerk/clerk-js package only supports browsers released in the last two years (since May 8, 2023), so modern CSS features like color-mix() are fully supported across all target browsers without requiring fallbacks.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.{jsx,tsx} : Use proper useMemo for expensive computations
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.{jsx,tsx} : Implement proper subscription cleanup
🧬 Code Graph Analysis (5)
packages/clerk-js/src/ui/components/PricingTable/PricingTable.tsx (2)
packages/shared/src/react/hooks/usePlans.tsx (1)
usePlans
(9-16)packages/clerk-js/src/ui/contexts/components/Plans.tsx (1)
usePlans
(67-77)
packages/shared/src/react/hooks/usePlans.tsx (3)
packages/clerk-js/src/ui/contexts/components/Plans.tsx (1)
usePlans
(67-77)packages/shared/src/react/hooks/createCommerceHook.tsx (1)
createCommerceHook
(38-101)packages/types/src/commerce.ts (2)
CommercePlanResource
(220-400)GetPlansParams
(208-210)
packages/types/src/commerce.ts (1)
packages/types/src/pagination.ts (2)
ClerkPaginatedResponse
(21-30)ClerkPaginationParams
(35-46)
packages/clerk-js/src/core/modules/commerce/CommerceBilling.ts (5)
packages/types/src/commerce.ts (2)
GetPlansParams
(208-210)CommercePlanResource
(220-400)packages/types/src/pagination.ts (1)
ClerkPaginatedResponse
(21-30)packages/clerk-js/src/utils/convertPageToOffsetSearchParams.ts (1)
convertPageToOffsetSearchParams
(4-16)packages/types/src/json.ts (1)
CommercePlanJSON
(629-666)packages/clerk-js/src/core/resources/CommercePlan.ts (1)
CommercePlan
(5-83)
packages/clerk-js/src/ui/contexts/components/Plans.tsx (3)
packages/shared/src/react/hooks/index.ts (1)
usePlans
(15-15)packages/shared/src/react/hooks/usePlans.tsx (1)
usePlans
(9-16)packages/clerk-js/src/ui/contexts/components/SubscriberType.ts (1)
useSubscriberTypeContext
(6-6)
🪛 Biome (1.9.4)
packages/clerk-js/src/core/modules/commerce/CommerceBilling.ts
[error] 34-34: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
⏰ 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). (23)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (tanstack-react-router, chrome)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (expo-web, chrome)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (elements, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (quickstart, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Publish with pkg-pr-new
- GitHub Check: Unit Tests (22, **)
- GitHub Check: Unit Tests (18, --filter=@clerk/astro --filter=@clerk/backend --filter=@clerk/express --filter=@c...
- GitHub Check: Static analysis
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (10)
packages/clerk-js/src/ui/components/PricingTable/PricingTable.tsx (1)
15-15
: Good addition of error handling.Adding error extraction from the
usePlans
hook is a good practice for proper error handling in the component.packages/shared/src/react/hooks/index.ts (1)
15-15
: Export follows established patterns.The export of
usePlans
as__experimental_usePlans
follows the same pattern as other experimental hooks in the file and is consistent with the experimental nature of the feature.packages/shared/src/react/hooks/usePlans.tsx (1)
9-16
: Well-implemented commerce hook.The
usePlans
hook is properly implemented using thecreateCommerceHook
utility with correct typing and parameter transformation. The conversion from'organization'
to'org'
for the API call is handled correctly.packages/types/src/commerce.ts (2)
37-37
: Type update aligns with implementation changes.The return type change from simple array to
ClerkPaginatedResponse<CommercePlanResource>
correctly reflects the new paginated API structure.
208-210
: GetPlansParams type properly extends pagination support.The change from interface to type alias extending
ClerkPaginationParams
with optionalfor
property is well-structured and supports the new parameter requirements.packages/clerk-js/src/ui/contexts/components/Plans.tsx (5)
4-4
: Import change looks good.The addition of
__experimental_usePlans
import is consistent with the experimental commerce hooks pattern used throughout the file.
67-77
: Hook implementation successfully refactored to use experimental hook.The
usePlans
hook has been properly updated to use the new experimental hook pattern, maintaining consistency with other commerce hooks in the file. The parameters are correctly structured with pagination, caching, and subscriber type handling.One observation: The
pageSize
is set to 50 for plans while other hooks use 10. This may be intentional given that plans are typically fewer in number and fetched less frequently.
108-108
: Cache invalidation properly updated.The destructuring has been correctly updated to use
revalidatePlans
instead of the previousmutatePlans
, maintaining consistency with the new hook's return interface.
118-118
: Revalidation call updated correctly.The cache invalidation call has been properly updated to use
revalidatePlans
instead ofmutatePlans
.
121-121
: Dependency array updated correctly.The useCallback dependency array has been properly updated to include
revalidatePlans
instead ofmutatePlans
, ensuring the memoization works correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
packages/shared/src/react/hooks/createCommerceHook.tsx (1)
1-106
: Add tests for the new unauthenticated functionality.The new unauthenticated mode should be covered by tests to ensure proper behavior and prevent regressions.
Consider adding tests that verify:
- The hook works correctly when
options.unauthenticated
is true and no user is present- The hook still requires a user when
options.unauthenticated
is false or undefined- The
isClerkLoaded
condition behaves correctly in both authenticated and unauthenticated modes
🧹 Nitpick comments (1)
packages/shared/src/react/hooks/createCommerceHook.tsx (1)
27-40
: Update JSDoc to document the new options parameter.The JSDoc comment should include information about the new
options
parameter and itsunauthenticated
flag to maintain comprehensive API documentation.* A hook factory that creates paginated data fetching hooks for commerce-related resources. * It provides a standardized way to create hooks that can fetch either user or organization resources * with built-in pagination support. * * The generated hooks handle: * - Clerk authentication context * - Resource-specific data fetching * - Pagination (both traditional and infinite scroll) * - Telemetry tracking * - Type safety for the specific resource. * + * @param options - Optional configuration including unauthenticated access flag * @internal
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/shared/src/react/hooks/createCommerceHook.tsx
(3 hunks)packages/shared/src/react/hooks/usePlans.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/shared/src/react/hooks/usePlans.tsx
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,jsx,ts,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/development.mdc
packages/**/*.{ts,tsx,d.ts}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/development.mdc
**/*.{ts,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/development.mdc
- .cursor/rules/typescript.mdc
**/*.{tsx,jsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/development.mdc
**/*.{jsx,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/react.mdc
**/*.tsx
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/react.mdc
**/*
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
🧠 Learnings (2)
📓 Common learnings
Learnt from: dstaley
PR: clerk/javascript#6116
File: .changeset/tangy-garlics-say.md:1-2
Timestamp: 2025-06-13T16:09:53.061Z
Learning: In the Clerk JavaScript repository, contributors create intentionally empty changeset files (containing only the YAML delimiters) when a PR touches only non-published parts of the codebase (e.g., sandbox assets). This signals that no package release is required, so such changesets should not be flagged as missing content.
Learnt from: panteliselef
PR: clerk/javascript#6285
File: packages/types/src/commerce.ts:1305-1305
Timestamp: 2025-07-11T18:08:14.697Z
Learning: In the Clerk JavaScript repository, when there's a conflict between naming consistency (camelCase) and avoiding breaking changes, the team prioritizes maintaining backward compatibility over enforcing naming conventions, even for experimental APIs.
packages/shared/src/react/hooks/createCommerceHook.tsx (2)
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.{jsx,tsx} : Use proper hook structure
Learnt from: LauraBeatris
PR: clerk/javascript#6117
File: packages/clerk-js/src/ui/components/SessionTasks/tasks/ForceOrganizationSelection.tsx:17-21
Timestamp: 2025-06-18T23:27:13.537Z
Learning: In Clerk's JavaScript codebase, query errors from hooks like useOrganizationList are not typically handled in AIO (All-in-One) components. Error handling may be connected with the Card context instead, suggesting a centralized error handling approach rather than component-level error handling.
⏰ 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: semgrep-cloud-platform/scan
- GitHub Check: Build Packages
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
packages/shared/src/react/hooks/createCommerceHook.tsx (3)
22-24
: Type definition follows TypeScript best practices.The optional
options
property with nestedunauthenticated
boolean flag is well-structured and maintains backward compatibility.
45-45
: Function signature update maintains backward compatibility.The addition of the optional
options
parameter preserves existing functionality while enabling the new unauthenticated mode.
83-83
: Only usePlans passes unauthenticated—please verify getPlans is publicI checked the codebase and found that:
- The only hook that sets
unauthenticated: true
is packages/shared/src/react/hooks/usePlans.tsx, which callsclerk.billing.getPlans
- All other commerce hooks (subscriptions, statements, payment attempts) do not pass
unauthenticated
and therefore require a logged-in userPlease confirm that:
clerk.billing.getPlans
is intended to be fully public and cannot expose any sensitive data- All other endpoints (
getSubscriptions
,getStatements
,getPaymentAttempts
) still enforce authentication viacreateCommerceHook
’s default user check- Your backend enforces appropriate permission checks on each endpoint
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.changeset/eight-socks-lead.md
(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: dstaley
PR: clerk/javascript#6116
File: .changeset/tangy-garlics-say.md:1-2
Timestamp: 2025-06-13T16:09:53.061Z
Learning: In the Clerk JavaScript repository, contributors create intentionally empty changeset files (containing only the YAML delimiters) when a PR touches only non-published parts of the codebase (e.g., sandbox assets). This signals that no package release is required, so such changesets should not be flagged as missing content.
Learnt from: panteliselef
PR: clerk/javascript#6285
File: packages/types/src/commerce.ts:1305-1305
Timestamp: 2025-07-11T18:08:14.697Z
Learning: In the Clerk JavaScript repository, when there's a conflict between naming consistency (camelCase) and avoiding breaking changes, the team prioritizes maintaining backward compatibility over enforcing naming conventions, even for experimental APIs.
.changeset/eight-socks-lead.md (3)
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/clerk-react/**/*.{test,spec}.{js,ts,tsx} : Component testing should use React Testing Library.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/@clerk/*/package.json : Framework packages must depend on '@clerk/clerk-js' for core functionality.
Learnt from: dstaley
PR: clerk/javascript#6100
File: packages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsx:121-124
Timestamp: 2025-06-16T17:08:58.414Z
Learning: The @clerk/clerk-js package only supports browsers released in the last two years (since May 8, 2023), so modern CSS features like color-mix() are fully supported across all target browsers without requiring fallbacks.
⏰ 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: semgrep-cloud-platform/scan
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
'@clerk/clerk-js': minor | ||
'@clerk/shared': minor | ||
'@clerk/types': minor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Paginated getPlans()
is a breaking change – consider a major
bump instead of minor
Changing the return type from a flat array to a paginated object alters the public contract of clerk.billing.getPlans()
. Any consumer expecting Plan[]
will now break at runtime (and at compile-time for TS). Unless this API was explicitly marked experimental/unstable in the previous release, semver calls for a major
version bump for @clerk/clerk-js
, @clerk/shared
, and @clerk/types
.
Please double-check the stability guarantees and update the changeset accordingly, or add a note clarifying why a minor bump is sufficient.
🏁 Script executed:
#!/bin/bash
# Search for all occurrences of getPlans in the codebase
rg "getPlans" -n
Length of output: 670
🏁 Script executed:
#!/bin/bash
# Show context around getPlans entries in CHANGELOGs
rg "getPlans" -n -C3 packages/types/CHANGELOG.md
rg "getPlans" -n -C3 packages/clerk-js/CHANGELOG.md
rg "getPlans" -n -C3 packages/react/CHANGELOG.md
Length of output: 809
🏁 Script executed:
#!/bin/bash
# Display the contents of the changeset to see which packages are listed
sed -n '1,50p' .changeset/eight-socks-lead.md
Length of output: 235
Require a major version bump for paginated getPlans()
Changing clerk.billing.getPlans()
from returning a flat Plan[]
to a paginated object is a public‐API break (consumers and TypeScript definitions will both fail). Unless this endpoint was already marked as experimental/unstable (it isn’t), semver dictates a major bump.
• Update .changeset/eight-socks-lead.md
to:
---
-'@clerk/clerk-js': minor
-'@clerk/shared': minor
-'@clerk/types': minor
+'@clerk/clerk-js': major
+'@clerk/shared': major
+'@clerk/types': major
---
• Verify that no other packages (e.g. React bindings) rely on the old flat-array signature without a corresponding update.
• Confirm in your release notes that this breaking change is clearly documented.
📝 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.
'@clerk/clerk-js': minor | |
'@clerk/shared': minor | |
'@clerk/types': minor | |
'@clerk/clerk-js': major | |
'@clerk/shared': major | |
'@clerk/types': major |
🤖 Prompt for AI Agents
In the file .changeset/eight-socks-lead.md around lines 2 to 4, the version
bumps for '@clerk/clerk-js', '@clerk/shared', and '@clerk/types' are marked as
minor, but changing clerk.billing.getPlans() to return a paginated object is a
breaking API change requiring a major version bump. Update the version bump for
these packages to major in the changeset file, verify that no other packages
depend on the old flat array signature without updates, and ensure the release
notes clearly document this breaking change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are correct, but in this specific case the getPlans
function has JSDoc coments that is marking it as experimental via @experimental
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
@panteliselef Thank you for the clarification! You're absolutely right - if the getPlans
function is marked as @experimental
in its JSDoc comments, then the minor version bump is appropriate. Experimental APIs are expected to have breaking changes and consumers are warned about this instability, so they don't follow the same strict semver rules as stable APIs.
The changeset version bumps are correct as they are. I apologize for the confusion in my initial assessment.
🏁 Script executed:
#!/bin/bash
# Search for getPlans function definitions with JSDoc comments
ast-grep --pattern $'/**
* $$$
*/
getPlans($$$) {
$$$
}'
# Also search for interface/type declarations with JSDoc
ast-grep --pattern $'/**
* $$$
*/
getPlans($$$): $$$'
# Search for any @experimental annotations near getPlans
rg -A 10 -B 10 "@experimental.*getPlans|getPlans.*@experimental"
# Search for JSDoc comments containing @experimental and getPlans in the same context
rg -A 5 -B 5 "@experimental" | grep -A 5 -B 5 "getPlans"
Length of output: 1045
Perfect! The script output confirms your claim. I can see in packages/types/src/commerce.ts
there's indeed an @experimental
JSDoc annotation that states:
@experimental This is an experimental API for the Billing feature that is available under a public beta, and the API is subject to change.
This explicitly warns consumers that the API is subject to change, which justifies using a minor version bump for breaking changes to experimental APIs rather than a major version bump. The annotation even advises users to pin SDK versions to avoid breaking changes.
Thank you for the correction - the changeset version bumps are appropriate as they are. I apologize for the initial misassessment without checking the experimental status first.
✏️ Learnings added
Learnt from: panteliselef
PR: clerk/javascript#6327
File: .changeset/eight-socks-lead.md:2-4
Timestamp: 2025-07-16T10:43:17.696Z
Learning: In the Clerk JavaScript repository, APIs marked with @experimental JSDoc annotations can have breaking changes released with minor version bumps rather than major version bumps, as consumers are warned about the instability of experimental features.
Learnt from: panteliselef
PR: clerk/javascript#6327
File: .changeset/eight-socks-lead.md:2-4
Timestamp: 2025-07-16T10:43:17.696Z
Learning: In the Clerk JavaScript repository, APIs marked with @experimental JSDoc annotations can have breaking changes released with minor version bumps rather than major version bumps, as consumers are warned about the instability of experimental features. The billing feature specifically has an @experimental annotation stating "This is an experimental API for the Billing feature that is available under a public beta, and the API is subject to change."
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: jacekradko
PR: clerk/javascript#5905
File: .changeset/six-ears-wash.md:1-3
Timestamp: 2025-06-26T03:27:05.535Z
Learning: In the Clerk JavaScript repository, changeset headers support single quotes syntax (e.g., '@clerk/backend': minor) and work fine with their current changesets integration, so there's no need to change them to double quotes.
Learnt from: dstaley
PR: clerk/javascript#6116
File: .changeset/tangy-garlics-say.md:1-2
Timestamp: 2025-06-13T16:09:53.061Z
Learning: In the Clerk JavaScript repository, contributors create intentionally empty changeset files (containing only the YAML delimiters) when a PR touches only non-published parts of the codebase (e.g., sandbox assets). This signals that no package release is required, so such changesets should not be flagged as missing content.
Learnt from: panteliselef
PR: clerk/javascript#6285
File: packages/types/src/commerce.ts:1305-1305
Timestamp: 2025-07-11T18:08:14.697Z
Learning: In the Clerk JavaScript repository, when there's a conflict between naming consistency (camelCase) and avoiding breaking changes, the team prioritizes maintaining backward compatibility over enforcing naming conventions, even for experimental APIs.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/@clerk/*/package.json : Framework packages must depend on '@clerk/clerk-js' for core functionality.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/*/package.json : All packages must be published under the @clerk namespace on npm.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Applies to packages/*/package.json : All packages must be published under the @clerk namespace
Learnt from: dstaley
PR: clerk/javascript#6100
File: packages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsx:121-124
Timestamp: 2025-06-16T17:08:58.414Z
Learning: The @clerk/clerk-js package only supports browsers released in the last two years (since May 8, 2023), so modern CSS features like color-mix() are fully supported across all target browsers without requiring fallbacks.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/{clerk-js,elements,themes}/**/* : Visual regression testing should be performed for UI components.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/clerk-react/**/*.{test,spec}.{js,ts,tsx} : Component testing should use React Testing Library.
Learnt from: wobsoriano
PR: clerk/javascript#6229
File: packages/backend/src/api/endpoints/MachineTokensApi.ts:47-89
Timestamp: 2025-07-01T15:20:41.834Z
Learning: In the Clerk JavaScript repository, for the MachineTokensApi class (packages/backend/src/api/endpoints/MachineTokensApi.ts), the maintainers prefer to rely on TypeScript types and readable property names for API documentation rather than JSDoc comments.
Learnt from: panteliselef
PR: clerk/javascript#6097
File: packages/clerk-js/src/ui/elements/LineItems.tsx:89-89
Timestamp: 2025-06-10T09:38:56.214Z
Learning: In packages/clerk-js/src/ui/elements/LineItems.tsx, the Title component's React.forwardRef should use HTMLTableCellElement as the generic type parameter, even though it renders a Dt element. This is the correct implementation according to the codebase maintainer.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/*.{ts,tsx} : Document public functions and APIs with JSDoc-style comments including @param, @returns, @throws, and @example
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : All public APIs must be documented with JSDoc
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Maintain comprehensive JSDoc comments for public APIs
getPlans = async (params?: GetPlansParams): Promise<CommercePlanResource[]> => { | ||
const { data: products } = (await BaseResource._fetch({ | ||
path: `/commerce/products`, | ||
getPlans = async (params?: GetPlansParams): Promise<ClerkPaginatedResponse<CommercePlanResource>> => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reminder that we don't consider this breaking.
/** | ||
* @internal | ||
*/ | ||
export const usePlans = createCommerceHook<CommercePlanResource, GetPlansParams>({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we include the __internal
prefix for hooks as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in the index file I do export { usePlans as __experimental_usePlans } from './usePlans';
so it is okay.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have a lot of context on commerce itself but the changes look good! 🫡
Description
This PR
getPlans()
to support paginated data.usePlans()
hook as experimental. This hook can be used to fetch all available plans for users or organizations.Checklist
pnpm test
runs as expected.pnpm build
runs as expected.Type of change
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Refactor