Conversation
🦋 Changeset detectedLatest commit: 69e1f77 The changes in this PR will be included in the next version bump. This PR includes changesets to release 21 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.
|
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
Contributor
📝 WalkthroughWalkthroughThis pull request adds seat-based billing (SBB) support across the billing system. It introduces new type definitions for unit-price tiers, seat entitlements, and per-unit totals in both TypeScript and JSON formats. The BillingPlan resource now parses unit pricing information, and BillingSubscriptionItem tracks seat quantities. The PricingTable and OrganizationMembers UI components are updated to display seat-based pricing tiers and membership seat usage indicators. New mock scenarios for testing SBB configurations are added to the sandbox. Localization keys for seat cost displays, abbreviations, and membership usage labels are introduced. 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ui/src/components/OrganizationProfile/OrganizationMembers.tsx (1)
37-47:⚠️ Potential issue | 🟠 MajorFix seat-usage gating and remove dead expression (currently hides valid usage and risks lint failure).
Line 47 is a no-op expression, and Lines 180-214 gate visibility on
memberships?.countonly. This hides the bar when invitations exist but memberships are zero, even though invitation counts are included in displayed usage.As per coding guidelines "`**/*.{js,jsx,ts,tsx}`: All code must pass ESLint checks with the project's configuration".Suggested fix
const { membershipRequests, memberships, invitations, organization } = useOrganization({ @@ }); - organization?.maxAllowedMemberships; + const seatUsageCount = (memberships?.count ?? 0) + (invitations?.count ?? 0); @@ - {canReadMemberships && !!memberships?.count && organization?.maxAllowedMemberships && ( + {canReadMemberships && seatUsageCount > 0 && organization?.maxAllowedMemberships != null && ( @@ <Text as='span' colorScheme='inherit' localizationKey={localizationKeys('organizationProfile.start.membershipSeatUsageLabel', { - count: memberships.count + (invitations?.count ?? 0), + count: seatUsageCount, limit: organization.maxAllowedMemberships, })} />Also applies to: 180-214
🤖 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 37 - 47, Remove the dead no-op expression organization?.maxAllowedMemberships and update the seat-usage visibility/gating logic in OrganizationMembers so it considers invitations as well as memberships: replace any checks that only use memberships?.count with a computed total (e.g., totalUsage = (memberships?.count ?? 0) + (invitations?.count ?? 0)) and use totalUsage for showing the usage bar and related gating; keep useOrganization call and preserve membershipRequests/invitations/memberships usage but ensure invitations are not ignored when deciding visibility.
🤖 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/msw/request-handlers.ts`:
- Around line 1117-1120: The memberships endpoint returns inconsistent shapes:
one branch returns an object with response.data (as shown by response: { data:
[SessionService.serialize(membership)], total_count: 1 }) while fallback
branches return top-level data; fix by making all branches return the same shape
— wrap fallback payloads in the same response object (response: { data: [...],
total_count: N }) so every code path in the memberships handler in
packages/msw/request-handlers.ts uses response.data and total_count
consistently.
In `@packages/ui/src/components/PricingTable/PricingTable.tsx`:
- Line 27: Remove the debug console.log in the PricingTable component that
prints plansToRender/plans/subscription; delete the line containing
console.log('plansToRender', { plansToRender, plans, subscription }) in
PricingTable.tsx and either remove the logging entirely or replace it with the
app's centralized logger at an appropriate level (e.g., logger.debug) ensuring
any logged data is sanitized or gated behind a debug/ENV flag so sensitive
billing/subscription payloads are never emitted to the browser console in
production.
In `@packages/ui/src/components/PricingTable/PricingTableDefault.tsx`:
- Around line 325-327: The rendering crashes because code in
PricingTableDefault.tsx accesses plan.unitPrices[0] without ensuring the array
has elements (see the conditional using plan.hasBaseFee and plan.unitPrices and
the similar use at line ~486); update both places to check that
plan.unitPrices.length > 0 (or use a safe optional/default) before reading
.[0].name and fall back to a sensible default/localization when the array is
empty so localizationKeys('billing.monthPerUnit', { unitName: ... }) is never
passed undefined.
---
Outside diff comments:
In `@packages/ui/src/components/OrganizationProfile/OrganizationMembers.tsx`:
- Around line 37-47: Remove the dead no-op expression
organization?.maxAllowedMemberships and update the seat-usage visibility/gating
logic in OrganizationMembers so it considers invitations as well as memberships:
replace any checks that only use memberships?.count with a computed total (e.g.,
totalUsage = (memberships?.count ?? 0) + (invitations?.count ?? 0)) and use
totalUsage for showing the usage bar and related gating; keep useOrganization
call and preserve membershipRequests/invitations/memberships usage but ensure
invitations are not ignored when deciding visibility.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: d68fb351-5af1-41ed-94dd-0146e5515526
📒 Files selected for processing (17)
.changeset/cute-ideas-appear.mdpackages/clerk-js/sandbox/scenarios/index.tspackages/clerk-js/sandbox/scenarios/org-profile-seat-limit.tspackages/clerk-js/sandbox/scenarios/pricing-table-sbb.tspackages/clerk-js/sandbox/template.htmlpackages/clerk-js/src/core/resources/BillingPlan.tspackages/clerk-js/src/core/resources/BillingSubscription.tspackages/clerk-js/src/utils/billing.tspackages/localizations/src/en-US.tspackages/msw/request-handlers.tspackages/shared/src/types/billing.tspackages/shared/src/types/json.tspackages/shared/src/types/localization.tspackages/ui/src/components/OrganizationProfile/OrganizationMembers.tsxpackages/ui/src/components/PricingTable/PricingTable.tsxpackages/ui/src/components/PricingTable/PricingTableDefault.tsxpackages/ui/src/components/PricingTable/__tests__/PricingTable.test.tsx
Comment on lines
+1117
to
+1120
| response: { | ||
| data: [SessionService.serialize(membership)], | ||
| total_count: 1, | ||
| }, |
Contributor
There was a problem hiding this comment.
Inconsistent response schema in the same memberships endpoint
After Line 1117, one code path returns response.data, but fallback paths (Line 1128 and Line 1134) still return top-level data. This makes the endpoint shape nondeterministic and can break consumers at runtime.
Suggested fix
// 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,
+ },
});📝 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
| response: { | |
| data: [SessionService.serialize(membership)], | |
| total_count: 1, | |
| }, | |
| // Fall back to current membership if it matches | |
| if (currentMembership && currentOrganization?.id === orgId) { | |
| return createNoStoreResponse({ | |
| response: { | |
| data: [SessionService.serialize(currentMembership)], | |
| total_count: 1, | |
| }, | |
| }); | |
| } | |
| return createNoStoreResponse({ | |
| 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 1117 - 1120, The memberships
endpoint returns inconsistent shapes: one branch returns an object with
response.data (as shown by response: { data:
[SessionService.serialize(membership)], total_count: 1 }) while fallback
branches return top-level data; fix by making all branches return the same shape
— wrap fallback payloads in the same response object (response: { data: [...],
total_count: N }) so every code path in the memberships handler in
packages/msw/request-handlers.ts uses response.data and total_count
consistently.
| : [] | ||
| : plans; | ||
| }, [clerk.isSignedIn, plans, subscription]); | ||
| console.log('plansToRender', { plansToRender, plans, subscription }); |
Contributor
There was a problem hiding this comment.
Remove debug logging of subscription/plan payloads
Line 27 logs billing/subscription state to the browser console. This risks leaking sensitive account/billing data and should not ship.
Suggested fix
- console.log('plansToRender', { plansToRender, plans, subscription });As per coding guidelines: "Implement proper logging with different levels".
📝 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
| console.log('plansToRender', { plansToRender, plans, subscription }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui/src/components/PricingTable/PricingTable.tsx` at line 27, Remove
the debug console.log in the PricingTable component that prints
plansToRender/plans/subscription; delete the line containing
console.log('plansToRender', { plansToRender, plans, subscription }) in
PricingTable.tsx and either remove the logging entirely or replace it with the
app's centralized logger at an appropriate level (e.g., logger.debug) ensuring
any logged data is sanitized or gated behind a debug/ENV flag so sensitive
billing/subscription payloads are never emitted to the browser console in
production.
Comment on lines
+325
to
+327
| if (!plan.hasBaseFee && plan.unitPrices) { | ||
| return localizationKeys('billing.monthPerUnit', { unitName: plan.unitPrices[0].name }); | ||
| } |
Contributor
There was a problem hiding this comment.
Guard unitPrices[0] access — empty arrays currently crash rendering.
Line 326 and Line 486 assume plan.unitPrices[0] exists when unitPrices is truthy. An empty array will throw at runtime.
Suggested fix
const feePeriodText = React.useMemo(() => {
- if (!plan.hasBaseFee && plan.unitPrices) {
- return localizationKeys('billing.monthPerUnit', { unitName: plan.unitPrices[0].name });
+ const firstUnitPrice = plan.unitPrices?.[0];
+ if (!plan.hasBaseFee && firstUnitPrice) {
+ return localizationKeys('billing.monthPerUnit', { unitName: firstUnitPrice.name });
}
return localizationKeys('billing.month');
- }, [plan.unitPrices]);
+ }, [plan.hasBaseFee, plan.unitPrices]);
@@
- {plan.unitPrices && (plan.hasBaseFee || plan.unitPrices[0].tiers.length > 0) ? (
+ {plan.unitPrices && (plan.hasBaseFee || (plan.unitPrices[0]?.tiers.length ?? 0) > 0) ? (
<CardFeaturesListSeatCost plan={plan} />
) : null}Also applies to: 486-486
🤖 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
325 - 327, The rendering crashes because code in PricingTableDefault.tsx
accesses plan.unitPrices[0] without ensuring the array has elements (see the
conditional using plan.hasBaseFee and plan.unitPrices and the similar use at
line ~486); update both places to check that plan.unitPrices.length > 0 (or use
a safe optional/default) before reading .[0].name and fall back to a sensible
default/localization when the array is empty so
localizationKeys('billing.monthPerUnit', { unitName: ... }) is never passed
undefined.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR offers an end-to-end sandbox for demoing the upcoming phase one implementation of seat-based billing.
Instructions
Summary by CodeRabbit
New Features
Localization