feat(backend): Add cancelSubscriptionItem to BillingApi
#6611
Conversation
🦋 Changeset detectedLatest commit: 5115fe6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds a new BillingApi method cancelSubscriptionItem with optional endNow param, issuing DELETE to /commerce/subscription_items/{id} and returning CommerceSubscriptionItem. Introduces CommerceSubscriptionItem resource with fromJSON mapper, exports it, and wires it into Deserializer. Expands and revises commerce JSON types: adds CommerceSubscriptionItemJSON fields; introduces webhook-specific JSON types for subscription, payment attempt, and subscription item; updates Webhooks type aliases to use these. Exports updated webhook event types and removes prior JSON exports from the public index. Exports CommerceMoneyAmount and CommerceSubscriptionItem from resources. Adds a changeset for a minor version bump. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (9)
packages/backend/src/api/resources/CommercePlan.ts (1)
4-9: Avoid duplicating CommerceMoneyAmount; alias the source type and add JSDoc.This locally redefined type can drift from
@clerk/types. Prefer a single source of truth by aliasing the upstream interface and documenting the public export.Apply this diff:
-export type CommerceMoneyAmount = { - amount: number; - amountFormatted: string; - currency: string; - currencySymbol: string; -}; +/** + * @experimental This is an experimental API for the Billing feature that is available under a public beta, and the API is subject to change. + * It is advised to pin the SDK version to avoid breaking changes. + */ +export type CommerceMoneyAmount = import('@clerk/types').CommerceMoneyAmount;packages/backend/src/api/resources/Deserializer.ts (1)
187-188: Add deserialization case — consider adding unit tests to lock behavior.The new case correctly maps
ObjectType.CommerceSubscriptionItemtoCommerceSubscriptionItem.fromJSON. Please add tests that:
- Verify single-item deserialization.
- Verify list/paginated payloads with mixed object types still work.
I can draft Jest tests for
jsonToObjectanddeserializecovering subscription item payloads if helpful.packages/backend/src/api/endpoints/BillingApi.ts (1)
15-21: Update JSDoc and Confirm Query Param Casing
- Added a JSDoc on the
CancelSubscriptionItemParamstype to mirror other endpoint-level typedefs.- No manual snake_case mapping is required: the HTTP layer uses
snakecase-keysto convert allqueryParamskeys (e.g.endNow→end_now) before appending them to the URL.--- a/packages/backend/src/api/endpoints/BillingApi.ts +++ b/packages/backend/src/api/endpoints/BillingApi.ts @@ -12,6 +12,11 @@ type CancelSubscriptionItemParams = { /** * If true, the subscription item will be canceled immediately. If false or undefined, the subscription item will be canceled at the end of the current billing period. * @default undefined */ endNow?: boolean; }; +/** + * Parameters for canceling a subscription item. + * + * The `endNow` field is automatically converted to `end_now` at the HTTP layer. + */ type CancelSubscriptionItemParams = { /** * If true, the subscription item will be canceled immediately. If false or undefined, the subscription item will be canceled at the end of the current billing period. * @default undefined */ endNow?: boolean; };packages/backend/src/api/resources/CommerceSubscriptionItem.ts (2)
25-28: Clarify timestamp units in JSDoc (seconds vs. milliseconds)
periodStart,periodEnd,canceledAt,pastDueAtare numbers; the unit isn’t stated. Elsewhere we use Unix epoch seconds. Add “Unix epoch seconds” to avoid ambiguity for integrators.Also applies to: 48-55, 58-58
1-4: Optional: avoid coupling to @clerk/types for JSON money shape
CommercePlan.tsderives the money shape fromCommercePlanJSON['fee']and avoids direct imports from@clerk/types. For consistency and to reduce cross-package coupling, consider usingCommerceSubscriptionItemJSON['amount']/['lifetime_paid']in the formatter signature rather than importingCommerceMoneyAmountJSON.If you take this path:
-import type { CommerceMoneyAmountJSON } from '@clerk/types'; +// Rely on local JSON shape to avoid external type coupling +// (no extra import needed)And change the function signature to:
function formatAmountJSON( amount: NonNullable<CommerceSubscriptionItemJSON['amount']> | null, ): CommerceMoneyAmount | null { ... }packages/backend/src/api/resources/Webhooks.ts (1)
78-89: SubscriptionItem webhook events: confirm full event taxonomyWe include created/updated/active/canceled/upcoming/ended/abandoned/incomplete/past_due. If 'expired' is a possible event (status exists), confirm whether it should be included or is intentionally omitted.
If needed:
| 'subscriptionItem.past_due', + | 'subscriptionItem.expired',Also consider adding a short JSDoc note above each new type to help SDK consumers discover the new shapes.
packages/backend/src/api/resources/JSON.ts (3)
846-855: Centralize and export CommerceSubscriptionItemStatus if referenced externallyThe status union looks right. If SDK consumers need to narrow against these statuses, consider re-exporting this type from the package’s public surface (or document the intended import path). Otherwise, keep it internal.
881-919: Webhook item plan shape diverges from CommercePlanJSON by design — consider a named aliasEmbedding a bespoke plan object is fine (webhook vs. BAPI shapes differ), but consider introducing a named alias (e.g.,
CommerceWebhookPlanJSON) and reusing it across webhook types to avoid duplication and ease future changes.
920-954: CommercePaymentAttemptWebhookEventJSON: webhook item collection rename
subscription_items: CommerceSubscriptionItemWebhookEventJSON[]is clear. If older consumers useditems, a migration note in the changelog would help. Otherwise, LGTM.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
.changeset/eight-sheep-matter.md(1 hunks)packages/backend/src/api/endpoints/BillingApi.ts(3 hunks)packages/backend/src/api/resources/CommercePlan.ts(1 hunks)packages/backend/src/api/resources/CommerceSubscriptionItem.ts(1 hunks)packages/backend/src/api/resources/Deserializer.ts(2 hunks)packages/backend/src/api/resources/JSON.ts(4 hunks)packages/backend/src/api/resources/Webhooks.ts(3 hunks)packages/backend/src/api/resources/index.ts(1 hunks)packages/backend/src/index.ts(2 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/backend/src/api/resources/index.tspackages/backend/src/api/resources/Deserializer.tspackages/backend/src/api/resources/CommercePlan.tspackages/backend/src/api/endpoints/BillingApi.tspackages/backend/src/index.tspackages/backend/src/api/resources/CommerceSubscriptionItem.tspackages/backend/src/api/resources/Webhooks.tspackages/backend/src/api/resources/JSON.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/backend/src/api/resources/index.tspackages/backend/src/api/resources/Deserializer.tspackages/backend/src/api/resources/CommercePlan.tspackages/backend/src/api/endpoints/BillingApi.tspackages/backend/src/index.tspackages/backend/src/api/resources/CommerceSubscriptionItem.tspackages/backend/src/api/resources/Webhooks.tspackages/backend/src/api/resources/JSON.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/backend/src/api/resources/index.tspackages/backend/src/api/resources/Deserializer.tspackages/backend/src/api/resources/CommercePlan.tspackages/backend/src/api/endpoints/BillingApi.tspackages/backend/src/index.tspackages/backend/src/api/resources/CommerceSubscriptionItem.tspackages/backend/src/api/resources/Webhooks.tspackages/backend/src/api/resources/JSON.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/backend/src/api/resources/index.tspackages/backend/src/api/resources/Deserializer.tspackages/backend/src/api/resources/CommercePlan.tspackages/backend/src/api/endpoints/BillingApi.tspackages/backend/src/index.tspackages/backend/src/api/resources/CommerceSubscriptionItem.tspackages/backend/src/api/resources/Webhooks.tspackages/backend/src/api/resources/JSON.ts
packages/**/index.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use tree-shaking friendly exports
Files:
packages/backend/src/api/resources/index.tspackages/backend/src/index.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/backend/src/api/resources/index.tspackages/backend/src/api/resources/Deserializer.tspackages/backend/src/api/resources/CommercePlan.tspackages/backend/src/api/endpoints/BillingApi.tspackages/backend/src/index.tspackages/backend/src/api/resources/CommerceSubscriptionItem.tspackages/backend/src/api/resources/Webhooks.tspackages/backend/src/api/resources/JSON.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/backend/src/api/resources/index.tspackages/backend/src/api/resources/Deserializer.tspackages/backend/src/api/resources/CommercePlan.tspackages/backend/src/api/endpoints/BillingApi.tspackages/backend/src/index.tspackages/backend/src/api/resources/CommerceSubscriptionItem.tspackages/backend/src/api/resources/Webhooks.tspackages/backend/src/api/resources/JSON.ts
**/index.ts
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
Use index.ts files for clean imports but avoid deep barrel exports
Avoid barrel files (index.ts re-exports) as they can cause circular dependencies
Files:
packages/backend/src/api/resources/index.tspackages/backend/src/index.ts
**/*
⚙️ CodeRabbit configuration file
If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes.
Files:
packages/backend/src/api/resources/index.tspackages/backend/src/api/resources/Deserializer.tspackages/backend/src/api/resources/CommercePlan.tspackages/backend/src/api/endpoints/BillingApi.tspackages/backend/src/index.tspackages/backend/src/api/resources/CommerceSubscriptionItem.tspackages/backend/src/api/resources/Webhooks.tspackages/backend/src/api/resources/JSON.ts
.changeset/**
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Automated releases must use Changesets.
Files:
.changeset/eight-sheep-matter.md
🧬 Code graph analysis (6)
packages/backend/src/api/resources/Deserializer.ts (3)
packages/backend/src/api/resources/JSON.ts (2)
ObjectType(19-74)ObjectType(76-76)packages/backend/src/api/resources/CommerceSubscriptionItem.ts (1)
CommerceSubscriptionItem(10-99)packages/backend/src/index.ts (1)
CommerceSubscriptionItem(147-147)
packages/backend/src/api/resources/CommercePlan.ts (1)
packages/types/src/commerce.ts (1)
CommerceMoneyAmount(1207-1244)
packages/backend/src/api/endpoints/BillingApi.ts (2)
packages/backend/src/api/resources/CommerceSubscriptionItem.ts (1)
CommerceSubscriptionItem(10-99)packages/backend/src/index.ts (1)
CommerceSubscriptionItem(147-147)
packages/backend/src/api/resources/CommerceSubscriptionItem.ts (3)
packages/backend/src/api/resources/JSON.ts (1)
CommerceSubscriptionItemJSON(860-876)packages/backend/src/api/resources/CommercePlan.ts (2)
CommerceMoneyAmount(4-9)CommercePlan(15-101)packages/types/src/json.ts (1)
CommerceMoneyAmountJSON(834-839)
packages/backend/src/api/resources/Webhooks.ts (1)
packages/backend/src/api/resources/JSON.ts (3)
CommercePaymentAttemptWebhookEventJSON(923-954)CommerceSubscriptionWebhookEventJSON(959-973)CommerceSubscriptionItemWebhookEventJSON(881-918)
packages/backend/src/api/resources/JSON.ts (1)
packages/types/src/json.ts (4)
CommerceSubscriptionItemJSON(773-794)ClerkResourceJSON(36-40)CommerceMoneyAmountJSON(834-839)CommercePlanJSON(632-658)
⏰ 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 (billing, chrome)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (tanstack-react-router, chrome)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (expo-web, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (quickstart, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (elements, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Unit Tests (18, --filter=@clerk/astro --filter=@clerk/backend --filter=@clerk/express --filter=@c...
- GitHub Check: Unit Tests (22, **)
- GitHub Check: Publish with pkg-pr-new
- GitHub Check: Static analysis
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (10)
.changeset/eight-sheep-matter.md (1)
1-6: No webhook JSON type exports changed—minor bump is correctI’ve verified that there are no added or removed
*JSONtype exports inpackages/backend/src/api/resources/Webhooks.tsorJSON.tscompared tomain. There are no webhook JSON type renames or removals, so this change is non-breaking and the minor version bump remains appropriate.packages/backend/src/api/resources/index.ts (1)
61-61: LGTM: public export for CommerceSubscriptionItem.Re-export aligns with the rest of the resources barrel and keeps import ergonomics consistent. Watch for circulars, but importing the class directly in Deserializer avoids that.
packages/backend/src/api/resources/Deserializer.ts (1)
41-41: Correct choice: direct class import avoids barrel cycles.Importing
CommerceSubscriptionItemfrom its module (not the barrel) prevents potential circular deps during deserialization.packages/backend/src/api/endpoints/BillingApi.ts (1)
5-5: Type-only import is correct.Using a type-only import for the return model avoids accidental runtime dependencies.
packages/backend/src/index.ts (1)
147-148: Exporting CommerceSubscriptionItem from resources looks goodPublicly surfacing the runtime class aligns with the new cancelSubscriptionItem response shape and keeps parity with CommercePlan.
packages/backend/src/api/resources/CommerceSubscriptionItem.ts (1)
84-97: Deserializer mapping LGTM with the tightened typesThe field mapping matches the new JSON contract and uses CommercePlan.fromJSON correctly. After applying the type fixes above, this constructor stays valid.
packages/backend/src/api/resources/Webhooks.ts (2)
68-71: PaymentAttempt webhook now references webhook-specific JSON — goodThe generic event wrapper remains unchanged; switching to
CommercePaymentAttemptWebhookEventJSONaligns types with the new payload.
73-76: Subscription webhook JSON type switch looks correctUsing
CommerceSubscriptionWebhookEventJSONforsubscription.*events matches the revised payload shape.packages/backend/src/api/resources/JSON.ts (2)
860-876: CommerceSubscriptionItemJSON shape: confirm lifetime_paid non-nullability across BAPIThis defines
lifetime_paidas a required money object (non-null). That’s preferable; just ensure all BAPI responses indeed include it (including free plans). If not, either make it nullable or guarantee a zero-amount money value.
959-973: CommerceSubscriptionWebhookEventJSON: items now typed to webhook item JSON — goodThis aligns the nested shape with the new webhook-specific item contract.
| /** | ||
| * @experimental This is an experimental API for the Billing feature that is available under a public beta, and the API is subject to change. | ||
| * It is advised to pin the SDK version to avoid breaking changes. | ||
| */ | ||
| public async cancelSubscriptionItem(subscriptionItemId: string, params?: CancelSubscriptionItemParams) { | ||
| this.requireId(subscriptionItemId); | ||
| return this.request<CommerceSubscriptionItem>({ | ||
| method: 'DELETE', | ||
| path: joinPaths(basePath, 'subscription_items', subscriptionItemId), | ||
| queryParams: params, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Explicit return type required for public API; also ensure endNow is serialized as end_now.
- Per guidelines, public APIs must have explicit return types.
- Unless the transport transforms query params,
end_nowis likely the server-expected shape. Pass a mapped object.
Apply this diff:
/**
* @experimental This is an experimental API for the Billing feature that is available under a public beta, and the API is subject to change.
* It is advised to pin the SDK version to avoid breaking changes.
*/
- public async cancelSubscriptionItem(subscriptionItemId: string, params?: CancelSubscriptionItemParams) {
+ public async cancelSubscriptionItem(
+ subscriptionItemId: string,
+ params?: CancelSubscriptionItemParams,
+ ): Promise<CommerceSubscriptionItem> {
this.requireId(subscriptionItemId);
- return this.request<CommerceSubscriptionItem>({
+ const queryParams =
+ params && typeof params.endNow !== 'undefined'
+ ? { end_now: params.endNow }
+ : undefined;
+ return this.request<CommerceSubscriptionItem>({
method: 'DELETE',
path: joinPaths(basePath, 'subscription_items', subscriptionItemId),
- queryParams: params,
+ queryParams,
});
}Optional: enrich JSDoc with param details and an example:
/**
* Cancels a subscription item.
* @param subscriptionItemId The subscription item id.
* @param params Optional cancellation options. When `endNow` is true, cancellation is immediate; otherwise effective at period end.
* @returns The updated CommerceSubscriptionItem.
*/Also, please add tests that assert:
- Path:
/commerce/subscription_items/{id} - Query serialization:
end_now=true|false - Response mapped to
CommerceSubscriptionItemvia Deserializer
I can provide a test stub using the existing request mock harness.
🤖 Prompt for AI Agents
In packages/backend/src/api/endpoints/BillingApi.ts around lines 36 to 47, the
public method cancelSubscriptionItem lacks an explicit return type and does not
serialize the params.endNow to the server-expected end_now query param; update
the method signature to declare an explicit return type
Promise<CommerceSubscriptionItem>, map params (e.g. const queryParams = params ?
{ end_now: params.endNow } : undefined) and pass that to request, and add the
suggested JSDoc block above the method; also add tests that assert the request
path is /commerce/subscription_items/{id}, that query serialization produces
end_now=true|false, and that the response is deserialized to
CommerceSubscriptionItem using the existing request mock harness.
| * The current amount for the subscription item. | ||
| */ | ||
| readonly amount: CommerceMoneyAmount | null | undefined, | ||
| /** |
There was a problem hiding this comment.
Narrow the amount type to match JSON (avoid undefined)
JSON shape defines amount: CommerceMoneyAmountJSON | null, not undefined. Allowing undefined here expands the public API unnecessarily and mismatches the deserializer contract.
Apply:
- readonly amount: CommerceMoneyAmount | null | undefined,
+ readonly amount: CommerceMoneyAmount | null,📝 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.
| * The current amount for the subscription item. | |
| */ | |
| readonly amount: CommerceMoneyAmount | null | undefined, | |
| /** | |
| * The current amount for the subscription item. | |
| */ | |
| readonly amount: CommerceMoneyAmount | null, | |
| /** |
🤖 Prompt for AI Agents
In packages/backend/src/api/resources/CommerceSubscriptionItem.ts around lines
36–39, the `amount` field is typed as `CommerceMoneyAmount | null | undefined`,
which permits `undefined` but the JSON contract is `CommerceMoneyAmountJSON |
null`; remove `undefined` and narrow the type to match the JSON contract (i.e.,
use `CommerceMoneyAmountJSON | null`), and update any related
serializer/deserializer or callers to treat absent values as `null` per the JSON
shape.
| * The lifetime amount paid for this subscription item. | ||
| */ | ||
| readonly lifetimePaid?: CommerceMoneyAmount | null, | ||
| ) {} |
There was a problem hiding this comment.
lifetimePaid should be required and non-null to match JSON
CommerceSubscriptionItemJSON.lifetime_paid is required and non-null. Making lifetimePaid optional (and nullable) diverges and forces extra undefined/null checks downstream.
Apply:
- readonly lifetimePaid?: CommerceMoneyAmount | null,
+ readonly lifetimePaid: CommerceMoneyAmount,📝 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.
| * The lifetime amount paid for this subscription item. | |
| */ | |
| readonly lifetimePaid?: CommerceMoneyAmount | null, | |
| ) {} | |
| /** | |
| * The lifetime amount paid for this subscription item. | |
| */ | |
| readonly lifetimePaid: CommerceMoneyAmount, |
🤖 Prompt for AI Agents
In packages/backend/src/api/resources/CommerceSubscriptionItem.ts around lines
60 to 63, the lifetimePaid property is declared as optional and nullable but the
JSON model (CommerceSubscriptionItemJSON.lifetime_paid) is required and
non-null; change the class/property signature to make lifetimePaid required and
non-null (remove the "?" and "| null"), update any constructor parameters and
assignments to expect a non-null CommerceMoneyAmount, and adjust
parsing/serialization code to validate and map
CommerceSubscriptionItemJSON.lifetime_paid into this required property (throw or
surface an error if the JSON field is missing/null).
| function formatAmountJSON( | ||
| amount: CommerceMoneyAmountJSON | null | undefined, | ||
| ): CommerceMoneyAmount | null | undefined; | ||
| function formatAmountJSON( | ||
| amount: CommerceMoneyAmountJSON | null | undefined, | ||
| ): CommerceMoneyAmount | null | undefined { | ||
| if (!amount) { | ||
| return amount; | ||
| } | ||
|
|
||
| return { | ||
| amount: amount.amount, | ||
| amountFormatted: amount.amount_formatted, | ||
| currency: amount.currency, | ||
| currencySymbol: amount.currency_symbol, | ||
| }; | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify and type the amount formatter overloads precisely
- The current overload is redundant and permits
undefinedwhich the JSON never returns. - Tighten types and keep one public overload for nullability plus a concrete implementation.
Apply:
- function formatAmountJSON(
- amount: CommerceMoneyAmountJSON | null | undefined,
- ): CommerceMoneyAmount | null | undefined;
- function formatAmountJSON(
- amount: CommerceMoneyAmountJSON | null | undefined,
- ): CommerceMoneyAmount | null | undefined {
- if (!amount) {
- return amount;
- }
-
- return {
- amount: amount.amount,
- amountFormatted: amount.amount_formatted,
- currency: amount.currency,
- currencySymbol: amount.currency_symbol,
- };
- }
+ function formatAmountJSON(amount: CommerceMoneyAmountJSON): CommerceMoneyAmount;
+ function formatAmountJSON(amount: CommerceMoneyAmountJSON | null): CommerceMoneyAmount | null;
+ function formatAmountJSON(amount: CommerceMoneyAmountJSON | null): CommerceMoneyAmount | null {
+ if (amount === null) {
+ return null;
+ }
+ return {
+ amount: amount.amount,
+ amountFormatted: amount.amount_formatted,
+ currency: amount.currency,
+ currencySymbol: amount.currency_symbol,
+ };
+ }And adjust the constructor call sites accordingly (type inference will keep lifetimePaid non-null while allowing amount to be null).
📝 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.
| function formatAmountJSON( | |
| amount: CommerceMoneyAmountJSON | null | undefined, | |
| ): CommerceMoneyAmount | null | undefined; | |
| function formatAmountJSON( | |
| amount: CommerceMoneyAmountJSON | null | undefined, | |
| ): CommerceMoneyAmount | null | undefined { | |
| if (!amount) { | |
| return amount; | |
| } | |
| return { | |
| amount: amount.amount, | |
| amountFormatted: amount.amount_formatted, | |
| currency: amount.currency, | |
| currencySymbol: amount.currency_symbol, | |
| }; | |
| } | |
| function formatAmountJSON(amount: CommerceMoneyAmountJSON): CommerceMoneyAmount; | |
| function formatAmountJSON(amount: CommerceMoneyAmountJSON | null): CommerceMoneyAmount | null; | |
| function formatAmountJSON(amount: CommerceMoneyAmountJSON | null): CommerceMoneyAmount | null { | |
| if (amount === null) { | |
| return null; | |
| } | |
| return { | |
| amount: amount.amount, | |
| amountFormatted: amount.amount_formatted, | |
| currency: amount.currency, | |
| currencySymbol: amount.currency_symbol, | |
| }; | |
| } |
🤖 Prompt for AI Agents
In packages/backend/src/api/resources/CommerceSubscriptionItem.ts around lines
66 to 83, the amount formatter currently has a redundant overload that allows
undefined (which the JSON never returns) and duplicates the signature; replace
both overloads with a single exported function signature that accepts
CommerceMoneyAmountJSON | null (no undefined) and returns CommerceMoneyAmount |
null, keep a single concrete implementation that handles null and maps amount,
amount_formatted, currency, currency_symbol to the camelCased fields, and then
update all constructor/call sites to pass non-undefined JSON (or null) so type
inference keeps fields like lifetimePaid non-null while still allowing amount to
be null.
| CommercePaymentAttemptWebhookEvent, | ||
| CommerceSubscriptionWebhookEvent, | ||
| CommerceSubscriptionItemWebhookEvent, | ||
| } from './api/resources/Webhooks'; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Webhook event type re-exports: verify event set coverage and SemVer impact
- Coverage: Do we intentionally omit a 'subscriptionItem.expired' event even though 'expired' exists in CommerceSubscriptionItemStatus? If 'expired' is a real webhook type, consider adding it to the union for completeness; if it’s not emitted, we’re good. Please confirm with BAPI/webhook docs.
- SemVer: This PR also removes previously exported JSON types (e.g., CommerceSubscriptionJSON, CommercePaymentAttemptJSON, CommercePayerJSON) from the public surface. If those were published before, that’s a breaking change. Consider shipping deprecation aliases for one release cycle or bumping major.
Proposed non-breaking aliases (deprecate old names, point to new webhook-specific JSONs):
+/**
+ * @deprecated Use CommerceSubscriptionWebhookEventJSON instead. Will be removed in the next major.
+ */
+export type {
+ CommerceSubscriptionWebhookEventJSON as CommerceSubscriptionJSON,
+ CommercePaymentAttemptWebhookEventJSON as CommercePaymentAttemptJSON,
+} from './api/resources/JSON';
+
+/**
+ * @deprecated This type is still referenced by webhook JSON payloads, but we no longer recommend importing it from the backend package.
+ * Consider removing in the next major.
+ */
+export type { CommercePayerJSON } from './api/resources/JSON';To check for potential breakages in consumers inside this repo:
🏁 Script executed:
#!/bin/bash
# Find usages of removed type names across the monorepo
rg -nP -C2 '\b(CommerceSubscriptionJSON|CommercePaymentAttemptJSON|CommercePayerJSON)\b'Length of output: 5038
SemVer-breaking removal of JSON types – deprecate or bump major
Confirmed that the following JSON interfaces are still referenced across the monorepo (in packages/types, packages/backend, and packages/clerk-js), so removing their re-exports from the public surface will be a breaking change:
- CommerceSubscriptionJSON (used to describe top-level subscriptions)
- CommercePaymentAttemptJSON
- CommercePayerJSON
Please address this before merging by either:
- Shipping one release with deprecation aliases, pointing the old names to the new JSON types, then removing them in the next major; or
- Bumping the major version immediately to acknowledge the breaking change.
Locations to update (in packages/backend/src/index.ts):
- After the existing
export { … } from './api/resources/Webhooks';block
Suggested deprecation alias diff:
// keep existing webhook re-exports
export {
CommercePaymentAttemptWebhookEvent,
CommerceSubscriptionWebhookEvent,
CommerceSubscriptionItemWebhookEvent,
} from './api/resources/Webhooks';
+/**
+ * @deprecated Use CommerceSubscriptionWebhookEventJSON instead. Will be removed in the next major.
+ */
+export type {
+ CommerceSubscriptionWebhookEventJSON as CommerceSubscriptionJSON,
+ CommercePaymentAttemptWebhookEventJSON as CommercePaymentAttemptJSON,
+} from './api/resources/JSON';
+
+/**
+ * @deprecated Use CommercePayerWebhookEventJSON instead. Will be removed in the next major.
+ */
+export type { CommercePayerJSON } from './api/resources/JSON';Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/backend/src/index.ts around lines 167 to 170, the public re-exports
removed the CommerceSubscriptionJSON, CommercePaymentAttemptJSON, and
CommercePayerJSON types which are still referenced elsewhere; restore
compatibility by adding deprecation aliases that re-export the old names
pointing to the new JSON types (add export statements for
CommerceSubscriptionJSON, CommercePaymentAttemptJSON, CommercePayerJSON that
forward to the current types and mark them with a deprecated JSDoc comment),
commit that change for a deprecation release, or alternatively, if you intend to
remove them now, bump the package major version to reflect the breaking change.
Description
While implementing this addition I discovered that our BAPI endpoints return different payloads than the ones consumed by our webhook events. Thus I had to I renamed the existing ones by adding
*WebhookEvent*in their names.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit