perf: implement lightweight tRPC userEventGroupsRouter to improve initial page load.#23163
perf: implement lightweight tRPC userEventGroupsRouter to improve initial page load.#23163zhyd1997 wants to merge 1 commit intocalcom:mainfrom
userEventGroupsRouter to improve initial page load.#23163Conversation
…nitial page load.
Walkthrough
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@zhyd1997 is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
dosubot
bot
added
the
performance
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (08/18/25)1 reviewer was added to this PR based on Keith Williams's automation. "Add community label" took an action on this PR • (08/18/25)1 label was added to this PR based on Keith Williams's automation. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
apps/web/app/(use-page-wrapper)/(main-nav)/event-types/page.tsx (2)
26-44: Critical: unstable_cache key is not scoped per user or filters (risk of cross-user data leakage).
unstable_cacheonly keys on["viewer.eventTypes.userEventGroups.get"], so results can be shared across users and filter sets. Scope the cache by userId (and filters) or avoid caching user-specific data.Apply this refactor to build a per-user, per-filter cache key:
-const getCachedEventGroups = unstable_cache( - async ( - headers: ReadonlyHeaders, - cookies: ReadonlyRequestCookies, - filters?: { - teamIds?: number[] | undefined; - userIds?: number[] | undefined; - upIds?: string[] | undefined; - } - ) => { - const userEventGroupsCaller = await createRouterCaller( - userEventGroupsRouter, - await getTRPCContext(headers, cookies) - ); - return await userEventGroupsCaller.get({ filters }); - }, - ["viewer.eventTypes.userEventGroups.get"], - { revalidate: 3600 } // seconds -); +const getCachedEventGroups = async ( + headers: ReadonlyHeaders, + cookies: ReadonlyRequestCookies, + userId: string, + filters?: { + teamIds?: number[] | undefined; + userIds?: number[] | undefined; + upIds?: string[] | undefined; + } +) => { + const filtersKey = JSON.stringify(filters ?? {}); + const load = unstable_cache( + async () => { + const userEventGroupsCaller = await createRouterCaller( + userEventGroupsRouter, + await getTRPCContext(headers, cookies) + ); + return await userEventGroupsCaller.get({ filters }); + }, + ["viewer.eventTypes.userEventGroups.get", userId, filtersKey], + { revalidate: 3600 } // seconds + ); + return load(); +};Follow-up change to the call site is in a separate comment below.
56-59: Pass userId to the cached getter to scope the cache.Update the call to include
session.user.idto match the new signature and avoid cross-user cache collisions.- const userEventGroupsData = await getCachedEventGroups(_headers, _cookies, filters); + const userEventGroupsData = await getCachedEventGroups(_headers, _cookies, session.user.id, filters);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/web/app/(use-page-wrapper)/(main-nav)/event-types/page.tsx(2 hunks)packages/trpc/server/routers/viewer/eventTypes/userEventGroups/_router.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:
packages/trpc/server/routers/viewer/eventTypes/userEventGroups/_router.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/userEventGroups/_router.tsapps/web/app/(use-page-wrapper)/(main-nav)/event-types/page.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:
apps/web/app/(use-page-wrapper)/(main-nav)/event-types/page.tsx
🧬 Code Graph Analysis (2)
packages/trpc/server/routers/viewer/eventTypes/userEventGroups/_router.ts (1)
packages/trpc/server/trpc.ts (1)
router(13-13)
apps/web/app/(use-page-wrapper)/(main-nav)/event-types/page.tsx (2)
apps/web/app/_trpc/context.ts (2)
createRouterCaller(18-22)getTRPCContext(13-16)packages/trpc/server/routers/viewer/eventTypes/userEventGroups/_router.ts (1)
userEventGroupsRouter(7-22)
⏰ 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)
- GitHub Check: Detect changes
🔇 Additional comments (5)
packages/trpc/server/routers/viewer/eventTypes/userEventGroups/_router.ts (3)
11-11: Confirm policy: avoid logging PII (user IDs) in perf logs if restricted.Logging
ctx.user.idmay be disallowed depending on privacy policy. Consider anonymizing (hashing) or removing the ID from the label if needed.Would you like me to switch the label to omit the userId or to a hashed form?
8-9: Good call on authedProcedure + lazy import.Auth gate with schema validation and lazy-loading the handler meets the perf goal and keeps the router lean.
7-21: No exposedcredential.keyin getUserEventGroups handlerI’ve reviewed the handler and its underlying repository calls:
• getUserEventGroups.handler.ts – no
select/includeforcredentialorkey
• ProfileRepository.findByUpId – no credential fields loaded
• MembershipRepository.findAllByUpIdIncludeTeam – no credential fields loadedThe endpoint returns only event-group and profile metadata (slugs, names, images, URLs, etc.) and does not surface any
credentialdata. No further action required.apps/web/app/(use-page-wrapper)/(main-nav)/event-types/page.tsx (2)
11-11: Import switch to the lightweight router looks good.Using the new
userEventGroupsRouteraligns with the perf objective.
40-41: Call surface update LGTM.Renaming to
.get({ filters })matches the new router API and keeps types consistent.
| return await userEventGroupsCaller.get({ filters }); | ||
| }, | ||
| ["viewer.eventTypes.getUserEventGroups"], | ||
| ["viewer.eventTypes.userEventGroups.get"], |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Cache key string updated — ensure all old keys are removed from clients.
The key changed to viewer.eventTypes.userEventGroups.get. Verify no code still references the old key for revalidation or tagging.
Run this to find stale references:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for old route key usages..."
rg -nP -C2 --type=ts --type=tsx '\bviewer\.eventTypes\.getUserEventGroups\b|\["viewer\.eventTypes\.getUserEventGroups"\]'Length of output: 256
To ensure we catch references in both .ts and .tsx files without relying on unsupported --type, let’s retry with glob filters:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for old route key usages in .ts and .tsx files..."
rg -nP -C2 '\bviewer\.eventTypes\.getUserEventGroups\b|\["viewer\.eventTypes\.getUserEventGroups"\]' -g '*.ts' -g '*.tsx'Length of output: 2432
Stale cache key references detected — old key viewer.eventTypes.getUserEventGroups still in use
You’ve updated the cache key to "viewer.eventTypes.userEventGroups.get", but the old key remains in several invalidate calls. Please update these to use the new key (utils.viewer.eventTypes.userEventGroups.get.invalidate()) or the corresponding revalidate/tag API:
• packages/features/eventtypes/components/DuplicateDialog.tsx:77
• packages/features/ee/teams/pages/team-profile-view.tsx:130, 282
• packages/features/ee/teams/components/CreateANewTeamForm.tsx:45
🤖 Prompt for AI Agents
In apps/web/app/(use-page-wrapper)/(main-nav)/event-types/page.tsx around line
42, stale cache key references remain elsewhere: change all uses of the old key
viewer.eventTypes.getUserEventGroups to the new key path
utils.viewer.eventTypes.userEventGroups.get (e.g. call
utils.viewer.eventTypes.userEventGroups.get.invalidate() or use the
corresponding revalidate/tag API). Specifically update the invalidate/revalidate
calls in packages/features/eventtypes/components/DuplicateDialog.tsx (line ~77),
packages/features/ee/teams/pages/team-profile-view.tsx (lines ~130 and ~282),
and packages/features/ee/teams/components/CreateANewTeamForm.tsx (line ~45) to
use utils.viewer.eventTypes.userEventGroups.get.* instead of the old
viewer.eventTypes.getUserEventGroups.*; search the repo for
viewer.eventTypes.getUserEventGroups and replace all occurrences with the new
API to ensure cache invalidation works consistently.
| get: authedProcedure.input(ZEventTypeInputSchema).query(async ({ ctx, input }) => { | ||
| const { getUserEventGroups } = await import("../getUserEventGroups.handler"); | ||
|
|
||
| const timer = logP(`getUserEventGroups(${ctx.user.id})`); | ||
|
|
||
| const result = await getUserEventGroups({ | ||
| ctx, | ||
| input, | ||
| }); | ||
|
|
||
| timer(); | ||
|
|
||
| return result; | ||
| }), |
There was a problem hiding this comment.
Always stop the perf timer even on errors (use try/finally).
If getUserEventGroups throws, timer() is never called, skewing perf telemetry.
Apply this diff to ensure the timer is stopped in all cases:
- const timer = logP(`getUserEventGroups(${ctx.user.id})`);
-
- const result = await getUserEventGroups({
- ctx,
- input,
- });
-
- timer();
-
- return result;
+ const stop = logP(`viewer.eventTypes.userEventGroups.get uid=${ctx.user.id}`);
+ try {
+ return await getUserEventGroups({
+ ctx,
+ input,
+ });
+ } finally {
+ stop();
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| get: authedProcedure.input(ZEventTypeInputSchema).query(async ({ ctx, input }) => { | |
| const { getUserEventGroups } = await import("../getUserEventGroups.handler"); | |
| const timer = logP(`getUserEventGroups(${ctx.user.id})`); | |
| const result = await getUserEventGroups({ | |
| ctx, | |
| input, | |
| }); | |
| timer(); | |
| return result; | |
| }), | |
| get: authedProcedure.input(ZEventTypeInputSchema).query(async ({ ctx, input }) => { | |
| const { getUserEventGroups } = await import("../getUserEventGroups.handler"); | |
| const stop = logP(`viewer.eventTypes.userEventGroups.get uid=${ctx.user.id}`); | |
| try { | |
| return await getUserEventGroups({ | |
| ctx, | |
| input, | |
| }); | |
| } finally { | |
| stop(); | |
| } | |
| }), |
🤖 Prompt for AI Agents
In packages/trpc/server/routers/viewer/eventTypes/userEventGroups/_router.ts
around lines 8 to 21, the performance timer started by logP is only stopped when
getUserEventGroups succeeds—if it throws the timer() call is skipped; wrap the
call to getUserEventGroups and the return in a try/finally and call timer() in
the finally block so the timer is always invoked regardless of success or error.
|
Mark as draft for review |
keithwillcode
left a comment
keithwillcode
left a comment
There was a problem hiding this comment.
Appreciate the PR, @zhyd1997! Couple of thoughts:
Wondering if by doing this you were able to find the ultimate culprit in the eventTypes router that was causing the major slowdown? i.e. Were you able to spot a particular handler that loaded tons of dependencies? Or did you determine that it's just having 15 handlers and their dependencies adds up?
Reason I ask is because I'm not sure the best path forward is to just split every handler out into its own router because that will create loads of files and repo overhead. I'm hoping to find a way where we can still individually load the handlers and their dependencies only when used. Unfortunately, the Next.js team has confirmed with us that dynamic imports are still compiled (as we see in the .next/server/chunks folder) even when a page is not using those handlers.
Another thing is that if we are going this route for a select few handlers as you've done, you have to add an API route to be able to actually use the tRPC handler. e.g. apps/web/pages/api/trpc/eventTypes/[trpc].ts.
|
I still haven’t found the exact root cause, but I suspect the performance bottleneck is related to calling tRPC. When I commented out these lines and replaced them with real data from the tRPC API, the compile speed was significantly faster: ○ Compiling /apps/installed/[category] ...
✓ Compiled /apps/installed/[category] in 1760ms |
|
Update: Please be cautious when using
in the codebase. |
|
Closing this PR in favor of #23198 |
2 participants
What does this PR do?
Visual Demo (For contributors especially)
A visual demonstration is strongly recommended, for both the original and new change (video / image - any one).
Video Demo (if applicable):
Image Demo (if applicable):
Before:
After:
🟢 ~4.7× faster, reducing cold-start latency by ~79%
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Checklist