Skip to content

feat(*): Seat-based billing support#8006

Merged
dstaley merged 29 commits intomainfrom
feat/seat-based-billing
Mar 31, 2026
Merged

feat(*): Seat-based billing support#8006
dstaley merged 29 commits intomainfrom
feat/seat-based-billing

Conversation

@dstaley
Copy link
Copy Markdown
Member

@dstaley dstaley commented Mar 6, 2026

Description

This PR updates our components to support rendering details for seat-based billing plans. Seat-based plans support two mechanisms of pricing: seat limits, and per-seat costs. This PR implements across-the-board support for seat limits, and partial support for per-seat costs (in the PricingTable component).

Checklist

  • pnpm test runs as expected.
  • pnpm build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

Summary by CodeRabbit

  • New Features

    • Seat-based billing for subscription items and per-unit pricing for plans
    • Per-unit cost breakdown added to checkout totals
  • UI

    • Pricing table displays per-seat pricing, capacity labels, unit-based fee periods, and seat-cost details
    • Checkout and line-item titles support multi-item plan descriptions (e.g., included seats + billing period)
    • Organization profile shows seat-usage bar with current usage vs. limit
    • New sandbox scenarios demonstrating complex pricing table and org seat-limit
  • Localization

    • Added month/year abbreviations, per-unit strings, and seat-cost tooltip keys
  • Tests

    • Added tests covering seat-tier rendering and tooltips in the pricing table
  • Chores

    • Release changeset added (minor version bumps)

@vercel
Copy link
Copy Markdown

vercel bot commented Mar 6, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
clerk-js-sandbox Ready Ready Preview, Comment Mar 31, 2026 7:41pm

Request Review

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Mar 6, 2026

🦋 Changeset detected

Latest commit: 0604eb3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 21 packages
Name Type
@clerk/localizations Minor
@clerk/clerk-js Minor
@clerk/shared Minor
@clerk/ui Minor
@clerk/react Patch
@clerk/chrome-extension Patch
@clerk/expo Patch
@clerk/agent-toolkit Patch
@clerk/astro Patch
@clerk/backend Patch
@clerk/expo-passkeys Patch
@clerk/express Patch
@clerk/fastify Patch
@clerk/hono Patch
@clerk/msw Patch
@clerk/nextjs Patch
@clerk/nuxt Patch
@clerk/react-router Patch
@clerk/tanstack-react-start Patch
@clerk/testing Patch
@clerk/vue Patch

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

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new bot commented Mar 6, 2026

Open in StackBlitz

@clerk/agent-toolkit

npm i https://pkg.pr.new/@clerk/agent-toolkit@8006

@clerk/astro

npm i https://pkg.pr.new/@clerk/astro@8006

@clerk/backend

npm i https://pkg.pr.new/@clerk/backend@8006

@clerk/chrome-extension

npm i https://pkg.pr.new/@clerk/chrome-extension@8006

@clerk/clerk-js

npm i https://pkg.pr.new/@clerk/clerk-js@8006

@clerk/dev-cli

npm i https://pkg.pr.new/@clerk/dev-cli@8006

@clerk/expo

npm i https://pkg.pr.new/@clerk/expo@8006

@clerk/expo-passkeys

npm i https://pkg.pr.new/@clerk/expo-passkeys@8006

@clerk/express

npm i https://pkg.pr.new/@clerk/express@8006

@clerk/fastify

npm i https://pkg.pr.new/@clerk/fastify@8006

@clerk/hono

npm i https://pkg.pr.new/@clerk/hono@8006

@clerk/localizations

npm i https://pkg.pr.new/@clerk/localizations@8006

@clerk/nextjs

npm i https://pkg.pr.new/@clerk/nextjs@8006

@clerk/nuxt

npm i https://pkg.pr.new/@clerk/nuxt@8006

@clerk/react

npm i https://pkg.pr.new/@clerk/react@8006

@clerk/react-router

npm i https://pkg.pr.new/@clerk/react-router@8006

@clerk/shared

npm i https://pkg.pr.new/@clerk/shared@8006

@clerk/tanstack-react-start

npm i https://pkg.pr.new/@clerk/tanstack-react-start@8006

@clerk/testing

npm i https://pkg.pr.new/@clerk/testing@8006

@clerk/ui

npm i https://pkg.pr.new/@clerk/ui@8006

@clerk/upgrade

npm i https://pkg.pr.new/@clerk/upgrade@8006

@clerk/vue

npm i https://pkg.pr.new/@clerk/vue@8006

commit: 0604eb3

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 6, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds seat-based and per-unit billing support across types, JSON shapes, and deserialization: new interfaces for unit prices, tiers, per-unit totals, and subscription seats; BillingPlan gains unitPrices; BillingSubscriptionItem gains seats; checkout totals gain perUnitTotals. Adds billing JSON types and parsing helpers. UI updates: PricingTable renders seat-cost rows (and includes a console.log), PricingTableDefault and CheckoutForm render seat-related details, LineItems.Title accepts multiple description elements, and OrganizationProfile shows a membership seat-usage bar. Adds sandbox scenarios (PricingTableSBB, OrgProfileSeatLimit, CheckoutSeats), tests for seat-tier rendering, adjusts two MSW handlers to wrap responses under response, and adds a changeset.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The pull request title 'feat(*): Seat-based billing support' clearly and concisely summarizes the main change—introducing seat-based billing functionality—which is confirmed by all modified files.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a 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

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/clerk-js/src/utils/billing.ts`:
- Around line 23-33: The change adds a new parser billingPerUnitTotalsFromJSON
(and related types like BillingPerUnitTotalJSON) but no tests were added; add
unit tests that cover the happy-path mapping of per_unit_totals to
BillingPerUnitTotal (verify name, block_size -> blockSize, tiers array,
fee_per_block and total mapped via billingMoneyAmountFromJSON) and
negative/null/optional cases (e.g., missing tiers, null fee_per_block/total) to
detect regressions—place tests exercising the billingPerUnitTotalsFromJSON
function and any upstream parsing that produces per_unit_totals so future
changes to unit_prices/seats/per_unit_totals are guarded.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: b7290789-ebef-42a4-ad61-b568277203b5

📥 Commits

Reviewing files that changed from the base of the PR and between 79d0ecf and f2e39f4.

📒 Files selected for processing (6)
  • .changeset/cute-ideas-appear.md
  • packages/clerk-js/src/core/resources/BillingPlan.ts
  • packages/clerk-js/src/core/resources/BillingSubscription.ts
  • packages/clerk-js/src/utils/billing.ts
  • packages/shared/src/types/billing.ts
  • packages/shared/src/types/json.ts

Comment on lines +23 to +33
const billingPerUnitTotalsFromJSON = (data: BillingPerUnitTotalJSON[]): BillingPerUnitTotal[] => {
return data.map(unitTotal => ({
name: unitTotal.name,
blockSize: unitTotal.block_size,
tiers: unitTotal.tiers.map(tier => ({
quantity: tier.quantity,
feePerBlock: billingMoneyAmountFromJSON(tier.fee_per_block),
total: billingMoneyAmountFromJSON(tier.total),
})),
}));
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Add regression tests for new billing parsers before merge.

This adds new parsing paths (per_unit_totals) and is part of a broader rollout (unit_prices, seats) but no tests were added/updated in this PR context. Please add coverage for happy-path mapping and null/optional field handling to prevent silent contract regressions.

As per coding guidelines: "**/*: If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes."

Also applies to: 70-72

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/clerk-js/src/utils/billing.ts` around lines 23 - 33, The change adds
a new parser billingPerUnitTotalsFromJSON (and related types like
BillingPerUnitTotalJSON) but no tests were added; add unit tests that cover the
happy-path mapping of per_unit_totals to BillingPerUnitTotal (verify name,
block_size -> blockSize, tiers array, fee_per_block and total mapped via
billingMoneyAmountFromJSON) and negative/null/optional cases (e.g., missing
tiers, null fee_per_block/total) to detect regressions—place tests exercising
the billingPerUnitTotalsFromJSON function and any upstream parsing that produces
per_unit_totals so future changes to unit_prices/seats/per_unit_totals are
guarded.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/clerk-js/sandbox/scenarios/pricing-table-sbb.ts`:
- Around line 65-77: The scenario authenticates via
setClerkState(EnvironmentService.MULTI_SESSION, session, user) but the mocked
subscriptionHandler
(http.get('https://*.clerk.accounts.dev/v1/me/billing/subscription')) returns an
empty data object so the PricingTable renders no plans; fix by returning a
realistic subscription shape in subscriptionHandler.response.response.data that
matches the pricing-table visibility tests (include active plan/tiers and seat
information) so plans remain visible, or alternatively remove the
setClerkState/login to make this scenario unauthenticated and keep the empty
subscription response—update the mock in subscriptionHandler or the
authentication setup accordingly.

In `@packages/ui/src/components/PricingTable/PricingTableDefault.tsx`:
- Around line 324-330: The memo for feePeriodText (React.useMemo) and the
CardFeaturesList rendering assume plan.unitPrices has at least one element; if
unitPrices is [] this will throw on plan.unitPrices[0]. Fix by adding a
non-empty guard (e.g., check plan.unitPrices && plan.unitPrices.length > 0)
before accessing index 0 and in the conditional that decides to use
localizationKeys('billing.monthPerUnit', { unitName: ... }). Also update the
useMemo dependency list to include plan.hasBaseFee and either
plan.unitPrices?.length or plan.unitPrices to ensure recalculation when the
array changes, and apply the same non-empty guard before indexing inside
CardFeaturesList.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 807ec30a-ded8-4044-a595-1f0364efcf56

📥 Commits

Reviewing files that changed from the base of the PR and between f2e39f4 and 0e036d2.

📒 Files selected for processing (7)
  • packages/clerk-js/sandbox/scenarios/index.ts
  • packages/clerk-js/sandbox/scenarios/pricing-table-sbb.ts
  • packages/localizations/src/en-US.ts
  • packages/shared/src/types/localization.ts
  • packages/ui/src/components/PricingTable/PricingTable.tsx
  • packages/ui/src/components/PricingTable/PricingTableDefault.tsx
  • packages/ui/src/components/PricingTable/__tests__/PricingTable.test.tsx
✅ Files skipped from review due to trivial changes (1)
  • packages/ui/src/components/PricingTable/PricingTable.tsx

Comment on lines +65 to +77
setClerkState({
environment: EnvironmentService.MULTI_SESSION,
session,
user,
});

const subscriptionHandler = http.get('https://*.clerk.accounts.dev/v1/me/billing/subscription', () => {
return HttpResponse.json({
response: {
data: {},
},
});
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

This signed-in sandbox scenario won't actually show the pricing table.

The scenario authenticates a user and then answers billing/subscription with an empty payload. In this PR's own PricingTable visibility tests, signed-in users without a subscription render no plans, so pricing-table-sbb is likely to come up blank instead of exercising the new seat-based states. Either return a subscription shape that keeps plans visible or make this scenario unauthenticated.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/clerk-js/sandbox/scenarios/pricing-table-sbb.ts` around lines 65 -
77, The scenario authenticates via
setClerkState(EnvironmentService.MULTI_SESSION, session, user) but the mocked
subscriptionHandler
(http.get('https://*.clerk.accounts.dev/v1/me/billing/subscription')) returns an
empty data object so the PricingTable renders no plans; fix by returning a
realistic subscription shape in subscriptionHandler.response.response.data that
matches the pricing-table visibility tests (include active plan/tiers and seat
information) so plans remain visible, or alternatively remove the
setClerkState/login to make this scenario unauthenticated and keep the empty
subscription response—update the mock in subscriptionHandler or the
authentication setup accordingly.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a 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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/msw/request-handlers.ts (1)

1109-1137: ⚠️ Potential issue | 🔴 Critical

Inconsistent response shape across branches in the same handler.

The first branch (lines 1116-1121) now wraps data in a response envelope, but the fallback branches (lines 1126-1131 and 1133-1136) still return the flat { data, total_count } structure. This will cause unpredictable API responses depending on which branch executes.

Proposed fix to make all branches consistent
     // Fall back to current membership if it matches
     if (currentMembership && currentOrganization?.id === orgId) {
       return createNoStoreResponse({
-        data: [SessionService.serialize(currentMembership)],
-        total_count: 1,
+        response: {
+          data: [SessionService.serialize(currentMembership)],
+          total_count: 1,
+        },
       });
     }
 
     return createNoStoreResponse({
-      data: [],
-      total_count: 0,
+      response: {
+        data: [],
+        total_count: 0,
+      },
     });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/msw/request-handlers.ts` around lines 1109 - 1137, The handler
registered with http.get for '*/v1/organizations/:orgId/memberships*' returns
inconsistent shapes: the first branch returns createNoStoreResponse({ response:
{ data, total_count } }) while the fallback branches return
createNoStoreResponse({ data, total_count }); make all branches return the same
envelope by updating the fallback returns to wrap their payload in the same
response field (i.e., use createNoStoreResponse({ response: { data:
[SessionService.serialize(currentMembership)], total_count: 1 } }) and
createNoStoreResponse({ response: { data: [], total_count: 0 } })), ensuring
consistency with SessionService.serialize, currentUser, currentMembership and
currentOrganization usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/ui/src/components/OrganizationProfile/OrganizationMembers.tsx`:
- Around line 179-217: The outer Col (elementDescriptor={descriptors.page})
currently lacks positioning, so the inner Box (the seat usage bar using
position: 'absolute') is not anchored correctly; fix this by adding sx={{
position: 'relative' }} to the Col component (the one with
elementDescriptor={descriptors.page} and gap={2}) so the absolute-positioned Box
is positioned relative to that page container.

---

Outside diff comments:
In `@packages/msw/request-handlers.ts`:
- Around line 1109-1137: The handler registered with http.get for
'*/v1/organizations/:orgId/memberships*' returns inconsistent shapes: the first
branch returns createNoStoreResponse({ response: { data, total_count } }) while
the fallback branches return createNoStoreResponse({ data, total_count }); make
all branches return the same envelope by updating the fallback returns to wrap
their payload in the same response field (i.e., use createNoStoreResponse({
response: { data: [SessionService.serialize(currentMembership)], total_count: 1
} }) and createNoStoreResponse({ response: { data: [], total_count: 0 } })),
ensuring consistency with SessionService.serialize, currentUser,
currentMembership and currentOrganization usage.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro

Run ID: 57e44511-254b-488b-8af5-a6266ed6f647

📥 Commits

Reviewing files that changed from the base of the PR and between 0e036d2 and c0052f1.

📒 Files selected for processing (6)
  • packages/clerk-js/sandbox/scenarios/index.ts
  • packages/clerk-js/sandbox/scenarios/org-profile-seat-limit.ts
  • packages/localizations/src/en-US.ts
  • packages/msw/request-handlers.ts
  • packages/shared/src/types/localization.ts
  • packages/ui/src/components/OrganizationProfile/OrganizationMembers.tsx

Comment on lines +179 to +217
{canReadMemberships && !!memberships?.count && organization && organization.maxAllowedMemberships > 0 ? (
<Box
sx={theme => ({
position: 'absolute',
bottom: 0,
left: 0,
right: 0,
backgroundColor: theme.colors.$colorBackground,
borderTop: `1px solid ${theme.colors.$borderAlpha100}`,
paddingInline: theme.space.$4,
height: theme.space.$13,
display: 'flex',
alignItems: 'center',
justifyContent: 'center',
})}
>
<Text
sx={t => ({
display: 'inline-flex',
alignItems: 'center',
gap: t.space.$2,
})}
>
<Icon
icon={Users}
size='md'
colorScheme='neutral'
/>
<Text
as='span'
colorScheme='inherit'
localizationKey={localizationKeys('organizationProfile.start.membershipSeatUsageLabel', {
count: memberships.count + (invitations?.count ?? 0),
limit: organization.maxAllowedMemberships,
})}
/>
</Text>
</Box>
) : null}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if the page descriptor or Col component applies position: relative
echo "=== Checking descriptors.page definition ==="
rg -n "page\s*[:=]" packages/ui/src/customizables/elementDescriptors.ts -A 3

echo "=== Checking Col component default styles ==="
ast-grep --pattern $'const Col = $_($$$)'
rg -n "position.*relative" packages/ui/src/primitives/Col.tsx packages/ui/src/customizables/ -l 2>/dev/null || echo "No direct matches found"

echo "=== Checking if profilePage or page applies relative positioning ==="
rg -nP "profilePage|\.page" packages/ui/src/styledSystem/ packages/ui/src/customizables/ -A 2

Repository: clerk/javascript

Length of output: 568


🏁 Script executed:

# Read the context around line 179 to see the parent container structure
sed -n '150,220p' packages/ui/src/components/OrganizationProfile/OrganizationMembers.tsx | cat -n

# Also check if there's a container div or Col with position: relative
rg -n "position.*relative" packages/ui/src/components/OrganizationProfile/OrganizationMembers.tsx

# Check the full file structure to understand parent containers
wc -l packages/ui/src/components/OrganizationProfile/OrganizationMembers.tsx

Repository: clerk/javascript

Length of output: 2947


🏁 Script executed:

# Read the first 100 lines to see the outer Col definition and its sx prop
head -n 100 packages/ui/src/components/OrganizationProfile/OrganizationMembers.tsx | cat -n

Repository: clerk/javascript

Length of output: 4749


🏁 Script executed:

# Check the descriptors.page definition in elementDescriptors.ts
grep -n "descriptors.page\|'page'" packages/ui/src/customizables/elementDescriptors.ts -B 2 -A 10 | head -n 40

# Check Col component implementation
cat -n packages/ui/src/primitives/Col.tsx | head -n 60

# Search for position: relative in makeCustomizable or Col styling
rg -n "position.*relative" packages/ui/src/ -t ts -t tsx | grep -i "col\|customizable" | head -n 20

Repository: clerk/javascript

Length of output: 463


🏁 Script executed:

# Find where Primitives is imported from in customizables/index.ts
grep -n "Primitives" packages/ui/src/customizables/index.ts -B 5 -A 5

# Find Col definition
fd "\.tsx?$" packages/ui/src/ | xargs grep -l "export.*Col\s*=" | grep -v node_modules | head -n 10

# Search for Col component or primitive
rg -n "^export const Col|^const Col\s*=" packages/ui/src/ -t ts -t tsx | head -n 10

Repository: clerk/javascript

Length of output: 4823


🏁 Script executed:

# Find all files in primitives directory
ls -la packages/ui/src/primitives/

# Search for Col implementation
grep -r "export.*Col" packages/ui/src/primitives/ --include="*.ts" --include="*.tsx"

# Check the structure of Flex.tsx since it was found
head -n 80 packages/ui/src/primitives/Flex.tsx

Repository: clerk/javascript

Length of output: 5085


🏁 Script executed:

# Get the rest of Flex.tsx to see the Col definition
tail -n 50 packages/ui/src/primitives/Flex.tsx

# Also check Box.tsx to see if it applies position: relative
cat packages/ui/src/primitives/Box.tsx

Repository: clerk/javascript

Length of output: 2257


🏁 Script executed:

# Final verification: confirm the outer Col structure around the absolute Box
sed -n '52,70p' packages/ui/src/components/OrganizationProfile/OrganizationMembers.tsx | cat -n

Repository: clerk/javascript

Length of output: 765


Add position: relative to the outer Col to ensure the absolute-positioned seat usage bar is positioned correctly.

The Box with position: absolute at lines 179-217 is a direct child of the outer Col (with elementDescriptor={descriptors.page}), which currently has no position: relative. Without it, the bar will be positioned relative to the nearest positioned ancestor or the viewport rather than the page container, causing incorrect layout.

Add sx={{ position: 'relative' }} to the outer Col at line 53:

<Col
  elementDescriptor={descriptors.page}
  gap={2}
  sx={{ position: 'relative' }}
>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/ui/src/components/OrganizationProfile/OrganizationMembers.tsx`
around lines 179 - 217, The outer Col (elementDescriptor={descriptors.page})
currently lacks positioning, so the inner Box (the seat usage bar using
position: 'absolute') is not anchored correctly; fix this by adding sx={{
position: 'relative' }} to the Col component (the one with
elementDescriptor={descriptors.page} and gap={2}) so the absolute-positioned Box
is positioned relative to that page container.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
packages/ui/src/components/PricingTable/PricingTableDefault.tsx (1)

317-323: ⚠️ Potential issue | 🔴 Critical

Guard unitPrices for non-empty arrays before indexing.

Line 319 and Line 524 still access plan.unitPrices[0] after a truthiness check only. unitPrices: [] is valid for the new plan shape, so a no-base-fee plan with an empty array will throw during render.

Suggested fix
-    if (!plan.hasBaseFee && plan.unitPrices) {
+    if (!plan.hasBaseFee && plan.unitPrices?.length) {
       return localizationKeys('billing.monthPerUnit', { unitName: plan.unitPrices[0].name });
     }
...
-        {plan.unitPrices && (plan.hasBaseFee || plan.unitPrices[0].tiers.length > 0) ? (
+        {plan.unitPrices?.length && (plan.hasBaseFee || plan.unitPrices[0].tiers.length > 0) ? (
           <CardFeaturesListSeatCost plan={plan} />
         ) : null}

Also applies to: 524-526

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/ui/src/components/PricingTable/PricingTableDefault.tsx` around lines
317 - 323, The code indexes plan.unitPrices[0] after only a truthiness check,
which breaks when unitPrices is an empty array; update the guards in
feePeriodText (and any other spots that access plan.unitPrices[0]) to ensure
plan.unitPrices.length > 0 before indexing, and fall back to an appropriate
default (e.g., a generic unitName or the month/year localization) when the array
is empty. Locate occurrences by searching for plan.unitPrices[0] (notably in the
feePeriodText useMemo) and add the length check to the conditional and
dependency array as needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@packages/ui/src/components/PricingTable/PricingTableDefault.tsx`:
- Around line 317-323: The code indexes plan.unitPrices[0] after only a
truthiness check, which breaks when unitPrices is an empty array; update the
guards in feePeriodText (and any other spots that access plan.unitPrices[0]) to
ensure plan.unitPrices.length > 0 before indexing, and fall back to an
appropriate default (e.g., a generic unitName or the month/year localization)
when the array is empty. Locate occurrences by searching for plan.unitPrices[0]
(notably in the feePeriodText useMemo) and add the length check to the
conditional and dependency array as needed.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9a6931e5-cb5f-4a9e-8d5c-eabb103c9ab0

📥 Commits

Reviewing files that changed from the base of the PR and between c0052f1 and 192be6e.

📒 Files selected for processing (8)
  • packages/clerk-js/sandbox/scenarios/index.ts
  • packages/clerk-js/src/core/resources/BillingPlan.ts
  • packages/localizations/src/en-US.ts
  • packages/shared/src/types/billing.ts
  • packages/shared/src/types/json.ts
  • packages/shared/src/types/localization.ts
  • packages/ui/src/components/Checkout/CheckoutForm.tsx
  • packages/ui/src/components/PricingTable/PricingTableDefault.tsx

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a 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

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/clerk-js/sandbox/scenarios/index.ts`:
- Around line 3-5: This change added new barrel re-exports (CheckoutSeats,
OrgProfileSeatLimit, PricingTableSBB) to the sandbox scenarios index.ts which
violates the rule banning index.ts barrels and can cause circular deps; remove
the three export lines from index.ts and instead update any consumers to import
CheckoutSeats, OrgProfileSeatLimit, and PricingTableSBB directly from their
specific files ('./checkout-seats', './org-profile-seat-limit',
'./pricing-table-sbb') so each consumer references the concrete module rather
than the barrel.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro

Run ID: 620bc863-b2e9-443c-9c43-216c1297d078

📥 Commits

Reviewing files that changed from the base of the PR and between 192be6e and d5b41a1.

📒 Files selected for processing (4)
  • packages/clerk-js/sandbox/scenarios/checkout-seats.ts
  • packages/clerk-js/sandbox/scenarios/index.ts
  • packages/ui/src/components/Checkout/CheckoutForm.tsx
  • packages/ui/src/elements/LineItems.tsx

@jacekradko
Copy link
Copy Markdown
Member

jacekradko commented Mar 27, 2026

Code review

Claude review found 2 issues:

  1. Unguarded [0] array access will crash on empty unitPrices. plan.unitPrices is checked for truthiness, but [] is truthy -- accessing [0].name or [0].tiers throws TypeError. The singleUnitPriceTierFee memo already correctly guards with .length !== 1, but two other locations do not.

const feePeriodText = React.useMemo(() => {
if (!plan.hasBaseFee && plan.unitPrices) {
return localizationKeys('billing.monthPerUnit', { unitName: plan.unitPrices[0].name });
}
return plan.fee ? localizationKeys('billing.month') : localizationKeys('billing.year');
}, [plan.unitPrices]);

>
{plan.unitPrices && (plan.hasBaseFee || plan.unitPrices[0].tiers.length > 0) ? (
<CardFeaturesListSeatCost plan={plan} />
) : null}

  1. getSeatUnitPrice searches for 'seats' (plural) but all sandbox mock data uses 'seat' (singular, 7 occurrences). Additionally, CheckoutForm.tsx reimplements the lookup with .toLowerCase() === 'seats' instead of calling the shared utility, creating a second inconsistency. If production data uses singular 'seat', the seat billing feature silently returns null from all utility functions.

export const getSeatUnitPrice = (plan: BillingPlanResource): BillingPlanUnitPrice | null => {
if (!plan.unitPrices?.length) {
return null;
}
const seatUnitPrice = plan.unitPrices.find(unitPrice => unitPrice.name === 'seats');
if (seatUnitPrice) {
return seatUnitPrice;
}
return null;
};

}
const seatUnitPrice = plan.unitPrices?.find(unitPrice => unitPrice.name.toLowerCase() === 'seats');
if (seatUnitPrice && seatUnitPrice.tiers.length === 1 && seatUnitPrice.tiers[0].feePerBlock.amount === 0) {

@dstaley
Copy link
Copy Markdown
Member Author

dstaley commented Mar 27, 2026

Unguarded [0] array access will crash on empty unitPrices

Our API doesn't respond with empty arrays, but there's no harm in adding additional length checks just to be safe!

getSeatUnitPrice searches for 'seats' (plural) but all sandbox mock data uses 'seat' (singular, 7 occurrences)

yup, this is just an artifact of writing the mock data before the actual API implementation. updated!

CheckoutForm.tsx reimplements the lookup with .toLowerCase() === 'seats' instead of calling the shared utility

This was intentional as the plan here is a different API type (the shared helpers use BillingPlanResource, but this plan is the plan field from Checkout). However, the type of unitPrices is the same between the two types, so I updated the shared helper to just expect { unitPrices?: BillingPlanUnitPrice[] } which matches both the BillingPlanResource and the plan field in Checkout.

Copy link
Copy Markdown
Member

@wobsoriano wobsoriano left a comment

Choose a reason for hiding this comment

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

Looks good overall! The separation of seat logic into billingPlanSeats.ts and sandbox scenarios are nice 👀 Left a few comments but nothing really blocking

Comment on lines +210 to +212
localizationKey={localizationKeys('organizationProfile.start.membershipSeatUsageLabel', {
count: memberships.count + (invitations?.count ?? 0),
limit: organization.maxAllowedMemberships,
Copy link
Copy Markdown
Member

@wobsoriano wobsoriano Mar 31, 2026

Choose a reason for hiding this comment

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

❓ Should this use an unfiltered organization-level member total instead of memberships.count I think with search active, memberships.count becomes the filtered result count, so the seat usage banner can show something like 1 of 25 instead of the real usage (correct me if Im wrong 🙏🏼 )

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

really good catch! you're indeed correct. I'll look into how we should obtain an unfiltered count of the organization members.

descriptionElements.push(localizationKeys('billing.billedAnnually'));
}
const seatUnitPrice = getSeatUnitPrice(plan);
if (seatUnitPrice && seatUnitPrice.tiers.length === 1 && seatUnitPrice.tiers[0].feePerBlock.amount === 0) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

consider a helper that codifies the intent:

if(isIncludedOrUnlimitedSeats(seatUnitPrice))

Comment on lines +19 to +30
const isBelowLimit = useMemo(() => {
if (!organization) {
return false;
}

// A value of 0 means unlimited memberships, thus the organization is always below the limit
if (organization.maxAllowedMemberships === 0) {
return true;
}

return organization.membersCount + organization.pendingInvitationsCount < organization.maxAllowedMemberships;
}, [organization]);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: useMemo() isn't doing much for us here, simple computation

}

const formatTierFee = (tier: BillingPlanUnitPrice['tiers'][number]) =>
`${tier.feePerBlock.currencySymbol}${normalizeFormatted(tier.feePerBlock.amountFormatted)}`;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

❓ do we already have a formatting helper that deals w/ symbols?

@dstaley dstaley merged commit 9a00a1c into main Mar 31, 2026
42 checks passed
@dstaley dstaley deleted the feat/seat-based-billing branch March 31, 2026 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants