refactor: remove excessive ternary operators in SubscriptionDisplay component #481
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe code refactors the subscription display component by extracting inline conditional rendering logic into smaller, reusable helper components and a render function. The main component now delegates description rendering to a dedicated function, improving modularity and readability while maintaining the existing UI behavior and control flow. The support contact link for paid plans was removed. Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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: 1
🧹 Nitpick comments (3)
frontend/src/components/settings/subscription-display.tsx (3)
19-24: Prefer an exhaustiveswitchfor compile-time safety
getPlanDescriptionrelies on anif/elsethat silently falls back for every non-Freeplan.
Using an exhaustiveswitch(or object literal map) makes it impossible to forget to add copy when a newPlanvalue is introduced and lets TypeScript warn you.-function getPlanDescription(plan: Plan): string { - if (plan === Plan.Free) { - return "Basic features for small teams"; - } - return "Advanced features for growing organizations"; -} +function getPlanDescription(plan: Plan): string { + switch (plan) { + case Plan.Free: + return "Basic features for small teams"; + case Plan.Pro: + return "Advanced features for growing organizations"; + case Plan.Enterprise: + return "Tailored features & SLA for large organisations"; + // `never` forces us to handle new enum members + default: + const _exhaustiveCheck: never = plan; + return _exhaustiveCheck; + } +}
77-82: Title-casing the plan name is brittle
charAt(0).toUpperCase() + slice(1)works only ifPlanis a lower-case string literal (e.g."free").
If the enum uses upper-case literals ("FREE") or numeric enums, this produces undesirable output ("FREE Plan"or blows up at runtime).Consider:
-<p className="text-sm font-medium"> - {subscription.plan.charAt(0).toUpperCase() + - subscription.plan.slice(1) + - " Plan"} -</p> +<p className="text-sm font-medium"> + {planDisplayName(subscription.plan)} +</p>with a helper:
function planDisplayName(plan: Plan) { return { [Plan.Free]: "Free", [Plan.Pro]: "Pro", [Plan.Enterprise]: "Enterprise", }[plan] ?? String(plan); }
102-122:renderDescriptioncould be memoised to avoid re-creating JSX each renderAlthough not a performance hotspot,
renderDescriptionis recreated on every parent render, triggering an unconditional re-render ofSettingsItem. Wrapping it withuseMemo(or pulling JSX inline) prevents unnecessary reconciliation.-export function SubscriptionDisplay({...}: SubscriptionDisplayProps) { - function renderDescription() { +export function SubscriptionDisplay({...}: SubscriptionDisplayProps) { + const description = useMemo(() => { if (isLoading) { return (...) } if (subscription) { return (<SubscriptionContent ... />) } return null; - } + }, [isLoading, subscription, onManageSubscription]);(and pass
descriptiontoSettingsItem).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/components/settings/subscription-display.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
frontend/src/components/settings/subscription-display.tsx (2)
backend/aci/common/db/sql_models.py (1)
Plan(476-503)frontend/src/components/settings/settings-item.tsx (1)
SettingsItem(15-39)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Compose Tests
- GitHub Check: Format & Lint
- GitHub Check: Format, Lint, and Test
…in-subscription-display
|
✨ No issues found! Your code is sparkling clean! ✨ Need help? Join our Discord for support! |
🏷️ Ticket
notion
📝 Description
🎥 Demo (if applicable)
📸 Screenshots (if applicable)
✅ Checklist
Summary by CodeRabbit