feat(clerk-js): Remove dummy default free plan subscription #6109
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 WalkthroughWalkthroughThe changes update API endpoint paths for subscription-related methods, enhance safety in extracting subscription data from hooks by providing default fallbacks, and simplify subscription logic by removing implicit default plan handling. The Changes
✨ Finishing Touches
🪧 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: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
packages/clerk-js/src/ui/contexts/components/Plans.tsx (1)
85-93:⚠️ Potential issueFetcher never fires for organization subscriptions
billing.getSubscriptionsis only invoked whenuserIdis truthy:({ args, userId }) => (userId ? billing.getSubscriptions(args) : undefined)For organization contexts (
subscriberType === 'org')userIdisundefined, so the fetcher returnsundefinedand SWR will never request the organization’s subscriptions—even thoughorgIdis available.-({ args, userId }) => (userId ? billing.getSubscriptions(args) : undefined) +({ args, userId }) => + userId || args.orgId ? billing.getSubscriptions(args) : undefinedWithout this, any org-level billing UI will think the org has no subscriptions.
🧹 Nitpick comments (4)
packages/clerk-js/src/ui/components/PricingTable/PricingTable.tsx (1)
14-16: Doubledatashadowing – prefer explicit namesTwo nested objects called
data(SWRResponse.dataand the paginated payload’sdata) forces the unpopulardata.dataaccess and hurts readability.-const { data } = useSubscriptions(); -const { data: subscriptions = [] } = data || {}; +const swr = useSubscriptions(); // SWR wrapper +const subscriptions = swr?.data?.data ?? []; // payloadUsing explicit identifiers removes the shadowing and instantly conveys which object is which.
packages/clerk-js/src/core/resources/CommerceSubscription.ts (1)
55-56: Hard-coded endpoint string – extract to a shared constant
'/commerce/subscription_items'is now duplicated here and inCommerceBilling.getSubscriptions. Consolidating into a constant (e.g.SUBSCRIPTION_ITEMS_PATH) keeps the code DRY and shields you from the next rename.// constants.ts export const SUBSCRIPTION_ITEMS_PATH = '/commerce/subscription_items';path: orgId ? `/organizations/${orgId}${SUBSCRIPTION_ITEMS_PATH}/${this.id}` : `/me${SUBSCRIPTION_ITEMS_PATH}/${this.id}`,packages/clerk-js/src/ui/components/Plans/PlanDetails.tsx (1)
350-352: Rename variables to avoiddata.dataconfusionSame pattern as in
PricingTable: shadoweddatavariables make the code harder to follow.-const { data } = useSubscriptions(); -const { data: subscriptions = [] } = data || {}; +const swr = useSubscriptions(); +const subscriptions = swr?.data?.data ?? [];packages/clerk-js/src/core/modules/commerce/CommerceBilling.ts (1)
48-48: Duplicate endpoint literal
'/commerce/subscription_items'is hard-coded here and inCommerceSubscription.cancel. Use a shared constant to avoid divergence:const SUBSCRIPTION_ITEMS_PATH = '/commerce/subscription_items'; path: orgId ? `/organizations/${orgId}${SUBSCRIPTION_ITEMS_PATH}` : `/me${SUBSCRIPTION_ITEMS_PATH}`,
📜 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/core/resources/CommerceSubscription.ts(1 hunks)packages/clerk-js/src/ui/components/Plans/PlanDetails.tsx(1 hunks)packages/clerk-js/src/ui/components/PricingTable/PricingTable.tsx(1 hunks)packages/clerk-js/src/ui/components/Subscriptions/SubscriptionsList.tsx(1 hunks)packages/clerk-js/src/ui/contexts/components/Plans.tsx(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
packages/clerk-js/src/ui/components/Subscriptions/SubscriptionsList.tsx (1)
packages/clerk-js/src/ui/contexts/components/Plans.tsx (1)
useSubscriptions(79-94)
packages/clerk-js/src/ui/components/PricingTable/PricingTable.tsx (1)
packages/clerk-js/src/ui/contexts/components/Plans.tsx (1)
useSubscriptions(79-94)
packages/clerk-js/src/ui/components/Plans/PlanDetails.tsx (1)
packages/clerk-js/src/ui/contexts/components/Plans.tsx (1)
useSubscriptions(79-94)
packages/clerk-js/src/ui/contexts/components/Plans.tsx (1)
packages/clerk-js/src/ui/contexts/components/SubscriberType.ts (1)
useSubscriberTypeContext(6-6)
⏰ Context from checks skipped due to timeout of 90000ms (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)
| const { data } = useSubscriptions(); | ||
| const { data: subscriptions = [] } = data || {}; | ||
| const canManageBilling = useProtect( |
There was a problem hiding this comment.
In-place sort mutates SWR cache – clone first
Later you call subscriptions.sort(...), which mutates the array coming directly from SWR.
That mutates the cached value for every consumer and can lead to UI inconsistencies.
-const sortedSubscriptions = subscriptions.sort((a, b) => {
+const sortedSubscriptions = [...subscriptions].sort((a, b) => {While here, consider the same “double data” shadowing fix suggested for PricingTable.tsx.
📝 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.
| const { data } = useSubscriptions(); | |
| const { data: subscriptions = [] } = data || {}; | |
| const canManageBilling = useProtect( | |
| const { data } = useSubscriptions(); | |
| const { data: subscriptions = [] } = data || {}; | |
| // Clone before sorting so we don’t mutate the SWR cache | |
| const sortedSubscriptions = [...subscriptions].sort((a, b) => { | |
| // existing sort logic here | |
| }); |
🤖 Prompt for AI Agents
In packages/clerk-js/src/ui/components/Subscriptions/SubscriptionsList.tsx
around lines 38 to 40, the code destructures subscriptions directly from the SWR
data and then sorts it in place, which mutates the SWR cache and can cause UI
inconsistencies. To fix this, create a shallow copy of the subscriptions array
before sorting it to avoid mutating the cached data. Also, apply the "double
data" shadowing pattern by renaming the destructured data to avoid variable
shadowing and improve clarity.
| const { data, mutate: mutateSubscriptions } = useSubscriptions(); | ||
| const { data: subscriptions = [] } = data || {}; | ||
|
|
There was a problem hiding this comment.
subscriptions is always [] due to wrong destructuring
useSubscriptions() already returns an SWR response; you grab its data here:
const { data, mutate: mutateSubscriptions } = useSubscriptions();data is the array (or undefined).
The follow-up line tries to destructure another data property from that array:
const { data: subscriptions = [] } = data || {};Since arrays don’t have a data field, subscriptions is invariably [], breaking every consumer downstream (e.g. plan buttons, default-plan logic).
Replace with a simple null-coalesce:
-const { data: subscriptions = [] } = data || {};
+const subscriptions = data ?? [];Unit tests around isDefaultPlanImplicitlyActiveOrUpcoming, button states, etc. should be updated once this is fixed.
🤖 Prompt for AI Agents
In packages/clerk-js/src/ui/contexts/components/Plans.tsx around lines 137 to
139, the destructuring of subscriptions is incorrect because it tries to extract
a data property from an array, which always results in an empty array. Fix this
by directly assigning subscriptions to data or an empty array using nullish
coalescing, like const subscriptions = data ?? []; then update related unit
tests to reflect this corrected logic.
| @@ -347,7 +347,8 @@ const Header = React.forwardRef<HTMLDivElement, HeaderProps>((props, ref) => { | |||
| const { plan, subscription, closeSlot, planPeriod, setPlanPeriod } = props; | |||
|
|
|||
| const { captionForSubscription, isDefaultPlanImplicitlyActiveOrUpcoming } = usePlansContext(); | |||
| const { data: subscriptions } = useSubscriptions(); | |||
| const { data } = useSubscriptions(); | |||
| const { data: subscriptions = [] } = data || {}; | |||
There was a problem hiding this comment.
This is the pattern we currently use for destructuring paginated SWR responses in PaymentSources, so I used it in these changes – any objections? @panteliselef
aeliox
changed the title
subscription_items endpoints
Description
Migrates to using new
/subscription_itemsendpoints.Also relies on FAPI to handle default free plan subscriptions.
Depends on https://github.com/clerk/clerk_go/pull/12163
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit