-
Notifications
You must be signed in to change notification settings - Fork 407
chore(clerk-js): Replace useOrganization with useOrganizationContext within billing components
#7257
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.
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
Conversation
🦋 Changeset detectedLatest commit: 83e7f51 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 |
WalkthroughReplaces Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (3)
🧰 Additional context used🧬 Code graph analysis (1)packages/clerk-js/src/ui/components/PaymentAttempts/PaymentAttemptPage.tsx (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). (1)
🔇 Additional comments (4)
Comment |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.changeset/cuddly-cougars-buy.md (1)
5-5: Minor grammar fix: use hyphen in compound adjective.The phrase "not user facing" should be hyphenated as "not user-facing" when used as a compound adjective.
Apply this diff:
-Internal change, not user facing: Replace `useOrganization` within billing components with `useOrganizationContext` +Internal change, not user-facing: Replace `useOrganization` within billing components with `useOrganizationContext`
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
.changeset/cuddly-cougars-buy.md(1 hunks)packages/clerk-js/src/ui/components/PaymentAttempts/PaymentAttemptPage.tsx(3 hunks)packages/clerk-js/src/ui/components/PaymentMethods/PaymentMethods.tsx(5 hunks)packages/clerk-js/src/ui/components/PricingTable/PricingTableDefault.tsx(3 hunks)packages/clerk-js/src/ui/components/Statements/StatementPage.tsx(3 hunks)packages/clerk-js/src/ui/components/SubscriptionDetails/index.tsx(4 hunks)packages/clerk-js/src/ui/contexts/components/Plans.tsx(3 hunks)
🧰 Additional context used
🪛 LanguageTool
.changeset/cuddly-cougars-buy.md
[grammar] ~5-~5: Use a hyphen to join words.
Context: ...s': patch --- Internal change, not user facing: Replace useOrganization within...
(QB_NEW_EN_HYPHEN)
🔇 Additional comments (6)
packages/clerk-js/src/ui/components/SubscriptionDetails/index.tsx (1)
1-1: LGTM! Clean refactoring fromuseOrganizationtouseOrganizationContext.The refactoring is correct and consistent:
- Import updated to
useOrganizationContext- Variable renamed to
organizationCtx- Access path updated to
organizationCtx?.organization?.id- Dependency array correctly references the primitive ID value
The optional chaining ensures null-safety is maintained throughout.
Also applies to: 176-176, 198-198, 216-216
packages/clerk-js/src/ui/components/PricingTable/PricingTableDefault.tsx (1)
1-1: LGTM! Consistent refactoring pattern.The changes follow the same pattern as other components:
- Hook updated to
useOrganizationContext- Variable renamed to
organizationCtx- Boolean check updated to
!!organizationCtx?.organizationThe refactoring maintains the same null-safety guarantees.
Also applies to: 107-107, 140-140
packages/clerk-js/src/ui/components/PaymentMethods/PaymentMethods.tsx (1)
1-1: LGTM! Both RemoveScreen and PaymentMethodMenu updated correctly.The refactoring is applied consistently across both nested components:
RemoveScreenusesorganizationCtx?.organization?.idin the remove flow (line 75)PaymentMethodMenuusesorganizationCtx?.organization?.idin the makeDefault flow (line 218)Both maintain proper null-safety with optional chaining.
Also applies to: 63-63, 75-75, 199-199, 218-218
packages/clerk-js/src/ui/components/Statements/StatementPage.tsx (1)
1-1: LGTM! SWR key and API call updated consistently.The refactoring correctly updates both:
- The SWR cache key (line 40) to include
organizationCtx?.organization?.id- The API call parameter (line 46) to pass
organizationCtx?.organization?.idThis ensures SWR caching behavior remains correct with the new context structure.
Also applies to: 26-26, 40-40, 46-46
packages/clerk-js/src/ui/components/PaymentAttempts/PaymentAttemptPage.tsx (1)
1-1: LGTM! Consistent refactoring with SWR integration.Following the same pattern as StatementPage, both the SWR cache key (line 46) and the API call (line 52) are updated to use
organizationCtx?.organization?.id. This maintains correct caching behavior.Also applies to: 32-32, 46-46, 52-52
packages/clerk-js/src/ui/contexts/components/Plans.tsx (1)
8-8: LGTM! Hook enablement logic correctly updated.The refactoring updates the billing hook enablement condition to check
Boolean(organizationCtx?.organization)instead of checking the organization directly. This maintains the same logical behavior while using the new context structure.Also applies to: 35-35, 47-47
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.changeset/cuddly-cougars-buy.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
.changeset/cuddly-cougars-buy.md
[grammar] ~5-~5: Use a hyphen to join words.
Context: ...s': patch --- Internal change, not user facing: Replace useOrganization with `...
(QB_NEW_EN_HYPHEN)
⏰ 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)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (1)
.changeset/cuddly-cougars-buy.md (1)
1-5: Changeset structure and versioning look good.The YAML format is correct, package identifier matches the target, and the
patchversion bump is appropriate for this internal refactoring. The description accurately captures the scope of the hook replacement across billing components.
@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: |
| const { params, navigate } = useRouter(); | ||
| const subscriberType = useSubscriberTypeContext(); | ||
| const { organization } = useOrganization(); | ||
| const organizationCtx = useOrganizationContext(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❔ is appending Ctx to a variable context a pattern?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct, it's unsafe to apply destructuring ({ organization }) to the return value, as the context might be undefined, so it's better to access with the nullish operator organizationCtx?.organization
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TIL
| const card = useCardState(); | ||
| const subscriberType = useSubscriberTypeContext(); | ||
| const { organization } = useOrganization(); | ||
| const organizationCtx = useOrganizationContext(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❔ Is it worth adding a comment explaining why we are using useOrganizationContext?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I was writing billing components, would be nice to see it as a reference. I recognize that commenting on every reference of it can be repetitive but seems worth it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, you are right. Feel free to ignore my comment
d8b067c to
83e7f51
Compare
Description
There's a PR, introducing a guard within the organization public hooks, to trigger a in-app UI prompt to enable organizations.
This flow doesn't work well when triggered within billing components or the
useCheckouthook, when used withfor=organizations, since the order of operations and the current UX when enabling billing from the Dashboard.For now, we'll keep the prompt specific to organization components, and later think about an in-app flow to enable billing if necessary.
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit