Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 0e036d2 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 |
@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: |
📝 WalkthroughWalkthroughAdds seat-based and per-unit billing support across the codebase: new JSON interfaces and shared types for unit prices, tiers, per-unit totals, and subscription seats; BillingPlan gains 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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
📒 Files selected for processing (6)
.changeset/cute-ideas-appear.mdpackages/clerk-js/src/core/resources/BillingPlan.tspackages/clerk-js/src/core/resources/BillingSubscription.tspackages/clerk-js/src/utils/billing.tspackages/shared/src/types/billing.tspackages/shared/src/types/json.ts
| 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), | ||
| })), | ||
| })); | ||
| }; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
📒 Files selected for processing (7)
packages/clerk-js/sandbox/scenarios/index.tspackages/clerk-js/sandbox/scenarios/pricing-table-sbb.tspackages/localizations/src/en-US.tspackages/shared/src/types/localization.tspackages/ui/src/components/PricingTable/PricingTable.tsxpackages/ui/src/components/PricingTable/PricingTableDefault.tsxpackages/ui/src/components/PricingTable/__tests__/PricingTable.test.tsx
✅ Files skipped from review due to trivial changes (1)
- packages/ui/src/components/PricingTable/PricingTable.tsx
| setClerkState({ | ||
| environment: EnvironmentService.MULTI_SESSION, | ||
| session, | ||
| user, | ||
| }); | ||
|
|
||
| const subscriptionHandler = http.get('https://*.clerk.accounts.dev/v1/me/billing/subscription', () => { | ||
| return HttpResponse.json({ | ||
| response: { | ||
| data: {}, | ||
| }, | ||
| }); | ||
| }); |
There was a problem hiding this comment.
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.
| const feePeriodText = React.useMemo(() => { | ||
| if (!plan.hasBaseFee && plan.unitPrices) { | ||
| return localizationKeys('billing.monthPerUnit', { unitName: plan.unitPrices[0].name }); | ||
| } | ||
|
|
||
| return localizationKeys('billing.month'); | ||
| }, [plan.unitPrices]); |
There was a problem hiding this comment.
Don't index unitPrices without a non-empty guard.
BillingPlanResource.unitPrices is an optional array, not a non-empty tuple. If a plan comes back with unitPrices: [] and hasBaseFee: false, this render path throws on plan.unitPrices[0]; the later CardFeaturesList gate has the same failure mode.
🤖 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
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.
1 participant
Description
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
UI
Localization
Tests