-
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
Changes from all commits
b8a2f0d
34ab43d
f991809
59e7354
3e75420
73f2138
2a24131
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
--- | ||
'@clerk/clerk-js': minor | ||
'@clerk/shared': minor | ||
'@clerk/types': minor | ||
--- | ||
|
||
[Billing Beta] Update `clerk.billing.getPlans()` to return paginated data and introduce the `usePlans()` hook. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,7 +6,6 @@ import type { | |
CommercePaymentResource, | ||
CommercePlanJSON, | ||
CommercePlanResource, | ||
CommerceProductJSON, | ||
CommerceStatementJSON, | ||
CommerceStatementResource, | ||
CommerceSubscriptionJSON, | ||
|
@@ -29,15 +28,21 @@ import { | |
} from '../../resources/internal'; | ||
|
||
export class CommerceBilling implements CommerceBillingNamespace { | ||
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 commentThe reason will be displayed to describe this comment to others. Learn more. Reminder that we don't consider this breaking. |
||
const { for: forParam, ...safeParams } = params || {}; | ||
const searchParams = { ...safeParams, payer_type: forParam || 'user' }; | ||
return await BaseResource._fetch({ | ||
path: `/commerce/plans`, | ||
method: 'GET', | ||
search: { payerType: params?.subscriberType || '' }, | ||
})) as unknown as ClerkPaginatedResponse<CommerceProductJSON>; | ||
search: convertPageToOffsetSearchParams(searchParams), | ||
}).then(res => { | ||
const { data: plans, total_count } = res as unknown as ClerkPaginatedResponse<CommercePlanJSON>; | ||
|
||
const defaultProduct = products.find(product => product.is_default); | ||
return defaultProduct?.plans.map(plan => new CommercePlan(plan)) || []; | ||
return { | ||
total_count, | ||
data: plans.map(plan => new CommercePlan(plan)), | ||
}; | ||
}); | ||
}; | ||
|
||
getPlan = async (params: { id: string }): Promise<CommercePlanResource> => { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
import type { CommercePlanResource, GetPlansParams } from '@clerk/types'; | ||
|
||
import { useClerkInstanceContext } from '../contexts'; | ||
import { createCommerceHook } from './createCommerceHook'; | ||
|
||
/** | ||
* @internal | ||
*/ | ||
export const usePlans = createCommerceHook<CommercePlanResource, GetPlansParams>({ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we include the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in the index file I do |
||
hookName: 'usePlans', | ||
resourceType: 'commerce-plans', | ||
useFetcher: _for => { | ||
const clerk = useClerkInstanceContext(); | ||
return params => clerk.billing.getPlans({ ...params, for: _for === 'organization' ? 'org' : 'user' }); | ||
}, | ||
options: { | ||
unauthenticated: true, | ||
}, | ||
}); |
Uh oh!
There was an error while loading. Please reload this page.
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 amajor
bump instead ofminor
Changing the return type from a flat array to a paginated object alters the public contract of
clerk.billing.getPlans()
. Any consumer expectingPlan[]
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 amajor
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:
Length of output: 670
🏁 Script executed:
Length of output: 809
🏁 Script executed:
Length of output: 235
Require a major version bump for paginated
getPlans()
Changing
clerk.billing.getPlans()
from returning a flatPlan[]
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:• 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
🤖 Prompt for AI Agents
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:
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: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
🧠 Learnings used