Skip to content

feat(clerk-js): Remove dummy default free plan subscription #6109

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.

Sign up for GitHub

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
+13 −55
Draft

feat(clerk-js): Remove dummy default free plan subscription #6109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/clerk-js/src/core/modules/commerce/CommerceBilling.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export class CommerceBilling implements CommerceBillingNamespace {
const { orgId, ...rest } = params;

return await BaseResource._fetch({
path: orgId ? `/organizations/${orgId}/commerce/subscriptions` : `/me/commerce/subscriptions`,
path: orgId ? `/organizations/${orgId}/commerce/subscription_items` : `/me/commerce/subscription_items`,
method: 'GET',
search: convertPageToOffsetSearchParams(rest),
}).then(res => {
Expand Down
4 changes: 2 additions & 2 deletions packages/clerk-js/src/core/resources/CommerceSubscription.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ export class CommerceSubscription extends BaseResource implements CommerceSubscr
const json = (
await BaseResource._fetch({
path: orgId
? `/organizations/${orgId}/commerce/subscriptions/${this.id}`
: `/me/commerce/subscriptions/${this.id}`,
? `/organizations/${orgId}/commerce/subscription_items/${this.id}`
: `/me/commerce/subscription_items/${this.id}`,
method: 'DELETE',
})
)?.response as unknown as DeletedObjectJSON;
Expand Down
3 changes: 2 additions & 1 deletion packages/clerk-js/src/ui/components/Plans/PlanDetails.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 || {};
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the pattern we currently use for destructuring paginated SWR responses in PaymentSources, so I used it in these changes – any objections? @panteliselef


const isImplicitlyActiveOrUpcoming = isDefaultPlanImplicitlyActiveOrUpcoming && plan.isDefault;

Expand Down
3 changes: 2 additions & 1 deletion packages/clerk-js/src/ui/components/PricingTable/PricingTable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ const PricingTableRoot = (props: PricingTableProps) => {
const clerk = useClerk();
const { mode = 'mounted' } = usePricingTableContext();
const isCompact = mode === 'modal';
const { data: subscriptions } = useSubscriptions();
const { data } = useSubscriptions();
const { data: subscriptions = [] } = data || {};
const { data: plans } = usePlans();
const { handleSelectPlan } = usePlansContext();

Expand Down
3 changes: 2 additions & 1 deletion packages/clerk-js/src/ui/components/Subscriptions/SubscriptionsList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ export function SubscriptionsList({
}) {
const { handleSelectPlan, captionForSubscription, canManageSubscription } = usePlansContext();
const subscriberType = useSubscriberTypeContext();
const { data: subscriptions } = useSubscriptions();
const { data } = useSubscriptions();
const { data: subscriptions = [] } = data || {};
const canManageBilling = useProtect(
Comment on lines +38 to 40
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

Suggested change
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.

has => has({ permission: 'org:sys_billing:manage' }) || subscriberType === 'user',
);
Expand Down
53 changes: 4 additions & 49 deletions packages/clerk-js/src/ui/contexts/components/Plans.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import type {
import { useCallback, useMemo } from 'react';
import useSWR from 'swr';

import { CommerceSubscription } from '../../../core/resources/internal';
import type { LocalizationKey } from '../../localization';
import { localizationKeys } from '../../localization';
import { getClosestProfileScrollBox } from '../../utils';
Expand Down Expand Up @@ -80,11 +79,10 @@ export const useStatements = () => {
export const useSubscriptions = () => {
const { billing } = useClerk();
const { organization } = useOrganization();
const { user, isSignedIn } = useUser();
const { user } = useUser();
const subscriberType = useSubscriberTypeContext();
const { data: plans } = usePlans();

const { data: _subscriptions, ...rest } = useSWR(
return useSWR(
{
key: `commerce-subscriptions`,
userId: user?.id,
Expand All @@ -93,42 +91,6 @@ export const useSubscriptions = () => {
({ args, userId }) => (userId ? billing.getSubscriptions(args) : undefined),
dedupeOptions,
);

const subscriptions = useMemo(() => {
if (!_subscriptions) {
return [];
}
const defaultFreePlan = plans?.find(plan => plan.hasBaseFee === false && plan.amount === 0);

// are we signed in, is there a default free plan, and should it be shown as active or upcoming? then add an implicit subscription
if (
isSignedIn &&
defaultFreePlan &&
(_subscriptions.data.length === 0 || !_subscriptions.data.some(subscription => !subscription.canceledAt))
) {
const canceledSubscription = _subscriptions.data.find(subscription => subscription.canceledAt);
return [
..._subscriptions.data,
new CommerceSubscription({
object: 'commerce_subscription',
id: '__implicit_default_plan_subscription__',
payment_source_id: '',
plan: defaultFreePlan.__internal_toSnapshot(),
plan_period: 'month',
canceled_at: null,
status: _subscriptions.data.length === 0 ? 'active' : 'upcoming',
period_start: canceledSubscription?.periodEnd || 0,
period_end: 0,
}),
];
}
return _subscriptions.data;
}, [_subscriptions, plans, isSignedIn]);

return {
data: subscriptions,
...rest,
};
};

export const usePlans = () => {
Expand Down Expand Up @@ -172,7 +134,8 @@ export const usePlansContext = () => {
return false;
}, [clerk, subscriberType]);

const { data: subscriptions, mutate: mutateSubscriptions } = useSubscriptions();
const { data, mutate: mutateSubscriptions } = useSubscriptions();
const { data: subscriptions = [] } = data || {};

Comment on lines +137 to 139
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

// Invalidates cache but does not fetch immediately
const { data: plans, mutate: mutatePlans } = useSWR<Awaited<ReturnType<typeof clerk.billing.getPlans>>>({
Expand Down Expand Up @@ -251,13 +214,6 @@ export const usePlansContext = () => {
[activeOrUpcomingSubscription],
);

// should the default plan be shown as active
const upcomingSubscriptionsExist = useMemo(() => {
return (
subscriptions.some(subscription => subscription.status === 'upcoming') || isDefaultPlanImplicitlyActiveOrUpcoming
);
}, [subscriptions, isDefaultPlanImplicitlyActiveOrUpcoming]);

// return the CTA button props for a plan
const buttonPropsForPlan = useCallback(
({
Expand Down Expand Up @@ -411,7 +367,6 @@ export const usePlansContext = () => {
buttonPropsForPlan,
canManageSubscription,
captionForSubscription,
upcomingSubscriptionsExist,
defaultFreePlan,
revalidateAll,
};
Expand Down
Loading