fix: Resolve BillingPlan tree-shaking issue causing runtime error#24229
fix: Resolve BillingPlan tree-shaking issue causing runtime error#24229
Conversation
…-shaking TypeScript enums compile to IIFEs which can be incorrectly tree-shaken by webpack when 'sideEffects: false' is set in package.json. Converting to a const object with 'as const' avoids the IIFE pattern while maintaining full type safety. This fixes the 'TRPCError: BillingPlan is not defined' error that occurred in the hasTeamPlan tRPC handler. Follows the existing CHECKOUT_SESSION_TYPES pattern in the same file. Co-Authored-By: alex@cal.com <me@alexvanandel.com>
The issue was a module initialization order problem. When BillingPlan was imported from constants.ts into billing-plans.ts and used inside the BillingPlanService class, webpack's tree-shaker (with sideEffects: false) couldn't properly track the value dependency across the package boundary. By re-exporting BillingPlan from the same module that exports BillingPlanService, we ensure the constant is bundled together with the class that uses it, preventing tree-shaking. Co-Authored-By: alex@cal.com <me@alexvanandel.com>
The const object conversion didn't fix the tree-shaking issue. The real fix is re-exporting BillingPlan from billing-plans.ts to ensure it's part of the same module as BillingPlanService. Co-Authored-By: alex@cal.com <me@alexvanandel.com>
By making BillingPlan a private static member of BillingPlanService, webpack now sees it as part of the class definition rather than just used inside methods. This creates a strong reference that prevents webpack's tree-shaker from removing the enum initialization IIFE when 'sideEffects: false' is set. With the previous approach where BillingPlan was only imported and used inside methods, webpack's static analysis couldn't properly track the enum usage across package boundaries (@calcom/features -> @calcom/trpc), causing it to incorrectly determine the enum initialization was unused code. Fixes the TRPCError: BillingPlan is not defined runtime error. Thread: https://calendso.slack.com/archives/C08LT9BLEET/p1759420015428149 Co-Authored-By: alex@cal.com <me@alexvanandel.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
WalkthroughReplaces the exported BillingPlan enum with a string-literal map Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
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. Comment |
- Remove unnecessary re-export statement - Use direct BillingPlan references instead of BillingPlanService.BillingPlan - Add detailed comment explaining webpack tree-shaking workaround - Keep import from constants.ts and private static member for webpack reference Co-Authored-By: alex@cal.com <me@alexvanandel.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
- Remove Slack URL from explanatory comment - Restore BillingPlanService.BillingPlan usage throughout class methods - This static member usage is essential for webpack to track the enum reference Co-Authored-By: alex@cal.com <me@alexvanandel.com>
![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
📜 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)
packages/features/ee/billing/domain/billing-plans.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/features/ee/billing/domain/billing-plans.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/ee/billing/domain/billing-plans.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/ee/billing/domain/billing-plans.ts
🧬 Code graph analysis (1)
packages/features/ee/billing/domain/billing-plans.ts (1)
packages/features/ee/billing/constants.ts (2)
PLATFORM_ENTERPRISE_SLUGS(31-31)ENTERPRISE_SLUGS(32-32)
🔇 Additional comments (1)
packages/features/ee/billing/domain/billing-plans.ts (1)
39-84: No direct BillingPlan imports found
Ripgrep confirms no files import theBillingPlanenum fromconstants.ts; all usages go throughBillingPlanService.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/features/ee/billing/domain/billing-plans.ts (1)
55-69: Remove outdated bundler comment and consider simplification.Lines 57-58 contain a comment about bundler intelligence that appears outdated. Since the PR switched from enum to const object (which solves the tree-shaking issue), this comment about early returns affecting
BILLING_PLANSclearing is no longer relevant.Apply this diff to remove the outdated comment:
- // (emrysal) if we do an early return on !teamMetadata?.subscriptionId here, the bundler is not smart enough to infer - // that it shouldn't clear out the BILLING_PLANS before the for() scope finishes. if (team.isOrganization && teamMetadata?.subscriptionId) {Additionally, the control flow could be simplified by using early returns:
const teamMetadataResult = teamMetadataStrictSchema.safeParse(team.metadata ?? {}); const teamMetadata = teamMetadataResult.success ? teamMetadataResult.data : null; + + if (!teamMetadata?.subscriptionId) continue; + if (team.isOrganization && teamMetadata?.subscriptionId) { return ENTERPRISE_SLUGS.includes(team.slug ?? "") ? BILLING_PLANS.ENTERPRISE : BILLING_PLANS.ORGANIZATIONS; } - if (teamMetadata?.subscriptionId) { - return BILLING_PLANS.TEAMS; - } - // no subscriptionId or parent subscription id in this loop, so this membership hasn't got a plan. - // continue; + return BILLING_PLANS.TEAMS;
📜 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 (2)
packages/features/ee/billing/constants.ts(1 hunks)packages/features/ee/billing/domain/billing-plans.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/features/ee/billing/constants.tspackages/features/ee/billing/domain/billing-plans.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/ee/billing/constants.tspackages/features/ee/billing/domain/billing-plans.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/ee/billing/constants.tspackages/features/ee/billing/domain/billing-plans.ts
🧬 Code graph analysis (1)
packages/features/ee/billing/domain/billing-plans.ts (2)
packages/features/ee/billing/constants.ts (5)
BillingPlan(23-23)BILLING_PLANS(11-21)PLATFORM_ENTERPRISE_SLUGS(33-33)PLATFORM_PLANS_MAP(25-31)ENTERPRISE_SLUGS(34-34)packages/prisma/zod-utils.ts (1)
teamMetadataStrictSchema(389-405)
🔇 Additional comments (8)
packages/features/ee/billing/constants.ts (3)
23-23: LGTM! Type-safe approach for billing plans.The
BillingPlantype derived fromBILLING_PLANSis idiomatic and provides better tree-shaking than the previous enum approach. This addresses the root cause mentioned in the PR objectives.
25-31: LGTM! Map correctly updated for new structure.The
PLATFORM_PLANS_MAPhas been correctly updated to reference the newBILLING_PLANSconstants, maintaining type safety withRecord<string, BillingPlan>.
34-34: LGTM! Safe environment variable handling.The
ENTERPRISE_SLUGSconstant follows the same safe pattern asPLATFORM_ENTERPRISE_SLUGS, with appropriate fallback to an empty array when the environment variable is not set.packages/features/ee/billing/domain/billing-plans.ts (5)
1-33: LGTM! Imports and return type correctly updated.The imports and return type have been correctly updated to use the new
BILLING_PLANSconstant andBillingPlantype, aligning with the improved tree-shaking approach.
43-54: LGTM! Parent organization logic is correct.The safeParse approach for parent metadata is safe and the logic correctly identifies enterprise vs. organizations plans based on
ENTERPRISE_SLUGS.
70-70: LGTM! Appropriate fallback for unknown plans.The final return of
BILLING_PLANS.UNKNOWNis correct and provides a safe default when no billing plan is determined from memberships.
1-72: Previous review concerns addressed by const object approach.The current implementation uses the const object + type union approach (Option 2 from the previous review), which is one of the recommended solutions. This eliminates the need for the
private static BillingPlanworkaround and provides better tree-shaking without coupling the enum to the service class.Based on past review comments.
36-42: Ensure type-safe fallback for platform billing plansLine 41 may return a raw string outside the
BillingPlanenum. Replace the fallback with an explicit value or cast:
return PLATFORM_PLANS_MAP[team.platformBilling.plan] ?? BILLING_PLANS.UNKNOWN;- or
return PLATFORM_PLANS_MAP[team.platformBilling.plan] ?? (team.platformBilling.plan as BillingPlan);Verify that
PLATFORM_PLANS_MAPincludes all possibleteam.platformBilling.planvalues.
E2E results are ready! |
3 participants
What does this PR do?
Fixes a critical runtime error where
BillingPlanenum was being tree-shaken by webpack despite being properly imported and used, causingTRPCError: BillingPlan is not definedin thehasTeamPlantRPC endpoint.Thread: https://calendso.slack.com/archives/C08LT9BLEET/p1759420015428149
Requested by: @emrysal
Link to Devin run: https://app.devin.ai/sessions/6e14b204c786437f94e25ae8e99221e0
How should this be tested?
hasTeamPlantRPC endpoint (occurs when accessing team-related features)TRPCError: BillingPlan is not definedshould occurHuman Review Checklist
Mandatory Tasks (DO NOT REMOVE)
Note: This is a webpack bundling issue that's difficult to unit test. The fix effectiveness can only be verified through runtime testing of the tRPC endpoint.