chore(clerk-js): Address leftover commerce TODOs #6183
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🦋 Changeset detectedLatest commit: 10bb031 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 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 |
| @@ -101,7 +100,6 @@ export const AddEmailForm = () => { | |||
| padding: t.space.$4, | |||
| })} | |||
| > | |||
| {/* TODO(@COMMERCE): How does ths operate for orgs ? */} | |||
There was a problem hiding this comment.
This flow works for orgs by grabbing the current member's email (once added) and associating that with the payer record
| @@ -250,7 +250,7 @@ const AddPaymentSourceForm = ({ children }: PropsWithChildren) => { | |||
| const { setupIntent, error } = await stripe.confirmSetup({ | |||
| elements, | |||
| confirmParams: { | |||
| return_url: '', // TODO(@COMMERCE): need to figure this out | |||
| return_url: '', | |||
There was a problem hiding this comment.
We prevent payment methods that require a redirect, so this param is never used (though required)
There was a problem hiding this comment.
Are we plan to support them in the future ? Are we planning to communicate that they will need to bump sdk versions in order to use it at that point ?
Maybe at least we so something like, unless '' has the same effect.
| return_url: '', | |
| return_url: '', |
| return_url: '', | |
| return_url: window.location.href, |
📝 WalkthroughWalkthroughThis change updates the Suggested labels
Suggested reviewers
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.changeset/silent-zebras-refuse.md (1)
5-5: Consider using "to-dos" for consistency with style guides.While "TODOs" is commonly used in technical contexts, "to-dos" with hyphens is the grammatically preferred form according to style guides.
-Cleanup leftover Commerce-launch TODOs +Cleanup leftover Commerce-launch to-dos
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.changeset/silent-zebras-refuse.md(1 hunks)packages/clerk-js/bundlewatch.config.json(1 hunks)packages/clerk-js/src/ui/components/Checkout/parts.tsx(0 hunks)packages/clerk-js/src/ui/components/OrganizationProfile/OrganizationProfileRoutes.tsx(1 hunks)packages/clerk-js/src/ui/components/PaymentSources/AddPaymentSource.tsx(2 hunks)packages/clerk-js/src/ui/components/UserProfile/UserProfileRoutes.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- packages/clerk-js/src/ui/components/Checkout/parts.tsx
🧰 Additional context used
📓 Path-based instructions (4)
`**/*.{js,ts,tsx,jsx}`: All code must pass ESLint checks with the project's configuration. Use Prettier for consistent code formatting.
**/*.{js,ts,tsx,jsx}: All code must pass ESLint checks with the project's configuration.
Use Prettier for consistent code formatting.
packages/clerk-js/src/ui/components/PaymentSources/AddPaymentSource.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/OrganizationProfileRoutes.tsxpackages/clerk-js/src/ui/components/UserProfile/UserProfileRoutes.tsx
`**/*.{ts,tsx}`: Maintain comprehensive JSDoc comments for public APIs.
**/*.{ts,tsx}: Maintain comprehensive JSDoc comments for public APIs.
packages/clerk-js/src/ui/components/PaymentSources/AddPaymentSource.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/OrganizationProfileRoutes.tsxpackages/clerk-js/src/ui/components/UserProfile/UserProfileRoutes.tsx
`packages/**`: All publishable packages under the @clerk namespace must be located in the packages/ directory.
packages/**: All publishable packages under the @clerk namespace must be located in the packages/ directory.
packages/clerk-js/src/ui/components/PaymentSources/AddPaymentSource.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/OrganizationProfileRoutes.tsxpackages/clerk-js/bundlewatch.config.jsonpackages/clerk-js/src/ui/components/UserProfile/UserProfileRoutes.tsx
`**/*.{jsx,tsx}`: Always use functional components with hooks instead of class components. Follow PascalCase naming for components; the filename and component name should match. Li...
**/*.{jsx,tsx}: Always use functional components with hooks instead of class components.
Follow PascalCase naming for components; the filename and component name should match.
Limit component size to 150-200 lines; extract logic into custom hooks.
Export components as named exports for better tree-shaking.
One component per file with matching filename and component name.
Co-locate related files (component, test, stories) in the same directory.
packages/clerk-js/src/ui/components/PaymentSources/AddPaymentSource.tsxpackages/clerk-js/src/ui/components/OrganizationProfile/OrganizationProfileRoutes.tsxpackages/clerk-js/src/ui/components/UserProfile/UserProfileRoutes.tsx
🧠 Learnings (1)
.changeset/silent-zebras-refuse.md (1)
Learnt from: dstaley
PR: clerk/javascript#6116
File: .changeset/tangy-garlics-say.md:1-2
Timestamp: 2025-06-13T16:09:53.061Z
Learning: In the Clerk JavaScript repository, contributors create intentionally empty changeset files (containing only the YAML delimiters) when a PR touches only non-published parts of the codebase (e.g., sandbox assets). This signals that no package release is required, so such changesets should not be flagged as missing content.
🪛 LanguageTool
.changeset/silent-zebras-refuse.md
[grammar] ~5-~5: It appears that a hyphen is missing in the plural noun “to-dos”?
Context: ...h --- Cleanup leftover Commerce-launch TODOs
(TO_DO_HYPHEN)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Build Packages
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
packages/clerk-js/src/ui/components/PaymentSources/AddPaymentSource.tsx (2)
252-254: Verify that empty return_url is acceptable for Stripe setup.The TODO comment removal left an empty string for
return_url. While Stripe allows this forredirect: 'if_required', please ensure this meets your integration requirements.
296-298: Apple Pay managementURL implementation looks appropriate.Using
displayConfig.homeUrlfor the Apple Pay recurring payment management URL is a reasonable default implementation after removing the TODO comment.packages/clerk-js/bundlewatch.config.json (1)
12-12: Bundle size adjustments align well with lazy loading implementation.The reduced limits for main profile bundles and new entries for lazy-loaded pages reflect the code splitting changes appropriately. This should improve initial bundle loading performance.
Also applies to: 18-18, 26-31
packages/clerk-js/src/ui/components/OrganizationProfile/OrganizationProfileRoutes.tsx (1)
22-38: Well-implemented lazy loading with proper code splitting.The conversion to lazy-loaded components follows React best practices with:
- Correct use of
React.lazy()- Webpack chunk names for predictable bundle naming
- Existing
Suspensewrappers maintained- Clean import structure
This should improve initial page load performance by deferring these components until needed.
packages/clerk-js/src/ui/components/UserProfile/UserProfileRoutes.tsx (1)
22-38: Consistent lazy loading implementation with shared components.The lazy loading implementation matches the organization profile pattern and correctly handles shared components (
StatementPageandPaymentAttemptPage) imported from separate directories. This promotes code reuse while maintaining performance benefits.
@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: |
| { "path": "./dist/statement-page*.js", "maxSize": "1.0KB" }, | ||
| { "path": "./dist/payment-attempt-page*.js", "maxSize": "3.0KB" }, |
There was a problem hiding this comment.
is there a reason we have seperate chunks between UP and OP for plans, but not for statements and PAs ?
| @@ -250,7 +250,7 @@ const AddPaymentSourceForm = ({ children }: PropsWithChildren) => { | |||
| const { setupIntent, error } = await stripe.confirmSetup({ | |||
| elements, | |||
| confirmParams: { | |||
| return_url: '', // TODO(@COMMERCE): need to figure this out | |||
| return_url: '', | |||
There was a problem hiding this comment.
Are we plan to support them in the future ? Are we planning to communicate that they will need to bump sdk versions in order to use it at that point ?
Maybe at least we so something like, unless '' has the same effect.
| return_url: '', | |
| return_url: '', |
| return_url: '', | |
| return_url: window.location.href, |
| @@ -294,7 +294,7 @@ const AddPaymentSourceForm = ({ children }: PropsWithChildren) => { | |||
| ? { | |||
| recurringPaymentRequest: { | |||
| paymentDescription: `${t(localizationKeys(checkout.planPeriod === 'month' ? 'commerce.paymentSource.applePayDescription.monthly' : 'commerce.paymentSource.applePayDescription.annual'))}`, | |||
| managementURL: displayConfig.homeUrl, // TODO(@COMMERCE): is this the right URL? | |||
| managementURL: displayConfig.homeUrl, | |||
There was a problem hiding this comment.
this will probably land them to the index page of their website or / of AP.
Would it be better to do
displayConfig.organizationProfileUrl for orgs and then displayConfig.userProfileUrl for users ?
Description
Address leftover
TODOs from Commerce launch.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
Performance Improvements
Chores