perf: Create 'heavy' eventTypes router#23664
Conversation
WalkthroughAdds a new viewer.eventTypes.heavy sub-router that exposes create, update, and duplicate mutations and removes those mutations from the original viewer.eventTypes router. Updates server routing to merge the heavy sub-router into viewer.eventTypes, adds a Next.js API route at apps/web/pages/api/trpc/eventTypes/heavy/[trpc].ts, moves related handler/schema files under the heavy directory (with adjusted imports and tests), and updates server schema re-exports. Updates client-side tRPC resolution and ENDPOINTS to recognize 4-segment paths like viewer.eventTypes.heavy.create and changes client code to call trpc.viewer.eventTypes.heavy.* for the moved mutations. Possibly related PRs
✨ 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 |
|
@CodeRabbit Please write a description for this PR |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (10)
packages/trpc/server/routers/viewer/eventTypes/heavy/duplicate.handler.ts (2)
30-60: Replace Prisma include with select (repo guideline) to cut payload and risk.Use
selectfor only the fields you truly need; drop unused relations likescheduleandwebhooks.Apply along these lines (fill exact fields required):
- const eventType = await prisma.eventType.findUnique({ - where: { id: originalEventTypeId }, - include: { - customInputs: true, - schedule: true, - users: { select: { id: true } }, - hosts: true, - team: true, - workflows: true, - webhooks: true, - hashedLink: true, - destinationCalendar: true, - calVideoSettings: { - select: { - disableRecordingForOrganizer: true, - disableRecordingForGuests: true, - enableAutomaticTranscription: true, - enableAutomaticRecordingForOrganizer: true, - redirectUrlOnExit: true, - disableTranscriptionForGuests: true, - disableTranscriptionForOrganizer: true, - }, - }, - }, - }); + const eventType = await prisma.eventType.findUnique({ + where: { id: originalEventTypeId }, + select: { + id: true, + userId: true, + teamId: true, + // fields actually used later... + locations: true, + recurringEvent: true, + bookingLimits: true, + durationLimits: true, + eventTypeColor: true, + customReplyToEmail: true, + metadata: true, + bookingFields: true, + rrSegmentQueryValue: true, + assignRRMembersUsingSegment: true, + secondaryEmailId: true, + instantMeetingScheduleId: true, + restrictionScheduleId: true, + users: { select: { id: true } }, + team: { select: { id: true } }, + hosts: { + select: { + userId: true, priority: true, weight: true, isFixed: true, scheduleId: true, groupId: true + }, + }, + workflows: { select: { workflowId: true } }, + hashedLink: { select: { eventTypeId: true, expiresAt: true, maxUsageCount: true } }, + destinationCalendar: { select: { externalId: true, integration: true, credentialId: true } }, + calVideoSettings: { + select: { + disableRecordingForOrganizer: true, + disableRecordingForGuests: true, + enableAutomaticTranscription: true, + enableAutomaticRecordingForOrganizer: true, + redirectUrlOnExit: true, + disableTranscriptionForGuests: true, + disableTranscriptionForOrganizer: true, + }, + }, + // If needed, fetch customInputs with a minimal sub-select + customInputs: true, // TODO: narrow to exact fields consumed below + }, + });
228-237: Don’t leak raw errors to clients.Return a generic message and log details server-side instead of interpolating the thrown
errorinto the client-visible message.- throw new TRPCError({ code: "INTERNAL_SERVER_ERROR", message: `Error duplicating event type ${error}` }); + throw new TRPCError({ + code: "INTERNAL_SERVER_ERROR", + message: "Error duplicating event type.", + });apps/web/modules/apps/installation/[[...step]]/step-view.tsx (1)
188-207: Fix duplicate toasts and shadowed variable in onError.
const messageshadows the outermessageand you also toast twice. Consolidate message construction and toast once.- onError: (err) => { - let message = ""; - if (err instanceof HttpError) { - const message = `${err.statusCode}: ${err.message}`; - showToast(message, "error"); - } - - if (err.data?.code === "UNAUTHORIZED") { - message = `${err.data.code}: ${t("error_event_type_unauthorized_update")}`; - } - - if (err.data?.code === "PARSE_ERROR" || err.data?.code === "BAD_REQUEST") { - message = `${err.data.code}: ${t(err.message)}`; - } - - if (err.data?.code === "INTERNAL_SERVER_ERROR") { - message = t("unexpected_error_try_again"); - } - - showToast(message ? t(message) : t(err.message), "error"); - }, + onError: (err) => { + let message: string | undefined; + if (err instanceof HttpError) { + message = `${err.statusCode}: ${err.message}`; + } else if (err.data?.code === "UNAUTHORIZED") { + message = `${err.data.code}: ${t("error_event_type_unauthorized_update")}`; + } else if (err.data?.code === "PARSE_ERROR" || err.data?.code === "BAD_REQUEST") { + message = t(err.message); + } else if (err.data?.code === "INTERNAL_SERVER_ERROR") { + message = t("unexpected_error_try_again"); + } else { + message = t(err.message); + } + showToast(message, "error"); + },apps/web/components/getting-started/steps-views/UserProfile.tsx (1)
56-66: Await event type creations during onboarding.
mutate()returns void;Promise.all([...mutate()])won’t wait. UsemutateAsyncto ensure completion before redirect.- await Promise.all( - DEFAULT_EVENT_TYPES.map(async (event) => { - return createEventType.mutate(event); - }) - ); + await Promise.all( + DEFAULT_EVENT_TYPES.map((event) => createEventType.mutateAsync(event)) + );packages/trpc/server/routers/viewer/eventTypes/heavy/update.handler.ts (2)
521-524: Use select instead of include for Prisma (repo guideline).Avoid
include: { steps: true }; select only the step fields needed by the allow-disabling checks.- include: { - steps: true, - }, + select: { + steps: { + // TODO: narrow to the exact properties used by + // allowDisablingHostConfirmationEmails / allowDisablingAttendeeConfirmationEmails + select: { id: true, type: true }, + }, + },
656-676: Return updated eventType data (title can be stale).You return the pre-update
eventType, so consumers may show outdated values. Select needed fields from the update and return those.- const updatedEventTypeSelect = { - slug: true, - schedulingType: true, - } satisfies Prisma.EventTypeSelect; + const updatedEventTypeSelect = { + id: true, + title: true, + slug: true, + schedulingType: true, + teamId: true, + } satisfies Prisma.EventTypeSelect; @@ - return { eventType }; + return { eventType: updatedEventType };Also applies to: 717-717
packages/features/eventtypes/components/DuplicateDialog.tsx (1)
92-107: Avoid double error toasts; compute once and toast once.Current logic toasts in the HttpError branch and again later. Consolidate.
- onError: (err) => { - if (err instanceof HttpError) { - const message = `${err.statusCode}: ${err.message}`; - showToast(message, "error"); - } - - if (err.data?.code === "INTERNAL_SERVER_ERROR" || err.data?.code === "BAD_REQUEST") { - const message = t("unexpected_error_try_again"); - showToast(message, "error"); - } - - if (err.data?.code === "UNAUTHORIZED" || err.data?.code === "FORBIDDEN") { - const message = `${err.data.code}: ${t("error_event_type_unauthorized_create")}`; - showToast(message, "error"); - } - }, + onError: (err) => { + let message: string | undefined; + if (err instanceof HttpError) { + message = `${err.statusCode}: ${err.message}`; + } else if (err.data?.code === "UNAUTHORIZED" || err.data?.code === "FORBIDDEN") { + message = `${err.data.code}: ${t("error_event_type_unauthorized_create")}`; + } else if (err.data?.code === "INTERNAL_SERVER_ERROR" || err.data?.code === "BAD_REQUEST") { + message = t("unexpected_error_try_again"); + } else { + message = t(err.message); + } + showToast(message, "error"); + },packages/platform/atoms/event-types/wrappers/EventTypeWebWrapper.tsx (1)
162-181: Fix error handler shadowing and double toasts
const messageinside the HttpError block shadows the outermessage, and you toast twice. Assign to the outer var and toast once at the end.- onError: (err) => { - let message = ""; - if (err instanceof HttpError) { - const message = `${err.statusCode}: ${err.message}`; - showToast(message, "error"); - } + onError: (err) => { + let message = ""; + if (err instanceof HttpError) { + message = `${err.statusCode}: ${err.message}`; + } @@ - showToast(message ? t(message) : t(err.message), "error"); + showToast(message || t(err.message), "error"); },packages/trpc/react/trpc.ts (1)
30-45: Guard heavy-path resolver for 4+ segments and support nested actionsApply this diff in packages/trpc/react/trpc.ts:
- } else if (parts.length >= 3 && parts[2] === "heavy") { - endpoint = parts[1] + "/heavy" as keyof typeof links; - path = parts[3]; + } else if (parts.length >= 4 && parts[2] === "heavy") { + endpoint = (parts[1] + "/heavy") as keyof typeof links; + path = parts.slice(3).join("."); } else {The
"eventTypes/heavy"endpoint is already registered in packages/trpc/react/shared.ts (line 19), so wiring is complete.apps/web/app/_trpc/trpc-client.ts (1)
18-34: Heavy-path resolver should require 4+ segments and handle nested actionsMirror the fix applied in shared react trpc resolver.
- } else if (parts.length >= 3 && parts[2] === "heavy") { - endpoint = parts[1] + "/heavy" as keyof typeof links; - path = parts[3]; - } - else { + } else if (parts.length >= 4 && parts[2] === "heavy") { + endpoint = (parts[1] + "/heavy") as keyof typeof links; + path = parts.slice(3).join("."); + } else {
🧹 Nitpick comments (8)
packages/trpc/server/routers/viewer/eventTypes/heavy/duplicate.handler.test.ts (1)
1-1: Introduce and use a path alias for test mocks
No@tests/*alias is configured inpackages/trpc/tsconfig.json; add one (for example:"compilerOptions": { "paths": { "@tests/*": ["../../../../../tests/*"] } }) and replace the deep-relative import with
import prismaMock from "@tests/libs/__mocks__/prismaMock".packages/trpc/server/routers/viewer/eventTypes/heavy/duplicate.handler.ts (1)
124-129: Whitelist host fields when cloning.Avoid spreading unknown fields from
hostsintocreateMany; explicitly map only allowed columns to prevent constraint or validation issues across schema changes.apps/web/modules/apps/installation/[[...step]]/step-view.tsx (1)
337-338: Prefer named export over default (repo guideline).Consider
export { OnboardingPage }to improve tree-shaking and refactors. Only keep default if a framework constraint requires it here.apps/web/components/getting-started/steps-views/UserProfile.tsx (1)
121-128: Localize placeholder text.Replace hardcoded “URL” with
t("url")to adhere to i18n policy.- placeholder="URL" + placeholder={t("url")}packages/trpc/server/routers/viewer/eventTypes/heavy/update.handler.ts (1)
549-555: Remove stray console logs.Use the repo logger at debug level if needed; avoid
console.login server handlers.- console.log("multiplePrivateLinks", multiplePrivateLinks); @@ - console.log("connectedLinks", connectedLinks);packages/features/eventtypes/components/DuplicateDialog.tsx (1)
47-49: Remove unused state.
searchTermis never updated; consider inliningdebouncedSearchTermor wiring the setter.- const [searchTerm, setSearchTerm] = useState(""); - const debouncedSearchTerm = useDebounce(searchTerm, 500); + const debouncedSearchTerm = useDebounce("", 500);apps/web/modules/event-types/views/event-types-listing-view.tsx (1)
271-306: Prefer immutably updating the cache over mutating props in onMutateYou’re mutating
pagesin place. UsesetInfiniteDatato avoid accidental shared-state mutations.- if (previousValue) { - pages?.forEach((page) => { - page?.eventTypes?.forEach((eventType) => { - if (eventType.id === data.id) { - eventType.hidden = !eventType.hidden; - } - }); - }); - } + if (previousValue) { + utils.viewer.eventTypes.getEventTypesFromGroup.setInfiniteData( + { limit: LIMIT, searchQuery: debouncedSearchTerm, group: { teamId: group?.teamId, parentId: group?.parentId } }, + (curr) => + curr && { + ...curr, + pages: curr.pages.map((p) => ({ + ...p, + eventTypes: p.eventTypes.map((et) => + et.id === data.id ? { ...et, hidden: !et.hidden } : et + ), + })), + } + ); + }packages/trpc/server/routers/viewer/eventTypes/heavy/_router.ts (1)
12-19: Remove unused handler cache scaffolding
BookingsRouterHandlerCacheandUNSTABLE_HANDLER_CACHEare unused. Drop them to reduce noise.-type BookingsRouterHandlerCache = { - create?: typeof import("./create.handler").createHandler; - duplicate?: typeof import("./duplicate.handler").duplicateHandler; - update?: typeof import("./update.handler").updateHandler; -}; - -const UNSTABLE_HANDLER_CACHE: BookingsRouterHandlerCache = {};
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (18)
apps/web/app/_trpc/trpc-client.ts(2 hunks)apps/web/components/getting-started/steps-views/UserProfile.tsx(1 hunks)apps/web/modules/apps/installation/[[...step]]/step-view.tsx(1 hunks)apps/web/modules/event-types/views/event-types-listing-view.tsx(1 hunks)apps/web/pages/api/trpc/eventTypes/heavy/[trpc].ts(1 hunks)packages/features/eventtypes/components/DuplicateDialog.tsx(1 hunks)packages/lib/hooks/useCreateEventType.ts(1 hunks)packages/platform/atoms/event-types/wrappers/EventTypeWebWrapper.tsx(1 hunks)packages/trpc/react/shared.ts(1 hunks)packages/trpc/react/trpc.ts(2 hunks)packages/trpc/server/routers/viewer/eventTypes/_router.ts(0 hunks)packages/trpc/server/routers/viewer/eventTypes/heavy/_router.ts(1 hunks)packages/trpc/server/routers/viewer/eventTypes/heavy/create.handler.ts(1 hunks)packages/trpc/server/routers/viewer/eventTypes/heavy/duplicate.handler.test.ts(1 hunks)packages/trpc/server/routers/viewer/eventTypes/heavy/duplicate.handler.ts(1 hunks)packages/trpc/server/routers/viewer/eventTypes/heavy/update.handler.ts(1 hunks)packages/trpc/server/routers/viewer/eventTypes/heavy/update.schema.ts(1 hunks)packages/trpc/server/routers/viewer/eventTypes/update.schema.ts(0 hunks)
💤 Files with no reviewable changes (2)
- packages/trpc/server/routers/viewer/eventTypes/update.schema.ts
- packages/trpc/server/routers/viewer/eventTypes/_router.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.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/trpc/server/routers/viewer/eventTypes/heavy/update.schema.tspackages/trpc/server/routers/viewer/eventTypes/heavy/create.handler.tspackages/trpc/react/shared.tspackages/lib/hooks/useCreateEventType.tsapps/web/app/_trpc/trpc-client.tsapps/web/pages/api/trpc/eventTypes/heavy/[trpc].tspackages/trpc/server/routers/viewer/eventTypes/heavy/update.handler.tspackages/trpc/server/routers/viewer/eventTypes/heavy/duplicate.handler.test.tspackages/trpc/server/routers/viewer/eventTypes/heavy/_router.tspackages/trpc/server/routers/viewer/eventTypes/heavy/duplicate.handler.tspackages/trpc/react/trpc.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/trpc/server/routers/viewer/eventTypes/heavy/update.schema.tspackages/trpc/server/routers/viewer/eventTypes/heavy/create.handler.tspackages/trpc/react/shared.tspackages/features/eventtypes/components/DuplicateDialog.tsxapps/web/modules/event-types/views/event-types-listing-view.tsxpackages/lib/hooks/useCreateEventType.tspackages/platform/atoms/event-types/wrappers/EventTypeWebWrapper.tsxapps/web/components/getting-started/steps-views/UserProfile.tsxapps/web/app/_trpc/trpc-client.tsapps/web/pages/api/trpc/eventTypes/heavy/[trpc].tspackages/trpc/server/routers/viewer/eventTypes/heavy/update.handler.tspackages/trpc/server/routers/viewer/eventTypes/heavy/duplicate.handler.test.tsapps/web/modules/apps/installation/[[...step]]/step-view.tsxpackages/trpc/server/routers/viewer/eventTypes/heavy/_router.tspackages/trpc/server/routers/viewer/eventTypes/heavy/duplicate.handler.tspackages/trpc/react/trpc.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/trpc/server/routers/viewer/eventTypes/heavy/update.schema.tspackages/trpc/server/routers/viewer/eventTypes/heavy/create.handler.tspackages/trpc/react/shared.tspackages/features/eventtypes/components/DuplicateDialog.tsxapps/web/modules/event-types/views/event-types-listing-view.tsxpackages/lib/hooks/useCreateEventType.tspackages/platform/atoms/event-types/wrappers/EventTypeWebWrapper.tsxapps/web/components/getting-started/steps-views/UserProfile.tsxapps/web/app/_trpc/trpc-client.tsapps/web/pages/api/trpc/eventTypes/heavy/[trpc].tspackages/trpc/server/routers/viewer/eventTypes/heavy/update.handler.tspackages/trpc/server/routers/viewer/eventTypes/heavy/duplicate.handler.test.tsapps/web/modules/apps/installation/[[...step]]/step-view.tsxpackages/trpc/server/routers/viewer/eventTypes/heavy/_router.tspackages/trpc/server/routers/viewer/eventTypes/heavy/duplicate.handler.tspackages/trpc/react/trpc.ts
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/features/eventtypes/components/DuplicateDialog.tsxapps/web/modules/event-types/views/event-types-listing-view.tsxpackages/platform/atoms/event-types/wrappers/EventTypeWebWrapper.tsxapps/web/components/getting-started/steps-views/UserProfile.tsxapps/web/modules/apps/installation/[[...step]]/step-view.tsx
🧠 Learnings (1)
📚 Learning: 2025-08-26T08:08:23.395Z
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23343
File: packages/features/insights/server/trpc-router.ts:1080-1101
Timestamp: 2025-08-26T08:08:23.395Z
Learning: In packages/features/insights/server/trpc-router.ts, when filtering personal event types (userId provided, no teamId, not isAll), the query correctly uses user.id (authenticated user) instead of the input userId parameter for security reasons. This prevents users from accessing other users' personal event types by passing arbitrary user IDs.
Applied to files:
packages/trpc/server/routers/viewer/eventTypes/heavy/create.handler.ts
🧬 Code graph analysis (4)
packages/features/eventtypes/components/DuplicateDialog.tsx (1)
packages/trpc/react/trpc.ts (1)
trpc(54-138)
packages/lib/hooks/useCreateEventType.ts (1)
packages/trpc/react/trpc.ts (1)
trpc(54-138)
apps/web/pages/api/trpc/eventTypes/heavy/[trpc].ts (2)
packages/trpc/server/createNextApiHandler.ts (1)
createNextApiHandler(10-88)packages/trpc/server/routers/viewer/eventTypes/heavy/_router.ts (1)
eventTypesRouter(20-45)
packages/trpc/server/routers/viewer/eventTypes/heavy/_router.ts (3)
packages/trpc/server/routers/viewer/eventTypes/_router.ts (1)
eventTypesRouter(31-163)packages/trpc/server/trpc.ts (1)
router(13-13)packages/trpc/server/routers/viewer/eventTypes/util.ts (1)
eventOwnerProcedure(19-93)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (10)
packages/lib/hooks/useCreateEventType.ts (1)
49-49: LGTM: migrated to heavy.create.Client path aligns with the new 4-segment endpoint and ENDPOINTS update.
packages/trpc/server/routers/viewer/eventTypes/heavy/duplicate.handler.ts (1)
30-60: duplicate.handler.ts does not expose credential.key
This handler invokesprisma.eventType.findUniquewith explicit includes and does not select anycredentialsfield, so nokeycan leak.packages/trpc/react/shared.ts (1)
19-19: LGTM: endpoint registered.Adding
eventTypes/heavykeeps client links in sync with the new router.apps/web/components/getting-started/steps-views/UserProfile.tsx (1)
40-40: LGTM: migrated to heavy.create.Hook targets the new heavy endpoint as intended.
packages/features/eventtypes/components/DuplicateDialog.tsx (1)
73-73: LGTM: migrated to heavy.duplicate.Matches the new heavy router path.
packages/platform/atoms/event-types/wrappers/EventTypeWebWrapper.tsx (1)
134-134: Switch to heavy.update hook looks correctClient now targets viewer.eventTypes.heavy.update; aligns with new router.
apps/web/modules/event-types/views/event-types-listing-view.tsx (1)
271-306: Heavy update mutation path LGTMUsing viewer.eventTypes.heavy.update for hide/show is consistent with the new router.
packages/trpc/server/routers/viewer/eventTypes/heavy/update.schema.ts (1)
1-2: Re-exports are correctUpdate schema/types surfaced from ../types under heavy; matches router usage.
apps/web/pages/api/trpc/eventTypes/heavy/[trpc].ts (1)
1-4: API route wiring LGTMHandler correctly mounts the heavy eventTypes router.
packages/trpc/server/routers/viewer/eventTypes/heavy/create.handler.ts (1)
12-12: Import path update is fineType-only import now points to ../../../../../types; no runtime impact.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/trpc/server/routers/viewer/_router.tsx (1)
27-27: Nit: Avoid aliasing a similarly named export.
If possible, rename the export in eventTypes/heavy/_router.ts to heavyEventTypesRouter and import it directly to reduce future confusion.Example in heavy/_router.ts:
// before export const eventTypesRouter = router({ /* heavy mutations */ }); // after export const heavyEventTypesRouter = router({ /* heavy mutations */ });Then here:
-import { eventTypesRouter as heavyEventTypesRouter } from "./eventTypes/heavy/_router"; +import { heavyEventTypesRouter } from "./eventTypes/heavy/_router";
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/features/eventtypes/lib/types.ts(1 hunks)packages/trpc/server/routers/viewer/_router.tsx(3 hunks)packages/trpc/server/routers/viewer/eventTypes/heavy/create.handler.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/trpc/server/routers/viewer/eventTypes/heavy/create.handler.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.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/eventtypes/lib/types.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/eventtypes/lib/types.tspackages/trpc/server/routers/viewer/_router.tsx
**/*.{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/eventtypes/lib/types.tspackages/trpc/server/routers/viewer/_router.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/trpc/server/routers/viewer/_router.tsx
🧬 Code graph analysis (2)
packages/features/eventtypes/lib/types.ts (1)
packages/trpc/react/trpc.ts (1)
RouterInputs(142-142)
packages/trpc/server/routers/viewer/_router.tsx (1)
packages/trpc/server/trpc.ts (2)
mergeRouters(14-14)router(13-13)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Production builds / Build Web App
- GitHub Check: Production builds / Build Docs
- GitHub Check: Production builds / Build Atoms
- GitHub Check: Tests / Unit
- GitHub Check: Production builds / Build API v2
- GitHub Check: Linters / lint
- GitHub Check: Production builds / Build API v1
- GitHub Check: Type check / check-types
🔇 Additional comments (4)
packages/features/eventtypes/lib/types.ts (1)
181-181: EventTypeUpdateInput correctly references the heavy path; no stale legacy imports found. Optional: add the deprecated alias to support any remaining straggler usages.packages/trpc/server/routers/viewer/_router.tsx (3)
7-7: Importing mergeRouters is appropriate here.
57-62: Composition LGTM — this cleanly namespaces heavy under viewer.eventTypes.heavy.
57-62: Confirm no client calls to non-heavy mutations remain.
Run a case-sensitive search across *.ts and *.tsx fortrpc.viewer.eventTypes.(create|update|duplicate)to ensure all create/update/duplicate calls are scoped under.heavy.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/trpc/server/routers/viewer/_router.tsx (1)
27-27: Avoid aliasing to reduce cognitive load.If feasible, export the heavy router with a distinct name from its module to drop the alias here.
Apply in this file:
-import { eventTypesRouter as heavyEventTypesRouter } from "./eventTypes/heavy/_router"; +import { heavyEventTypesRouter } from "./eventTypes/heavy/_router";And in packages/trpc/server/routers/viewer/eventTypes/heavy/_router.ts:
// rename the exported symbol for clarity export const heavyEventTypesRouter = router({ /* ... */ });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/trpc/server/routers/viewer/_router.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/trpc/server/routers/viewer/_router.tsx
**/*.{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/trpc/server/routers/viewer/_router.tsx
**/*.{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/trpc/server/routers/viewer/_router.tsx
🧬 Code graph analysis (1)
packages/trpc/server/routers/viewer/_router.tsx (1)
packages/trpc/server/trpc.ts (2)
mergeRouters(14-14)router(13-13)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Production builds / Build API v2
- GitHub Check: Production builds / Build Web App
- GitHub Check: Production builds / Build Atoms
- GitHub Check: Tests / Unit
- GitHub Check: Production builds / Build API v1
- GitHub Check: Linters / lint
- GitHub Check: Type check / check-types
🔇 Additional comments (2)
packages/trpc/server/routers/viewer/_router.tsx (2)
5-5: mergeRouters import looks correct.Good addition; aligns with the re-export in ../../trpc and enables composition below.
57-62: Verify heavy router isolation vs merge
- Merging heavyEventTypesRouter into the main viewer router will include all “heavy” handlers in your primary TRPC bundle.
- No standalone
/api/trpc/eventTypes/heavy/[trpc].tsroute exists, so endpoints aren’t duplicated but also aren’t isolated.- If you intend to serve heavy handlers separately, remove this merge and add a dedicated API route (or implement lazy composition), and confirm no key collisions with eventTypesRouter.
…:calcom/cal.com into perf/create-heavy-event-type-trpc-router
E2E results are ready! |
hbjORbj
left a comment
There was a problem hiding this comment.
Looks great! Just pushed a small fix for e2e tests, should pass
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (9)
apps/web/playwright/lib/testUtils.ts (1)
479-482: Centralize heavy.update endpoint constant and replace hardcoded usages
In apps/web/playwright/lib/testUtils.ts, add
export const HEAVY_EVENT_TYPES_UPDATE_ENDPOINT = "/api/trpc/eventTypes/heavy/update?batch=1";and update the call at line 479 to
- await submitAndWaitForResponse(page, "/api/trpc/eventTypes/heavy/update?batch=1", { + await submitAndWaitForResponse(page, HEAVY_EVENT_TYPES_UPDATE_ENDPOINT, { action: () => page.locator("[data-testid=update-eventtype]").click(), });Replace all other hardcoded
"/api/trpc/eventTypes/heavy/update?batch=1"occurrences (≈17 locations across apps/web/playwright/**/*.ts) with the importedHEAVY_EVENT_TYPES_UPDATE_ENDPOINT.apps/web/playwright/fixtures/regularBookings.ts (1)
53-56: Reuse a single helper instead of duplicating the heavy.update wait logic.You can lean on testUtils.saveEventType() for consistency and built-in status assertion.
- await submitAndWaitForResponse(page, "/api/trpc/eventTypes/heavy/update?batch=1", { - action: () => page.locator("[data-testid=update-eventtype]").click(), - }); + await saveEventType(page);Additionally, update the import:
// replace: import { submitAndWaitForResponse } from "../lib/testUtils"; // with: import { saveEventType } from "../lib/testUtils";apps/web/playwright/managed-event-types.e2e.ts (1)
241-243: Prefer the shared saveEventType helper over a file-local clone.This local saveAndWaitForResponse duplicates testUtils.saveEventType. Reusing the shared helper reduces drift and improves status checking consistency.
Implement by:
- Importing saveEventType from "./lib/testUtils"
- Replacing calls to saveAndWaitForResponse(...) with saveEventType(...)
- Removing the local helper
apps/web/playwright/dynamic-booking-pages.e2e.ts (1)
114-117: Use the submitAndWaitForResponse helper for robustness and status assertion.This ensures we assert 200 and avoids brittle exact-URL matching.
- await page.locator('[data-testid="update-eventtype"]').click(); - await page.waitForResponse("/api/trpc/eventTypes/heavy/update?batch=1"); + await submitAndWaitForResponse(page, "/api/trpc/eventTypes/heavy/update?batch=1", { + action: () => page.locator("[data-testid=update-eventtype]").click(), + });Add import at top:
import { submitAndWaitForResponse } from "./lib/testUtils";apps/web/playwright/organization/organization-invitation.e2e.ts (1)
460-462: Align with helper to assert status and reduce flakiness.Replace direct waitForResponse with submitAndWaitForResponse to assert 200 and avoid exact-match pitfalls.
- await page.locator('[data-testid="update-eventtype"]').click(); - await page.waitForResponse("/api/trpc/eventTypes/heavy/update?batch=1"); + await submitAndWaitForResponse(page, "/api/trpc/eventTypes/heavy/update?batch=1", { + action: () => page.locator("[data-testid=update-eventtype]").click(), + });Also extend the existing import from "../lib/testUtils":
import { bookTeamEvent, doOnOrgDomain, expectPageToBeNotFound, getInviteLink, submitAndWaitForResponse } from "../lib/testUtils";apps/web/playwright/manage-booking-questions.e2e.ts (1)
680-683: Deduplicate: import saveEventType from testUtils instead of redefining it.There’s already a shared saveEventType helper; reusing it avoids duplication.
Option A (preferred): remove this function and add:
import { saveEventType } from "./lib/testUtils";Option B: if you keep a local helper, at least reference a shared endpoint constant (e.g., HEAVY_EVENT_TYPES_UPDATE_ENDPOINT) to avoid hardcoding the URL in multiple places.
apps/web/playwright/hash-my-url.e2e.ts (1)
45-47: Heavy endpoint migration is correct; consider deduping the URL.All four calls correctly point to /api/trpc/eventTypes/heavy/update. To reduce repetition and ease future changes, use a shared constant.
Apply within these ranges:
- await submitAndWaitForResponse(page, "/api/trpc/eventTypes/heavy/update?batch=1", { + await submitAndWaitForResponse(page, HEAVY_UPDATE_ENDPOINT, { action: () => page.locator("[data-testid=update-eventtype]").click(), });Add near the top (after imports):
const HEAVY_UPDATE_ENDPOINT = "/api/trpc/eventTypes/heavy/update?batch=1";Also applies to: 77-79, 116-118, 167-169
apps/web/playwright/event-types.e2e.ts (2)
134-138: Use existing submitAndWaitForResponse helper for consistency.Replace the manual wait/assert with the shared helper to keep style consistent and reduce boilerplate.
- const submitPromise = page.waitForResponse("/api/trpc/eventTypes/heavy/duplicate?batch=1"); - await page.getByTestId("continue").click(); - const response = await submitPromise; - expect(response.status()).toBe(200); + await submitAndWaitForResponse(page, HEAVY_DUPLICATE_ENDPOINT, { + action: () => page.getByTestId("continue").click(), + });Add near the top (after imports):
const HEAVY_DUPLICATE_ENDPOINT = "/api/trpc/eventTypes/heavy/duplicate?batch=1";
147-149: Heavy update endpoint looks right; centralize the URL string.The moves to /eventTypes/heavy/update are correct. Suggest a shared constant to avoid scattering the literal across the file.
Apply within these ranges:
- await submitAndWaitForResponse(page, "/api/trpc/eventTypes/heavy/update?batch=1", { + await submitAndWaitForResponse(page, HEAVY_UPDATE_ENDPOINT, { action: () => page.locator("[data-testid=update-eventtype]").click(), });Add near the top (after imports):
const HEAVY_UPDATE_ENDPOINT = "/api/trpc/eventTypes/heavy/update?batch=1";Also applies to: 170-172, 317-319, 432-434
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
apps/web/playwright/dynamic-booking-pages.e2e.ts(1 hunks)apps/web/playwright/event-types.e2e.ts(5 hunks)apps/web/playwright/fixtures/regularBookings.ts(1 hunks)apps/web/playwright/hash-my-url.e2e.ts(4 hunks)apps/web/playwright/lib/testUtils.ts(1 hunks)apps/web/playwright/manage-booking-questions.e2e.ts(1 hunks)apps/web/playwright/managed-event-types.e2e.ts(1 hunks)apps/web/playwright/organization/booking.e2e.ts(3 hunks)apps/web/playwright/organization/organization-invitation.e2e.ts(1 hunks)apps/web/playwright/payment-apps.e2e.ts(1 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:
apps/web/playwright/managed-event-types.e2e.tsapps/web/playwright/dynamic-booking-pages.e2e.tsapps/web/playwright/lib/testUtils.tsapps/web/playwright/payment-apps.e2e.tsapps/web/playwright/manage-booking-questions.e2e.tsapps/web/playwright/fixtures/regularBookings.tsapps/web/playwright/hash-my-url.e2e.tsapps/web/playwright/organization/organization-invitation.e2e.tsapps/web/playwright/event-types.e2e.tsapps/web/playwright/organization/booking.e2e.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:
apps/web/playwright/managed-event-types.e2e.tsapps/web/playwright/dynamic-booking-pages.e2e.tsapps/web/playwright/lib/testUtils.tsapps/web/playwright/payment-apps.e2e.tsapps/web/playwright/manage-booking-questions.e2e.tsapps/web/playwright/fixtures/regularBookings.tsapps/web/playwright/hash-my-url.e2e.tsapps/web/playwright/organization/organization-invitation.e2e.tsapps/web/playwright/event-types.e2e.tsapps/web/playwright/organization/booking.e2e.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:
apps/web/playwright/managed-event-types.e2e.tsapps/web/playwright/dynamic-booking-pages.e2e.tsapps/web/playwright/lib/testUtils.tsapps/web/playwright/payment-apps.e2e.tsapps/web/playwright/manage-booking-questions.e2e.tsapps/web/playwright/fixtures/regularBookings.tsapps/web/playwright/hash-my-url.e2e.tsapps/web/playwright/organization/organization-invitation.e2e.tsapps/web/playwright/event-types.e2e.tsapps/web/playwright/organization/booking.e2e.ts
🧠 Learnings (2)
📚 Learning: 2025-08-27T13:32:46.887Z
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.
Applied to files:
apps/web/playwright/dynamic-booking-pages.e2e.tsapps/web/playwright/payment-apps.e2e.tsapps/web/playwright/manage-booking-questions.e2e.tsapps/web/playwright/fixtures/regularBookings.tsapps/web/playwright/organization/organization-invitation.e2e.tsapps/web/playwright/event-types.e2e.tsapps/web/playwright/organization/booking.e2e.ts
📚 Learning: 2025-08-21T13:44:06.805Z
Learnt from: supalarry
PR: calcom/cal.com#23217
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/services/output-event-types.service.ts:93-94
Timestamp: 2025-08-21T13:44:06.805Z
Learning: In apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.ts, repository functions that use explicit Prisma select clauses (like getEventTypeWithSeats) are used for specific purposes and don't need to include all EventType fields like bookingRequiresAuthentication. These methods don't feed into the general OutputEventTypesService_2024_06_14 flow.
Applied to files:
apps/web/playwright/dynamic-booking-pages.e2e.tsapps/web/playwright/lib/testUtils.tsapps/web/playwright/manage-booking-questions.e2e.tsapps/web/playwright/fixtures/regularBookings.ts
🧬 Code graph analysis (7)
apps/web/playwright/managed-event-types.e2e.ts (1)
apps/web/playwright/lib/testUtils.ts (1)
submitAndWaitForResponse(488-497)
apps/web/playwright/payment-apps.e2e.ts (1)
apps/web/playwright/lib/testUtils.ts (1)
submitAndWaitForResponse(488-497)
apps/web/playwright/manage-booking-questions.e2e.ts (1)
apps/web/playwright/lib/testUtils.ts (1)
submitAndWaitForResponse(488-497)
apps/web/playwright/fixtures/regularBookings.ts (1)
apps/web/playwright/lib/testUtils.ts (1)
submitAndWaitForResponse(488-497)
apps/web/playwright/hash-my-url.e2e.ts (1)
apps/web/playwright/lib/testUtils.ts (1)
submitAndWaitForResponse(488-497)
apps/web/playwright/event-types.e2e.ts (1)
apps/web/playwright/lib/testUtils.ts (1)
submitAndWaitForResponse(488-497)
apps/web/playwright/organization/booking.e2e.ts (1)
apps/web/playwright/lib/testUtils.ts (1)
submitAndWaitForResponse(488-497)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (4)
apps/web/playwright/lib/testUtils.ts (1)
479-482: Switch to heavy.update endpoint looks correct.This aligns tests with the new heavy router.
apps/web/playwright/organization/booking.e2e.ts (1)
410-411: LGTM on switching to heavy.update with a predicate.Using includes()+status check is resilient to querystring variations and asserts success.
apps/web/playwright/payment-apps.e2e.ts (1)
38-41: Good move to submitAndWaitForResponse with heavy.update.This makes the test more reliable by asserting 200 and using the new router.
apps/web/playwright/event-types.e2e.ts (1)
147-149: All Playwright tests reference heavy endpoints
Search inapps/web/playwrightfound no non-heavy/api/trpc/eventTypes/{update|duplicate}?batch=1calls.
…e separation - Remove all merged router files that used mergeRouters pattern - Update viewer router to directly import queries and mutations sub-routers - Achieve true separation by eliminating intermediate merged router layer - Follow PR #23664 guidance for proper router structure Co-Authored-By: keith@cal.com <keithwillcode@gmail.com>
Performance: Create 'heavy' eventTypes router
This PR introduces a performance optimization by creating a dedicated "heavy" eventTypes router that separates resource-intensive operations from lighter, more frequently used operations.
🎯 Problem
The main eventTypes router was handling both lightweight operations (like listing event types) and heavy operations (create, update, duplicate) in the same endpoint. This could impact performance for common read operations due to the overhead of loading heavy mutation handlers.
🚀 Solution
Created a separate
heavysub-router that isolates the most resource-intensive eventTypes operations:Decided to break #23555 into smaller chunks and to create "heavy" sub-routers right next to the existing routers instead of moving a ton of code to 1 quarantine router.
Reduces compilation of
/event-typesby around 40% locally, down from 7.5s to 5.5s.🏗️ Architecture Changes
New Router Structure
New API Endpoint
/api/trpc/eventTypes/heavy/[trpc].ts- Dedicated endpoint for heavy operationsviewer.eventTypes.heavy.create)Client-Side Updates
Updated all consumers to use the new heavy router endpoints:
📈 Performance Benefits
🔄 Migration Impact
📊 Changes Summary
/api/trpc/eventTypes/heavy/[trpc].tsheavy/subdirectoryThis architectural improvement provides a solid foundation for future performance optimizations while maintaining the existing developer experience.