feat(astro): Add Billing buttons #6583
Conversation
🦋 Changeset detectedLatest commit: 7b27c8a The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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/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: |
WalkthroughIntroduces experimental billing UI buttons (Checkout, Plan Details, Subscription Details) for Astro and React. Adds unstyled Astro components, React counterparts, and exports from component indexes. Includes three Astro example pages under integration/templates and updates React utils to accept these buttons. Adds a changeset declaring a minor bump. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant B as CheckoutButton
participant S as Inline Script
participant C as window.Clerk
participant F as Checkout Flow
U->>B: Click
B->>S: data-clerk-unstyled-id handler
S->>C: __internal_openCheckout(options)
C-->>F: Launch checkout UI
Note over C,F: Validates signed-in state and (optional) organization context
sequenceDiagram
autonumber
participant U as User
participant B as PlanDetailsButton
participant S as Inline Script / React handler
participant C as window.Clerk
participant M as Plan Details Modal
U->>B: Click
B->>S: onClick
S->>C: __internal_openPlanDetails(options)
C-->>M: Open plan details
sequenceDiagram
autonumber
participant U as User
participant B as SubscriptionDetailsButton
participant S as Inline Script / React handler
participant C as window.Clerk
participant M as Subscription Details Modal
U->>B: Click
B->>S: onClick
S->>C: __internal_openSubscriptionDetails(options)
C-->>M: Open subscription details
Note over S: Validates signed-in and organization when for="organization"
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks (3 passed, 2 inconclusive)❌ Failed checks (2 inconclusive)
✅ Passed checks (3 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
✨ Finishing Touches🧪 Generate unit tests
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: 15
🧹 Nitpick comments (37)
integration/templates/astro-node/src/pages/billing/plan-details-btn.astro (3)
6-12: Wrap with SignedIn to enforce correct usageButton requires an authenticated user; the example should demonstrate best practice.
--- -import { __experimental_PlanDetailsButton as PlanDetailsButton } from '@clerk/astro/components'; +import { __experimental_PlanDetailsButton as PlanDetailsButton, SignedIn } from '@clerk/astro/components'; import Layout from '../../layouts/Layout.astro'; --- <Layout title="Plan Details Button"> <main> - <PlanDetailsButton planId='cplan_2wMjqdlza0hTJc4HLCoBwAiExhF'> - Plan details - </PlanDetailsButton> + <SignedIn> + <PlanDetailsButton planId="your_plan_id_here"> + Plan details + </PlanDetailsButton> + </SignedIn> </main> </Layout>
8-10: Avoid hard-coding real plan ids in templatesUse a placeholder to prevent confusion or accidental coupling to internal resources.
- <PlanDetailsButton planId='cplan_2wMjqdlza0hTJc4HLCoBwAiExhF'> + <PlanDetailsButton planId="your_plan_id_here">
1-12: Add integration tests for the new Astro billing pagesNo tests were added. Please add basic integration/E2E checks that:
- route renders,
- button exists and is focusable/clickable,
- ideally, click wiring stubs out Clerk open calls without real network.
I can draft Playwright tests under integration/** to cover these pages. Want me to open a follow-up PR with test scaffolding?
packages/astro/src/react/utils.tsx (1)
49-68: Add explicit return type to assertSingleChildThis is an exported utility in a package; add an explicit return type to satisfy TS/ESLint rules and improve DX.
export const assertSingleChild = (children: React.ReactNode) => ( name: | 'SignInButton' | 'SignUpButton' | 'SignOutButton' | 'SignInWithMetamaskButton' | 'SubscriptionDetailsButton' | 'CheckoutButton' - | 'PlanDetailsButton', - ) => { + | 'PlanDetailsButton', + ): React.ReactElement | string => { try { return React.Children.only(children); } catch { return `You've passed multiple children components to <${name}/>. You can only pass a single child component or text.`; } };packages/vue/src/utils/childrenUtils.ts (1)
15-24: Default rendered button should have type="button"Prevents unintended form submission when used inside forms, and matches the React util’s behavior.
export const normalizeWithDefaultValue = (slotContent: VNode[] | undefined, defaultValue: string) => { // Render a button with the default value if no slot content is provided if (!slotContent) { - return h('button', defaultValue); + return h('button', { type: 'button' }, defaultValue); } // Render a button with the slot content if it's a text node if (slotContent[0].type === Text) { - return h('button', slotContent); + return h('button', { type: 'button' }, slotContent); }packages/astro/src/astro-components/unstyled/SubscriptionDetailsButton.astro (2)
17-24: Set type="button" by default when rendering a nativeAvoids accidental form submissions if consumers don’t specify type. Compute tagProps once and spread.
const { as: Tag = 'button', asChild, for: _for, subscriptionDetailsProps, onSubscriptionCancel, ...props } = Astro.props; +const tagProps = { + ...props, + ...(Tag === 'button' && !('type' in props) ? { type: 'button' } : {}), +}; @@ ) : ( <Tag - {...props} + {...tagProps} data-clerk-unstyled-id={safeId} > <slot>Subscription details</slot> </Tag> )Also applies to: 44-49
1-52: Add tests for the Astro unstyled button behaviorPlease add a minimal browser test that:
- mounts the component in a page (with and without asChild),
- verifies data-clerk-unstyled-id is present,
- verifies click handler invokes a stubbed __internal_openSubscriptionDetails.
I can scaffold an Astro integration test harness with a mock window.Clerk and run assertions via Playwright. Want me to push a test bundle?
packages/vue/src/components/SubscriptionDetailsButton.vue (1)
1-50: Add component testsPlease add unit tests to assert:
- multiple default slot children throw the expected error,
- attrs (class, disabled) are forwarded to the rendered child,
- click calls __internal_openSubscriptionDetails with correct options.
I can add Vitest + Vue Test Utils tests exercising these cases. Want me to include the scaffolding?
packages/vue/tsup.config.ts (1)
20-27: Minor: the EsbuildPlugin alias is now unused; either remove it or reintroduce typing for the plugin list.Since the explicit casts were dropped, Line 7’s
type EsbuildPlugin = …is dead code. Two options:
- Keep it simple: remove the alias.
- Maintain type-safety: reintroduce casts or use
satisfieswith an intermediatepluginsarray.Apply this diff to remove the unused alias:
- type EsbuildPlugin = NonNullable<Options['esbuildPlugins']>[number];Alternatively, re-add precise typing:
esbuildPlugins: [ // Adds .vue files support - vuePlugin(), + vuePlugin() as NonNullable<Options['esbuildPlugins']>[number], // Automatically generates runtime props from TypeScript types/interfaces for all // control and UI components, adding them to Vue components during build via // Object.defineProperty autoPropsPlugin({ include: ['**/*.ts'], - }), + }) as NonNullable<Options['esbuildPlugins']>[number], ],integration/templates/vue-vite/src/router.ts (1)
50-65: Secure new billing routes and tighten route guard types
Confirm whether PlanDetailsBtn and SubscriptionDetailsBtn should be protected by authentication. If so, add their route names to the
authenticatedPagesarray:const authenticatedPages = [ 'Profile', 'Admin', 'CustomUserProfile', 'CustomOrganizationProfile', - // … + 'PlanDetailsBtn', + 'SubscriptionDetailsBtn', ];Harden the guard’s type-check to handle
to.namebeingstring | symbol | null:-} else if (!isSignedIn.value && authenticatedPages.includes(to.name)) { +} else if ( + !isSignedIn.value && + typeof to.name === 'string' && + authenticatedPages.includes(to.name) +) {(Optional) Add a lightweight E2E smoke test under
integration/to visit each billing route, assert the button renders, and verify auth enforcement.integration/templates/vue-vite/src/views/billing/PlanDetailsBtn.vue (1)
1-9: Avoid hard-coding plan IDs; make it configurable (with a sensible fallback).Hard-coded plan IDs easily go stale across environments. Read from a query param or env, keep a fallback for the template to work out of the box.
Apply this diff:
<template> <main> - <PlanDetailsButton planId="cplan_2wMjqdlza0hTJc4HLCoBwAiExhF"> Plan details </PlanDetailsButton> + <PlanDetailsButton :planId="planId"> Plan details </PlanDetailsButton> </main> </template> <script setup lang="ts"> import { PlanDetailsButton } from '@clerk/vue/experimental'; +import { useRoute } from 'vue-router'; + +const route = useRoute(); +// Prefer passing ?planId=... or set via env. Fallback keeps the template functional. +const planId = (route.query.planId as string) ?? 'cplan_2wMjqdlza0hTJc4HLCoBwAiExhF'; </script>If you prefer envs instead, swap the planId line for:
const planId = import.meta.env.VITE_CLERK_PLAN_ID ?? 'cplan_…';integration/templates/vue-vite/src/views/billing/CheckoutBtn.vue (3)
2-12: Optional: Provide a SignedOut fallback for better UXCurrently nothing renders for signed-out users; consider a SignedOut branch or a small notice/CTA to sign in.
1-17: Add basic integration/E2E coverage for the new routePer repository guidelines, please add/extend E2E tests under integration/ to assert the button is rendered for SignedIn users and triggers the expected Clerk action.
I can scaffold a minimal Playwright test that navigates to the route, signs in via helpers, and clicks the button to assert the expected side effect. Want me to draft it?
4-9: Centralize planId via env/config across all templatesThe
cplan_2wMjqdlza0hTJc4HLCoBwAiExhFvalue is hard-coded in multiple template files, which risks accidental leakage and complicates testing. Define it once via an env var (e.g.VITE_CLERK_EXAMPLE_PLAN_IDorNEXT_PUBLIC_CLERK_EXAMPLE_PLAN_ID) and bind it as a prop everywhere.Affected files:
- integration/templates/vue-vite/src/views/billing/PlanDetailsBtn.vue
- integration/templates/vue-vite/src/views/billing/CheckoutBtn.vue
- integration/templates/next-app-router/src/app/billing/plan-details-btn/page.tsx
- integration/templates/next-app-router/src/app/billing/checkout-btn/page.tsx
- integration/templates/astro-node/src/pages/billing/plan-details-btn.astro
- integration/templates/astro-node/src/pages/billing/checkout-btn.astro
Example refactor for Vue (CheckoutBtn.vue):
<template> <main> <SignedIn> <CheckoutButton - planId="cplan_2wMjqdlza0hTJc4HLCoBwAiExhF" + :planId="planId" planPeriod="month" > Checkout Now </CheckoutButton> </SignedIn> </main> </template> <script setup lang="ts"> import { SignedIn } from '@clerk/vue'; import { CheckoutButton } from '@clerk/vue/experimental'; +// Load from env with fallback +const planId = import.meta.env.VITE_CLERK_EXAMPLE_PLAN_ID ?? 'cplan_example_id'; </script>Apply analogous changes in:
- PlanDetailsBtn.vue: bind
:planId="planId"and defineplanIdin<script setup>.- Next.js pages: use
const planId = process.env.NEXT_PUBLIC_CLERK_EXAMPLE_PLAN_ID ?? 'cplan_example_id'and passplanId={planId}.- Astro pages: use frontmatter to import
planIdfromimport.meta.envand passplanId={planId}to the component.integration/templates/astro-node/src/pages/billing/checkout-btn.astro (2)
10-11: Parameterize planId instead of hard-codingMove planId to an environment variable so apps generated from this template don’t ship a real/opaque ID and can configure it easily.
- <CheckoutButton - planId='cplan_2wMjqdlza0hTJc4HLCoBwAiExhF' - planPeriod='month' - > + <CheckoutButton + planId={Astro?.props?.planId ?? import.meta.env.CLERK_EXAMPLE_PLAN_ID ?? 'cplan_example_id'} + planPeriod='month' + >If you prefer not to thread props through, reading only from import.meta.env is fine for the template.
8-15: Optional: Add a SignedOut branchFor parity with the Vue example and clearer behavior in templates, consider rendering a simple SignedOut message/CTA or redirect.
integration/templates/vue-vite/src/views/billing/SubscriptionDetailsBtn.vue (1)
2-4: Gate behind SignedIn for consistency and to avoid unauthenticated interactionsThe Checkout and (per PR summary) Plan Details examples are gated. Align this one as well.
<template> <main> - <SubscriptionDetailsButton> Subscription details </SubscriptionDetailsButton> + <SignedIn> + <SubscriptionDetailsButton>Subscription details</SubscriptionDetailsButton> + </SignedIn> </main> </template> <script setup lang="ts"> -import { SubscriptionDetailsButton } from '@clerk/vue/experimental'; +import { SignedIn } from '@clerk/vue'; +import { SubscriptionDetailsButton } from '@clerk/vue/experimental'; </script>Also applies to: 7-9
packages/astro/src/astro-components/index.ts (1)
15-17: Consider exporting the associated prop types and adding an experimental JSDoc bannerHelps consumers with TS discoverability and sets expectations about instability.
export { default as __experimental_SubscriptionDetailsButton } from './unstyled/SubscriptionDetailsButton.astro'; export { default as __experimental_CheckoutButton } from './unstyled/CheckoutButton.astro'; export { default as __experimental_PlanDetailsButton } from './unstyled/PlanDetailsButton.astro'; + +/** Experimental unstyled billing buttons — API is unstable and may change without notice. */ +export type { + __experimental_SubscriptionDetailsButtonProps, + __experimental_CheckoutButtonProps, + __experimental_PlanDetailsButtonProps, +} from '@clerk/types';packages/astro/src/astro-components/unstyled/PlanDetailsButton.astro (2)
36-39: Optional: Validate asChild usage yields an elementIf the default slot is empty or starts with text, addUnstyledAttributeToFirstTag won’t find a tag and the script will no-op. Consider warning when htmlElement is empty.
Example:
if (asChild) { htmlElement = await Astro.slots.render('default'); if (!htmlElement?.trim()) { console.warn('PlanDetailsButton: asChild requires a single root element in the default slot.'); } else { htmlElement = addUnstyledAttributeToFirstTag(htmlElement, safeId); } }
55-63: CSP note: Inline scripts may violate strict CSPIf you plan to support strict CSP without 'unsafe-inline', consider centralizing event binding via a delegated script with nonces or external JS.
packages/vue/src/components/PlanDetailsButton.vue (3)
23-29: Avoid unnecessary type assertion on the payloadThe explicit cast to __experimental_PlanDetailsButtonProps on the object literal is likely unnecessary and can mask shape mismatches. Let TS infer or type the __internal_openPlanDetails signature instead.
Apply:
- return clerk.value.__internal_openPlanDetails({ + return clerk.value.__internal_openPlanDetails({ plan: props.plan, planId: props.planId, initialPlanPeriod: props.initialPlanPeriod, ...props.planDetailsProps, - } as __experimental_PlanDetailsButtonProps); + });
18-29: Add lightweight error handling around internal callTo aid debugging, wrap the call to __internal_openPlanDetails in try/catch and surface a concise, actionable log. If a repo-level logger exists, prefer it over console.
Would you like me to patch this to use your logging utility (if any) instead of console.error?
1-30: Docs and tests for a new public component
- Add a brief JSDoc above defineProps describing the experimental API and the expected behavior (default label, auth requirement, shape of planDetailsProps).
- No tests appear in this PR. Please add at least a smoke test for:
- default rendering with text fallback
- click calling __internal_openPlanDetails with the expected payload when clerk is available
- no-op when clerk is absent
If you want, I can scaffold a minimal unit/integration test harness for Vue SFCs in this package.
packages/astro/src/react/index.ts (2)
12-16: Experimental re-exports look good; add JSDoc to clarify status and propsThe aliasing to _experimental* is clear. Add JSDoc noting that these exports are experimental and their props may change.
Would you like me to draft the JSDoc block for consistency with other public APIs?
1-26: Tests missing for new API surfaceNo tests were added. Consider minimal coverage to prevent regressions:
- Rendering of each experimental button from this index path
- Ensuring the props types are emitted in the .d.ts bundle (typegen snapshot)
packages/astro/src/react/CheckoutButton.tsx (3)
1-6: Fix import order to satisfy ESLint (simple-import-sort/imports)ESLint flagged import sorting. Apply the autofix or sort imports as below.
Apply:
-import type { __experimental_CheckoutButtonProps } from '@clerk/types'; -import React from 'react'; - -import { assertSingleChild, normalizeWithDefaultValue, safeExecute, withClerk } from './utils'; -import type { WithClerkProp } from './utils'; +import React from 'react'; +import type { __experimental_CheckoutButtonProps } from '@clerk/types'; +import { assertSingleChild, normalizeWithDefaultValue, safeExecute, withClerk } from './utils'; +import type { WithClerkProp } from './utils';Then run: pnpm eslint --fix
9-53: Add concise JSDoc for public component and propsThis is a new public API. Add JSDoc indicating:
- Experimental status
- What the button does (opens checkout flow)
- Required vs optional props and how child defaulting works (“Checkout” label, single child)
- That the click is a no-op until Clerk is loaded
I can draft the JSDoc inline if you want it ready-to-commit.
1-53: Tests recommended for the checkout flowAdd tests to cover:
- Default rendering with text child → normalized to
- Chaining of child onClick (verify child’s handler is invoked before checkout)
- Payload passed to __internal_openCheckout includes planId/planPeriod/for and spreads checkoutProps
- No-op when clerk is undefined
packages/astro/src/react/PlanDetailsButton.tsx (4)
1-6: Fix import order to satisfy ESLint (simple-import-sort/imports)Same sorting issue as CheckoutButton.tsx. Apply autofix or sort as below.
Apply:
-import type { __experimental_PlanDetailsButtonProps } from '@clerk/types'; -import React from 'react'; - -import { assertSingleChild, normalizeWithDefaultValue, safeExecute, withClerk } from './utils'; -import type { WithClerkProp } from './utils'; +import React from 'react'; +import type { __experimental_PlanDetailsButtonProps } from '@clerk/types'; +import { assertSingleChild, normalizeWithDefaultValue, safeExecute, withClerk } from './utils'; +import type { WithClerkProp } from './utils';Then run: pnpm eslint --fix
21-27: Remove unnecessary type assertion on the plan details payloadThe cast to __experimental_PlanDetailsButtonProps is redundant and could hide mistakes. Let inference work or type the clerk method signature.
Apply:
return clerk.__internal_openPlanDetails({ plan, planId, initialPlanPeriod, ...planDetailsProps, - } as __experimental_PlanDetailsButtonProps); + });
9-15: Add concise JSDoc for the public component and propsMirror CheckoutButton’s docs: experimental status, behavior (opens plan details modal/sheet), default label, single-child constraint, and no-op until Clerk is ready.
Happy to provide the docblock text.
1-41: Tests recommended for plan details flowCover:
- Default rendering (text → )
- Child onClick is invoked before __internal_openPlanDetails
- Payload fields plan/planId/initialPlanPeriod plus spread planDetailsProps
- No-op when clerk is undefined
packages/astro/src/react/SubscriptionDetailsButton.tsx (2)
1-6: Sort imports to satisfy ESLint (simple-import-sort/imports)The linter requests import reordering. Suggested ordering below usually satisfies the rule. Alternatively, run the repo’s lint autofix.
Apply this diff:
-import type { __experimental_SubscriptionDetailsButtonProps } from '@clerk/types'; -import React from 'react'; - -import { assertSingleChild, normalizeWithDefaultValue, safeExecute, withClerk } from './utils'; -import type { WithClerkProp } from './utils'; +import React from 'react'; +import type { __experimental_SubscriptionDetailsButtonProps } from '@clerk/types'; +import { assertSingleChild, normalizeWithDefaultValue, safeExecute, withClerk } from './utils'; +import type { WithClerkProp } from './utils';
7-15: Document the new public API (JSDoc) for discoverabilityPer repository guidelines, public exports should have JSDoc. Add brief JSDoc to SubscriptionDetailsButton and the props alias.
Example (add above the exports):
/**
- SubscriptionDetailsButton opens Clerk's subscription details for the active user or organization.
- Renders a single child (or a default Subscription details).
*/
export const SubscriptionDetailsButton .../** Props for SubscriptionDetailsButton. */
export type { __experimental_SubscriptionDetailsButtonProps as SubscriptionDetailsButtonProps };packages/vue/src/components/CheckoutButton.vue (2)
15-22: Auth/Org checks timing: consider deferring to click timeThrowing in setup will error during SSR/hydration if a consumer accidentally places this outside . Deferring these checks to clickHandler (like the Astro unstyled path does) avoids breaking render and provides the same developer guidance at interaction time.
Possible approach: move both checks into clickHandler, early-return with the same error messages.
1-53: Add tests for the new public componentNo tests are included in this PR. Add basic tests covering:
- renders default content and asserts single-child rule
- calls __internal_openCheckout with expected options
- enforces SignedIn and organization checks
I can scaffold Vitest component tests for this file if desired.
packages/astro/src/astro-components/unstyled/CheckoutButton.astro (1)
1-57: Add tests for the new Astro unstyled componentConsider adding integration tests verifying:
- asChild renders and wiring via data-clerk-unstyled-id
- default label, and type="button" behavior when Tag is "button"
- click invokes __internal_openCheckout with the right payload
Happy to scaffold Astro component tests using Playwright or a lightweight DOM harness.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (24)
integration/templates/astro-node/src/pages/billing/checkout-btn.astro(1 hunks)integration/templates/astro-node/src/pages/billing/plan-details-btn.astro(1 hunks)integration/templates/astro-node/src/pages/billing/subscription-details-btn.astro(1 hunks)integration/templates/vue-vite/src/router.ts(1 hunks)integration/templates/vue-vite/src/views/billing/CheckoutBtn.vue(1 hunks)integration/templates/vue-vite/src/views/billing/PlanDetailsBtn.vue(1 hunks)integration/templates/vue-vite/src/views/billing/SubscriptionDetailsBtn.vue(1 hunks)integration/tests/pricing-table.test.ts(0 hunks)packages/astro/src/astro-components/index.ts(1 hunks)packages/astro/src/astro-components/unstyled/CheckoutButton.astro(1 hunks)packages/astro/src/astro-components/unstyled/PlanDetailsButton.astro(1 hunks)packages/astro/src/astro-components/unstyled/SubscriptionDetailsButton.astro(1 hunks)packages/astro/src/react/CheckoutButton.tsx(1 hunks)packages/astro/src/react/PlanDetailsButton.tsx(1 hunks)packages/astro/src/react/SubscriptionDetailsButton.tsx(1 hunks)packages/astro/src/react/index.ts(1 hunks)packages/astro/src/react/utils.tsx(1 hunks)packages/vue/package.json(1 hunks)packages/vue/src/components/CheckoutButton.vue(1 hunks)packages/vue/src/components/PlanDetailsButton.vue(1 hunks)packages/vue/src/components/SubscriptionDetailsButton.vue(1 hunks)packages/vue/src/experimental.ts(1 hunks)packages/vue/src/utils/childrenUtils.ts(1 hunks)packages/vue/tsup.config.ts(1 hunks)
💤 Files with no reviewable changes (1)
- integration/tests/pricing-table.test.ts
🧰 Additional context used
📓 Path-based instructions (15)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/astro/src/astro-components/index.tsintegration/templates/vue-vite/src/router.tspackages/astro/src/react/utils.tsxpackages/vue/src/utils/childrenUtils.tspackages/astro/src/react/PlanDetailsButton.tsxpackages/astro/src/react/CheckoutButton.tsxpackages/vue/src/experimental.tspackages/vue/tsup.config.tspackages/astro/src/react/SubscriptionDetailsButton.tsxpackages/astro/src/react/index.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/astro/src/astro-components/index.tsintegration/templates/vue-vite/src/router.tspackages/astro/src/react/utils.tsxpackages/vue/src/utils/childrenUtils.tspackages/astro/src/react/PlanDetailsButton.tsxpackages/astro/src/react/CheckoutButton.tsxpackages/vue/src/experimental.tspackages/vue/tsup.config.tspackages/astro/src/react/SubscriptionDetailsButton.tsxpackages/astro/src/react/index.tspackages/vue/package.json
packages/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/astro/src/astro-components/index.tspackages/astro/src/react/utils.tsxpackages/vue/src/utils/childrenUtils.tspackages/astro/src/react/PlanDetailsButton.tsxpackages/astro/src/react/CheckoutButton.tsxpackages/vue/src/experimental.tspackages/vue/tsup.config.tspackages/astro/src/react/SubscriptionDetailsButton.tsxpackages/astro/src/react/index.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/astro/src/astro-components/index.tspackages/astro/src/react/utils.tsxpackages/vue/src/utils/childrenUtils.tspackages/astro/src/react/PlanDetailsButton.tsxpackages/astro/src/react/CheckoutButton.tsxpackages/vue/src/experimental.tspackages/vue/tsup.config.tspackages/astro/src/react/SubscriptionDetailsButton.tsxpackages/astro/src/react/index.ts
packages/**/index.{js,ts}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use tree-shaking friendly exports
Files:
packages/astro/src/astro-components/index.tspackages/astro/src/react/index.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/astro/src/astro-components/index.tsintegration/templates/vue-vite/src/router.tspackages/astro/src/react/utils.tsxpackages/vue/src/utils/childrenUtils.tspackages/astro/src/react/PlanDetailsButton.tsxpackages/astro/src/react/CheckoutButton.tsxpackages/vue/src/experimental.tspackages/vue/tsup.config.tspackages/astro/src/react/SubscriptionDetailsButton.tsxpackages/astro/src/react/index.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/astro/src/astro-components/index.tsintegration/templates/vue-vite/src/router.tspackages/astro/src/react/utils.tsxpackages/vue/src/utils/childrenUtils.tspackages/astro/src/react/PlanDetailsButton.tsxpackages/astro/src/react/CheckoutButton.tsxpackages/vue/src/experimental.tspackages/vue/tsup.config.tspackages/astro/src/react/SubscriptionDetailsButton.tsxpackages/astro/src/react/index.ts
**/index.ts
📄 CodeRabbit Inference Engine (.cursor/rules/react.mdc)
Use index.ts files for clean imports but avoid deep barrel exports
Avoid barrel files (index.ts re-exports) as they can cause circular dependencies
Files:
packages/astro/src/astro-components/index.tspackages/astro/src/react/index.ts
**/*
⚙️ CodeRabbit Configuration File
If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes.
Files:
packages/astro/src/astro-components/index.tsintegration/templates/vue-vite/src/router.tsintegration/templates/astro-node/src/pages/billing/checkout-btn.astrointegration/templates/astro-node/src/pages/billing/subscription-details-btn.astropackages/astro/src/react/utils.tsxpackages/vue/src/utils/childrenUtils.tsintegration/templates/vue-vite/src/views/billing/PlanDetailsBtn.vuepackages/astro/src/react/PlanDetailsButton.tsxpackages/astro/src/react/CheckoutButton.tsxintegration/templates/astro-node/src/pages/billing/plan-details-btn.astropackages/vue/src/experimental.tspackages/vue/src/components/SubscriptionDetailsButton.vueintegration/templates/vue-vite/src/views/billing/SubscriptionDetailsBtn.vuepackages/vue/tsup.config.tspackages/vue/src/components/PlanDetailsButton.vuepackages/astro/src/react/SubscriptionDetailsButton.tsxpackages/astro/src/react/index.tsintegration/templates/vue-vite/src/views/billing/CheckoutBtn.vuepackages/astro/src/astro-components/unstyled/PlanDetailsButton.astropackages/astro/src/astro-components/unstyled/SubscriptionDetailsButton.astropackages/astro/src/astro-components/unstyled/CheckoutButton.astropackages/vue/src/components/CheckoutButton.vuepackages/vue/package.json
integration/**
📄 CodeRabbit Inference Engine (.cursor/rules/global.mdc)
Framework integration templates and E2E tests should be placed under the integration/ directory
Files:
integration/templates/vue-vite/src/router.tsintegration/templates/astro-node/src/pages/billing/checkout-btn.astrointegration/templates/astro-node/src/pages/billing/subscription-details-btn.astrointegration/templates/vue-vite/src/views/billing/PlanDetailsBtn.vueintegration/templates/astro-node/src/pages/billing/plan-details-btn.astrointegration/templates/vue-vite/src/views/billing/SubscriptionDetailsBtn.vueintegration/templates/vue-vite/src/views/billing/CheckoutBtn.vue
integration/**/*
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
End-to-end tests and integration templates must be located in the 'integration/' directory.
Files:
integration/templates/vue-vite/src/router.tsintegration/templates/astro-node/src/pages/billing/checkout-btn.astrointegration/templates/astro-node/src/pages/billing/subscription-details-btn.astrointegration/templates/vue-vite/src/views/billing/PlanDetailsBtn.vueintegration/templates/astro-node/src/pages/billing/plan-details-btn.astrointegration/templates/vue-vite/src/views/billing/SubscriptionDetailsBtn.vueintegration/templates/vue-vite/src/views/billing/CheckoutBtn.vue
**/*.{jsx,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
**/*.{jsx,tsx}: Use error boundaries in React components
Minimize re-renders in React components
**/*.{jsx,tsx}: Always use functional components with hooks instead of class components
Follow PascalCase naming for components:UserProfile,NavigationMenu
Keep components focused on a single responsibility - split large components
Limit component size to 150-200 lines; extract logic into custom hooks
Use composition over inheritance - prefer smaller, composable components
Export components as named exports for better tree-shaking
One component per file with matching filename and component name
Use useState for simple state management
Use useReducer for complex state logic
Implement proper state initialization
Use proper state updates with callbacks
Implement proper state cleanup
Use Context API for theme/authentication
Implement proper state selectors
Use proper state normalization
Implement proper state persistence
Use React.memo for expensive components
Implement proper useCallback for handlers
Use proper useMemo for expensive computations
Implement proper virtualization for lists
Use proper code splitting with React.lazy
Implement proper cleanup in useEffect
Use proper refs for DOM access
Implement proper event listener cleanup
Use proper abort controllers for fetch
Implement proper subscription cleanup
Use proper HTML elements
Implement proper ARIA attributes
Use proper heading hierarchy
Implement proper form labels
Use proper button types
Implement proper focus management
Use proper keyboard shortcuts
Implement proper tab order
Use proper skip links
Implement proper focus traps
Implement proper error boundaries
Use proper error logging
Implement proper error recovery
Use proper error messages
Implement proper error fallbacks
Use proper form validation
Implement proper error states
Use proper error messages
Implement proper form submission
Use proper form reset
Use proper component naming
Implement proper file naming
Use proper prop naming
Implement proper...
Files:
packages/astro/src/react/utils.tsxpackages/astro/src/react/PlanDetailsButton.tsxpackages/astro/src/react/CheckoutButton.tsxpackages/astro/src/react/SubscriptionDetailsButton.tsx
**/*.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/react.mdc)
**/*.tsx: Use proper type definitions for props and state
Leverage TypeScript's type inference where possible
Use proper event types for handlers
Implement proper generic types for reusable components
Use proper type guards for conditional rendering
Files:
packages/astro/src/react/utils.tsxpackages/astro/src/react/PlanDetailsButton.tsxpackages/astro/src/react/CheckoutButton.tsxpackages/astro/src/react/SubscriptionDetailsButton.tsx
packages/*/tsup.config.{js,ts}
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
TypeScript compilation and bundling must use tsup.
Files:
packages/vue/tsup.config.ts
packages/*/package.json
📄 CodeRabbit Inference Engine (.cursor/rules/global.mdc)
All publishable packages should be placed under the packages/ directory
packages/*/package.json: All publishable packages must be located in the 'packages/' directory.
All packages must be published under the @clerk namespace on npm.
Semantic versioning must be used across all packages.
Files:
packages/vue/package.json
🧬 Code Graph Analysis (3)
packages/astro/src/react/PlanDetailsButton.tsx (2)
packages/astro/src/react/index.ts (1)
PlanDetailsButton(15-15)packages/astro/src/react/utils.tsx (5)
withClerk(16-43)WithClerkProp(45-47)normalizeWithDefaultValue(70-78)assertSingleChild(50-67)safeExecute(81-87)
packages/astro/src/react/CheckoutButton.tsx (2)
packages/astro/src/react/index.ts (1)
CheckoutButton(14-14)packages/astro/src/react/utils.tsx (5)
withClerk(16-43)WithClerkProp(45-47)normalizeWithDefaultValue(70-78)assertSingleChild(50-67)safeExecute(81-87)
packages/astro/src/react/SubscriptionDetailsButton.tsx (1)
packages/astro/src/react/utils.tsx (5)
withClerk(16-43)WithClerkProp(45-47)normalizeWithDefaultValue(70-78)assertSingleChild(50-67)safeExecute(81-87)
🪛 ESLint
packages/astro/src/react/PlanDetailsButton.tsx
[error] 1-5: Run autofix to sort these imports!
(simple-import-sort/imports)
packages/astro/src/react/CheckoutButton.tsx
[error] 1-5: Run autofix to sort these imports!
(simple-import-sort/imports)
packages/astro/src/react/SubscriptionDetailsButton.tsx
[error] 1-5: Run autofix to sort these imports!
(simple-import-sort/imports)
🔇 Additional comments (10)
packages/astro/src/react/utils.tsx (1)
52-61: Union extension for new button names looks goodThe added names align with the new experimental billing buttons.
packages/vue/src/utils/childrenUtils.ts (1)
6-13: Union extension for new button names looks goodMatches the new Vue billing buttons.
packages/vue/package.json (1)
33-36: Export map and sources verified – please confirm d.ts emission in build pipeline
- packages/vue/src/experimental.ts exists
- All referenced components under packages/vue/src/components/ are present
- Export map entry in packages/vue/package.json (lines 33–36) correctly points to dist/experimental.d.ts & dist/experimental.js
Please ensure that the CI build (vue-tsc) emits dist/experimental.d.ts as expected.
integration/templates/astro-node/src/pages/billing/subscription-details-btn.astro (1)
1-12: LGTM — minimal, idiomatic Astro usage of the experimental button.This aligns with the new experimental Astro components API and matches the template’s layout pattern. Consider adding an integration test that navigates to /billing/subscription-details-btn and asserts the button renders.
packages/astro/src/astro-components/index.ts (1)
15-17: Looks good: experimental unstyled buttons are exported in a tree-shake-friendly wayThe new exports follow existing conventions.
packages/vue/src/components/PlanDetailsButton.vue (1)
13-16: Ignore child VNode prop-preservation concern: utilities already return VNodes
BothnormalizeWithDefaultValueandassertSingleChildproduce actual VNode instances (viahor by returning the slot’s VNode), not raw component types or strings. When you pass a VNode to<component :is="…">, Vue will mount that VNode directly—without re-creating it—so any props, classes or listeners on the original VNode stay intact. No need to switch tocloneVNodehere.Likely an incorrect or invalid review comment.
packages/astro/src/react/index.ts (1)
18-25: Astro export map covers new React entry
- The root
packages/astro/package.jsonexports object includes"./react"pointing todist/react/index.jsanddist/react/index.d.ts, so all new component and type re-exports insrc/react/index.ts(including the__experimental_*Propsaliases) will be packaged.- The
__experimental_SubscriptionDetailsButtonProps,__experimental_CheckoutButtonProps, and__experimental_PlanDetailsButtonPropsaliases are tree-shake friendly and available under@clerk/astro/react.- Please ensure your public documentation or changelog is updated to mention these experimental types.
packages/astro/src/react/CheckoutButton.tsx (1)
24-26: Child click handler is correctly chained and element is clonedParity with the React approach is solid here: default label, single-child assertion, safe chaining of the child’s onClick, and no-op when Clerk is not ready.
Also applies to: 42-47, 49-51
packages/astro/src/react/PlanDetailsButton.tsx (1)
13-15: LGTM on child handling parity with ReactYou normalize to “Plan details”, assert a single child, chain onClick, and clone the element—matching the established pattern.
Also applies to: 29-38
packages/vue/src/components/CheckoutButton.vue (1)
24-27: Default to a non-submitting button when using default contentIf normalizeWithDefaultValue renders a , ensure it sets type="button" to avoid unintended form submissions. Align with the React util that sets type="button".
Please confirm the Vue normalizeWithDefaultValue implementation sets type="button" for string children. If not, I can open a follow-up PR to add it.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
integration/tests/pricing-table.test.ts (3)
88-99: Make the skip intentional (fixme) and fix test title grammar
- test.fixme conveys “temporarily disabled; to be fixed,” better than skip.
- Minor grammar nit in the test title.
- test('when signed in, clicking checkout button open checkout drawer', async ({ page, context }) => { - test.skip(app.name.includes('astro'), 'Still working on it'); + test('when signed in, clicking checkout button opens checkout drawer', async ({ page, context }) => { + test.fixme( + app.name.includes('astro'), + 'Astro: CheckoutButton E2E pending wiring of experimental billing buttons (BILL-1181)' + );
123-138: Use test.fixme with a concrete reason instead of a generic skipKeep the rationale actionable and tied to the initiative, so it’s easier to re-enable once the Astro work lands.
- test.skip(app.name.includes('astro'), 'Still working on it'); + test.fixme( + app.name.includes('astro'), + 'Astro: SubscriptionDetailsButton E2E pending wiring of experimental billing buttons (BILL-1181)' + );
31-37: Consolidate vaguetest.skipcalls intotest.fixmewith actionable reasonsNo early-return guards were found. However, there are three occurrences of
test.skip(app.name.includes('astro'), 'Still working on it')in
integration/tests/pricing-table.test.ts that should be replaced with
test.fixmeand tied to their respective tracking tickets for clarity and future cleanup:• Lines 30–34
• Lines 87–91
• Lines 122–126Suggested diff for each block (example for the first occurrence):
- test.skip(app.name.includes('astro'), 'Still working on it'); + test.fixme( + app.name.includes('astro'), + 'Astro: PricingTable “renders pricing details” E2E awaiting billing buttons wiring (BILL-1181)' + );Please apply a similarly structured
test.fixmewith a clear, ticket-linked reason to the other two skips.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.changeset/cold-parks-push.md(1 hunks)integration/tests/pricing-table.test.ts(3 hunks)packages/vue/src/experimental.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .changeset/cold-parks-push.md
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/vue/src/experimental.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
integration/tests/pricing-table.test.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
integration/tests/pricing-table.test.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
integration/tests/pricing-table.test.ts
integration/**
📄 CodeRabbit Inference Engine (.cursor/rules/global.mdc)
Framework integration templates and E2E tests should be placed under the integration/ directory
Files:
integration/tests/pricing-table.test.ts
integration/**/*
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
End-to-end tests and integration templates must be located in the 'integration/' directory.
Files:
integration/tests/pricing-table.test.ts
integration/**/*.{test,spec}.{js,ts}
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
Integration tests should use Playwright.
Files:
integration/tests/pricing-table.test.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
integration/tests/pricing-table.test.ts
**/*
⚙️ CodeRabbit Configuration File
If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes.
Files:
integration/tests/pricing-table.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/astro/src/astro-components/unstyled/SubscriptionDetailsButton.astro (1)
55-74: Guard against null element and missing Clerk; prevent default on clickAdd null checks for the queried element and window.Clerk, and prevent default to avoid unintended form submits or navigation. This mirrors patterns used in similar components.
-<script is:inline define:vars={{ props, subscriptionDetailsOptions, safeId }}> - const btn = document.querySelector(`[data-clerk-unstyled-id="${safeId}"]`); - - btn.addEventListener('click', () => { - const clerk = window.Clerk; +<script is:inline define:vars={{ subscriptionDetailsOptions, safeId }}> + const btn = document.querySelector(`[data-clerk-unstyled-id="${safeId}"]`); + if (!btn) { + console.warn('SubscriptionDetailsButton: target element not found', { safeId }); + } else { + btn.addEventListener('click', (evt) => { + evt.preventDefault(); + const clerk = window.Clerk; + if (!clerk) { + return; + } // Authentication checks - if (!clerk.user) { + if (!clerk.user) { throw new Error('Ensure that `<SubscriptionDetailsButton />` is rendered inside a `<SignedIn />` component.'); } if (!clerk.organization && subscriptionDetailsOptions.for === 'organization') { throw new Error( 'Wrap `<SubscriptionDetailsButton for="organization" />` with a check for an active organization.', ); } - return clerk.__internal_openSubscriptionDetails(subscriptionDetailsOptions); - }); + return clerk.__internal_openSubscriptionDetails(subscriptionDetailsOptions); + }); + } </script>
🧹 Nitpick comments (3)
packages/astro/src/astro-components/unstyled/SubscriptionDetailsButton.astro (3)
36-39: Handle empty asChild slot gracefullyIf the default slot renders nothing, addUnstyledAttributeToFirstTag gets an empty string and the client querySelector returns null. Warn and skip mutation when empty.
-if (asChild) { - htmlElement = await Astro.slots.render('default'); - htmlElement = addUnstyledAttributeToFirstTag(htmlElement, safeId); -} +if (asChild) { + htmlElement = await Astro.slots.render('default'); + if (!htmlElement?.trim()) { + console.warn('SubscriptionDetailsButton: asChild slot is empty; button will not be interactive.', { safeId }); + } else { + htmlElement = addUnstyledAttributeToFirstTag(htmlElement, safeId); + } +}
46-51: Default type="button" to avoid accidental form submissionsWhen Tag is a native button and no type is provided, browsers default to type="submit". Set a safe default.
<Tag {...props} data-clerk-unstyled-id={safeId} + type={Tag === 'button' ? ((props as any)?.type ?? 'button') : undefined} >
1-75: Add tests for new experimental Astro billing buttonNo tests were added in this PR. Please add coverage for:
- Renders as native button with type="button" and asChild.
- Click invokes __internal_openSubscriptionDetails with correct options for user and organization.
- Guards: no-op when element missing or Clerk not available; throws helpful errors when not signed in or organization missing.
I can scaffold minimal Astro integration tests and a basic DOM click test using Playwright or Vitest + @astrojs/renderer if you prefer.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
packages/astro/src/astro-components/unstyled/PlanDetailsButton.astro(1 hunks)packages/astro/src/astro-components/unstyled/SubscriptionDetailsButton.astro(1 hunks)packages/astro/src/react/CheckoutButton.tsx(1 hunks)packages/astro/src/react/PlanDetailsButton.tsx(1 hunks)packages/astro/src/react/SubscriptionDetailsButton.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/astro/src/astro-components/unstyled/PlanDetailsButton.astro
- packages/astro/src/react/PlanDetailsButton.tsx
- packages/astro/src/react/SubscriptionDetailsButton.tsx
- packages/astro/src/react/CheckoutButton.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*
⚙️ CodeRabbit Configuration File
If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes.
Files:
packages/astro/src/astro-components/unstyled/SubscriptionDetailsButton.astro
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (26)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (elements, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (tanstack-react-router, chrome)
- GitHub Check: Integration Tests (expo-web, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Integration Tests (quickstart, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Publish with pkg-pr-new
- GitHub Check: Unit Tests (18, --filter=@clerk/astro --filter=@clerk/backend --filter=@clerk/express --filter=@c...
- GitHub Check: Static analysis
- GitHub Check: Unit Tests (22, **)
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
| const subscriptionDetailsOptions = { | ||
| for: _for, | ||
| onSubscriptionCancel, | ||
| ...subscriptionDetailsProps, | ||
| }; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Make top-level props take precedence over nested subscriptionDetailsProps
Currently subscriptionDetailsProps can override for/onSubscriptionCancel, which is surprising. Prefer explicit top-level props to win.
Apply:
-const subscriptionDetailsOptions = {
- for: _for,
- onSubscriptionCancel,
- ...subscriptionDetailsProps,
-};
+const subscriptionDetailsOptions = {
+ ...subscriptionDetailsProps,
+ for: _for,
+ onSubscriptionCancel,
+};Verification (consistency with sibling Astro buttons):
🏁 Script executed:
#!/bin/bash
# Inspect options precedence across related unstyled Astro buttons
rg -n -C2 --type=astro 'checkoutOptions|planDetailsOptions|subscriptionDetailsOptions' packages/astro/src/astro-components/unstyledLength of output: 163
🏁 Script executed:
#!/bin/bash
# Inspect options precedence across related unstyled Astro buttons
rg -n -C2 'checkoutOptions' packages/astro/src/astro-components/unstyled -g '*.astro'
rg -n -C2 'planDetailsOptions' packages/astro/src/astro-components/unstyled -g '*.astro'
rg -n -C2 'subscriptionDetailsOptions' packages/astro/src/astro-components/unstyled -g '*.astro'Length of output: 5783
🏁 Script executed:
#!/bin/bash
sed -n '1,60p' packages/astro/src/astro-components/unstyled/CheckoutButton.astro
echo '---'
sed -n '1,60p' packages/astro/src/astro-components/unstyled/PlanDetailsButton.astroLength of output: 2932
Align prop precedence across all unstyled Astro buttons
To ensure top-level props always override nested ones, we need to reverse the spread order in all three buttons—currently each one spreads nested props after the explicit fields, allowing nested props to win. Update as follows:
• packages/astro/src/astro-components/unstyled/CheckoutButton.astro
Replace:
const checkoutOptions = {
planId,
planPeriod,
for: _for,
onSubscriptionComplete,
newSubscriptionRedirectUrl,
- ...checkoutProps,
};
+ ...checkoutProps,
+ planId,
+ planPeriod,
+ for: _for,
+ onSubscriptionComplete,
+ newSubscriptionRedirectUrl,
};• packages/astro/src/astro-components/unstyled/PlanDetailsButton.astro
Replace:
const planDetailsOptions = {
plan,
planId,
initialPlanPeriod,
- ...planDetailsProps,
};
+ ...planDetailsProps,
+ plan,
+ planId,
+ initialPlanPeriod,
};• packages/astro/src/astro-components/unstyled/SubscriptionDetailsButton.astro
Replace:
const subscriptionDetailsOptions = {
for: _for,
onSubscriptionCancel,
- ...subscriptionDetailsProps,
};
+ ...subscriptionDetailsProps,
+ for: _for,
+ onSubscriptionCancel,
};This guarantees explicit for, planId, etc., always take precedence over any overrides passed in via *Props.
📝 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 subscriptionDetailsOptions = { | |
| for: _for, | |
| onSubscriptionCancel, | |
| ...subscriptionDetailsProps, | |
| }; | |
| const subscriptionDetailsOptions = { | |
| ...subscriptionDetailsProps, | |
| for: _for, | |
| onSubscriptionCancel, | |
| }; |
🤖 Prompt for AI Agents
In packages/astro/src/astro-components/unstyled/SubscriptionDetailsButton.astro
around lines 28-32 (and also apply the same change to
packages/astro/src/astro-components/unstyled/CheckoutButton.astro and
packages/astro/src/astro-components/unstyled/PlanDetailsButton.astro), the
object spread currently places the nested props last so they win; reverse the
spread order so nested props are spread first and then explicit fields follow
(i.e., move ...subscriptionDetailsProps before the explicit for and
onSubscriptionCancel entries) so top-level explicit props take precedence over
the *Props overrides.
| if (userId.value === null) { | ||
| throw new Error('Ensure that `<CheckoutButton />` is rendered inside a `<SignedIn />` component.'); | ||
| } | ||
|
|
||
| if (orgId.value === null && props.for === 'organization') { | ||
| throw new Error('Wrap `<CheckoutButton for="organization" />` with a check for an active organization.'); | ||
| } |
There was a problem hiding this comment.
Should we also throw this error when value is still undefined (Clerk is loading)?
| if (userId.value === null) { | ||
| throw new Error('Ensure that `<SubscriptionDetailsButton />` is rendered inside a `<SignedIn />` component.'); | ||
| } | ||
|
|
||
| if (orgId.value === null && props.for === 'organization') { | ||
| throw new Error('Wrap `<SubscriptionDetailsButton for="organization" />` with a check for an active organization.'); | ||
| } |
There was a problem hiding this comment.
| export { default as __experimental_SubscriptionDetailsButton } from './unstyled/SubscriptionDetailsButton.astro'; | ||
| export { default as __experimental_CheckoutButton } from './unstyled/CheckoutButton.astro'; | ||
| export { default as __experimental_PlanDetailsButton } from './unstyled/PlanDetailsButton.astro'; |
There was a problem hiding this comment.
Is there any reason why we're not exporting them in a /experimental path?
…for-astro-and-vue # Conflicts: # integration/tests/pricing-table.test.ts
…for-astro-and-vue # Conflicts: # packages/vue/src/components/CheckoutButton.vue # packages/vue/src/components/SubscriptionDetailsButton.vue
This reverts commit c9e754e.
|
!snapshot |
|
Hey @panteliselef - the snapshot version command generated the following package versions:
Tip: Use the snippet copy button below to quickly install the required packages. npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact |
panteliselef
changed the title
| ); | ||
| } | ||
|
|
||
| return clerk.__internal_openSubscriptionDetails(subscriptionDetailsOptions); |
There was a problem hiding this comment.
Since we can't pass a function as prop here, the onSubscriptionCancel prop is unusable, but we can use a custom event like this
const customEvent = new CustomEvent('clerk:subscription-cancel');
clerk.__internal_openSubscriptionDetails({
...subscriptionDetailsOptions,
onSubscriptionCancel: () => document.dispatchEvent(customEvent),
});and in the parent component, they can listen to it
<script>
document.addEventListener('clerk:subscription-cancel', (e) => {
//...
})
</script>We do this pattern in our <UserButton> custom menu items in Astro
| btn.addEventListener('click', () => { | ||
| const clerk = window.Clerk; | ||
|
|
||
| return clerk.__internal_openPlanDetails(planDetailsOptions); |
There was a problem hiding this comment.
ditto with the onSubscriptionComplete event https://github.com/clerk/javascript/pull/6583/files#r2333981663
Description
Fixes BILL-1181
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Documentation
Chores