Skip to content

chore(clerk-js): Address leftover commerce TODOs #6183

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

chore(clerk-js): Address leftover commerce TODOs #6183

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/silent-zebras-refuse.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@clerk/clerk-js': patch
---

Cleanup leftover Commerce-launch TODOs
10 changes: 7 additions & 3 deletions packages/clerk-js/bundlewatch.config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,26 @@
{ "path": "./dist/coinbase*.js", "maxSize": "38KB" },
{ "path": "./dist/createorganization*.js", "maxSize": "5KB" },
{ "path": "./dist/impersonationfab*.js", "maxSize": "5KB" },
{ "path": "./dist/organizationprofile*.js", "maxSize": "12KB" },
{ "path": "./dist/organizationprofile*.js", "maxSize": "10KB" },
{ "path": "./dist/organizationswitcher*.js", "maxSize": "5KB" },
{ "path": "./dist/organizationlist*.js", "maxSize": "5.5KB" },
{ "path": "./dist/signin*.js", "maxSize": "14KB" },
{ "path": "./dist/signup*.js", "maxSize": "8.5KB" },
{ "path": "./dist/userbutton*.js", "maxSize": "5KB" },
{ "path": "./dist/userprofile*.js", "maxSize": "16.5KB" },
{ "path": "./dist/userprofile*.js", "maxSize": "16KB" },
{ "path": "./dist/userverification*.js", "maxSize": "5KB" },
{ "path": "./dist/onetap*.js", "maxSize": "1KB" },
{ "path": "./dist/waitlist*.js", "maxSize": "1.5KB" },
{ "path": "./dist/keylessPrompt*.js", "maxSize": "6.5KB" },
{ "path": "./dist/pricingTable*.js", "maxSize": "4.02KB" },
{ "path": "./dist/checkout*.js", "maxSize": "7.25KB" },
{ "path": "./dist/paymentSources*.js", "maxSize": "9.17KB" },
{ "path": "./dist/up-billing-page*.js", "maxSize": "3.0KB" },
{ "path": "./dist/up-billing-page*.js", "maxSize": "3.5KB" },
{ "path": "./dist/op-billing-page*.js", "maxSize": "3.0KB" },
{ "path": "./dist/up-plans-page*.js", "maxSize": "1.0KB" },
{ "path": "./dist/op-plans-page*.js", "maxSize": "1.0KB" },
{ "path": "./dist/statement-page*.js", "maxSize": "1.0KB" },
{ "path": "./dist/payment-attempt-page*.js", "maxSize": "3.0KB" },
Comment on lines +30 to +31
Copy link
Member

Choose a reason for hiding this comment

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

is there a reason we have seperate chunks between UP and OP for plans, but not for statements and PAs ?

{ "path": "./dist/sessionTasks*.js", "maxSize": "1.5KB" }
]
}
2 changes: 0 additions & 2 deletions packages/clerk-js/src/ui/components/Checkout/parts.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { LineItems } from '@/ui/elements/LineItems';

import { useCheckoutContext } from '../../contexts';
import { Box, descriptors, Flex, localizationKeys, useLocalizations } from '../../customizables';
// TODO(@COMMERCE): Is this causing bundle size issues ?
import { EmailForm } from '../UserProfile/EmailForm';
import { useCheckoutContextRoot } from './CheckoutPage';

Expand Down Expand Up @@ -101,7 +100,6 @@ export const AddEmailForm = () => {
padding: t.space.$4,
})}
>
{/* TODO(@COMMERCE): How does ths operate for orgs ? */}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This flow works for orgs by grabbing the current member's email (once added) and associating that with the payer record

<EmailForm
title={localizationKeys('commerce.checkout.emailForm.title')}
subtitle={localizationKeys('commerce.checkout.emailForm.subtitle')}
Expand Down
24 changes: 18 additions & 6 deletions packages/clerk-js/src/ui/components/OrganizationProfile/OrganizationProfileRoutes.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,6 @@ import { useEnvironment, useOrganizationProfileContext } from '../../contexts';
import { Route, Switch } from '../../router';
import { OrganizationGeneralPage } from './OrganizationGeneralPage';
import { OrganizationMembers } from './OrganizationMembers';
import { OrganizationPaymentAttemptPage } from './OrganizationPaymentAttemptPage';
import { OrganizationPlansPage } from './OrganizationPlansPage';
import { OrganizationStatementPage } from './OrganizationStatementPage';

const OrganizationBillingPage = lazy(() =>
import(/* webpackChunkName: "op-billing-page"*/ './OrganizationBillingPage').then(module => ({
Expand All @@ -22,6 +19,24 @@ const OrganizationAPIKeysPage = lazy(() =>
})),
);

const OrganizationPlansPage = lazy(() =>
import(/* webpackChunkName: "op-plans-page"*/ './OrganizationPlansPage').then(module => ({
default: module.OrganizationPlansPage,
})),
);

const OrganizationStatementPage = lazy(() =>
import(/* webpackChunkName: "statement-page"*/ './OrganizationStatementPage').then(module => ({
default: module.OrganizationStatementPage,
})),
);

const OrganizationPaymentAttemptPage = lazy(() =>
import(/* webpackChunkName: "payment-attempt-page"*/ './OrganizationPaymentAttemptPage').then(module => ({
default: module.OrganizationPaymentAttemptPage,
})),
);

export const OrganizationProfileRoutes = () => {
const { pages, isMembersPageRoot, isGeneralPageRoot, isBillingPageRoot, isApiKeysPageRoot } =
useOrganizationProfileContext();
Expand Down Expand Up @@ -82,19 +97,16 @@ export const OrganizationProfileRoutes = () => {
</Suspense>
</Route>
<Route path='plans'>
{/* TODO(@commerce): Should this be lazy loaded ? */}
<Suspense fallback={''}>
<OrganizationPlansPage />
</Suspense>
</Route>
<Route path='statement/:statementId'>
{/* TODO(@commerce): Should this be lazy loaded ? */}
<Suspense fallback={''}>
<OrganizationStatementPage />
</Suspense>
</Route>
<Route path='payment-attempt/:paymentAttemptId'>
{/* TODO(@commerce): Should this be lazy loaded ? */}
<Suspense fallback={''}>
<OrganizationPaymentAttemptPage />
</Suspense>
Expand Down
4 changes: 2 additions & 2 deletions packages/clerk-js/src/ui/components/PaymentSources/AddPaymentSource.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: '',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We prevent payment methods that require a redirect, so this param is never used (though required)

Copy link
Member

Choose a reason for hiding this comment

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

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.

Suggested change
return_url: '',
return_url: '',
Suggested change
return_url: '',
return_url: window.location.href,

},
redirect: 'if_required',
});
Expand Down Expand Up @@ -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,
Copy link
Member

Choose a reason for hiding this comment

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

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 ?

regularBilling: {
amount: checkout.totals.totalDueNow?.amount || checkout.totals.grandTotal.amount,
label: checkout.plan.name,
Expand Down
24 changes: 18 additions & 6 deletions packages/clerk-js/src/ui/components/UserProfile/UserProfileRoutes.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,7 @@ import { CustomPageContentContainer } from '../../common/CustomPageContentContai
import { USER_PROFILE_NAVBAR_ROUTE_ID } from '../../constants';
import { useEnvironment, useUserProfileContext } from '../../contexts';
import { Route, Switch } from '../../router';
import { PaymentAttemptPage } from '../PaymentAttempts';
import { StatementPage } from '../Statements';
import { AccountPage } from './AccountPage';
import { PlansPage } from './PlansPage';
import { SecurityPage } from './SecurityPage';

const BillingPage = lazy(() =>
Expand All @@ -22,6 +19,24 @@ const APIKeysPage = lazy(() =>
})),
);

const PlansPage = lazy(() =>
import(/* webpackChunkName: "up-plans-page"*/ './PlansPage').then(module => ({
default: module.PlansPage,
})),
);

const StatementPage = lazy(() =>
import(/* webpackChunkName: "statement-page"*/ '../Statements').then(module => ({
default: module.StatementPage,
})),
);

const PaymentAttemptPage = lazy(() =>
import(/* webpackChunkName: "payment-attempt-page"*/ '../PaymentAttempts').then(module => ({
default: module.PaymentAttemptPage,
})),
);

export const UserProfileRoutes = () => {
const { pages } = useUserProfileContext();
const { apiKeysSettings, commerceSettings } = useEnvironment();
Expand Down Expand Up @@ -74,19 +89,16 @@ export const UserProfileRoutes = () => {
</Suspense>
</Route>
<Route path='plans'>
{/* TODO(@commerce): Should this be lazy loaded ? */}
<Suspense fallback={''}>
<PlansPage />
</Suspense>
</Route>
<Route path='statement/:statementId'>
{/* TODO(@commerce): Should this be lazy loaded ? */}
<Suspense fallback={''}>
<StatementPage />
</Suspense>
</Route>
<Route path='payment-attempt/:paymentAttemptId'>
{/* TODO(@commerce): Should this be lazy loaded ? */}
<Suspense fallback={''}>
<PaymentAttemptPage />
</Suspense>
Expand Down
Loading