perf: optimize bookings and event-types pages with client-side components#23612
perf: optimize bookings and event-types pages with client-side components#23612BenraouaneSoufiane wants to merge 44 commits intocalcom:mainfrom
Conversation
…ents - Move BookingsList and EventTypes to client-side with dynamic imports - Add loading skeletons for better UX during chunk loading - Maintain server-side data fetching while moving UI to client - Fix TypeScript prop type compatibility and linting issues - Resolve ESLint and Prettier violations in client components
|
@BenraouaneSoufiane is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughMultiple app pages switched from static imports to Next.js dynamic() imports (availability, bookings, event-types, teams, and the server teams page), adding loading fallbacks for some components or removing no-op dynamic calls. The root app page replaced a server-side auth flow with a lightweight cookie-based checkSession used for redirect logic. apps/web/next.config.js now detects dev mode, enables swcMinify, makes productionBrowserSourceMaps conditional, extends serverExternalPackages, adjusts caching and webpack dev options, moves avatar rewrite to rewrites/afterFiles and adds per-route headers, and tweaks redirects and env logic. A single test file had a formatting-only blank-line change. Public APIs and page signatures remain unchanged. Possibly related PRs
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 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 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
✨ 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 Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
|
|
Hey there and thank you for opening this pull request! 👋🏼 We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted. Details: |
dosubot
bot
added
the
🧹 Improvements
BenraouaneSoufiane
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/app/(use-page-wrapper)/(main-nav)/event-types/page.tsx (2)
28-46: Do not globally cache viewer-specific data without per-user/filters keying.
unstable_cachehere uses a static key and may serve another user’s cached result or wrong filtered data. IncludeuserIdandfiltersin the cache key (or avoidunstable_cachefor viewer data).Apply:
-const getCachedEventGroups = unstable_cache( - async ( - headers: ReadonlyHeaders, - cookies: ReadonlyRequestCookies, - filters?: { - teamIds?: number[] | undefined; - userIds?: number[] | undefined; - upIds?: string[] | undefined; - } - ) => { - const eventTypesCaller = await createRouterCaller( - eventTypesRouter, - await getTRPCContext(headers, cookies) - ); - return await eventTypesCaller.getUserEventGroups({ filters }); - }, - ["viewer.eventTypes.getUserEventGroups"], - { revalidate: 3600 } // seconds -); +const getCachedEventGroupsFor = ( + userId: string | number, + filters?: { + teamIds?: number[] | undefined; + userIds?: number[] | undefined; + upIds?: string[] | undefined; + } +) => + unstable_cache( + async (headers: ReadonlyHeaders, cookies: ReadonlyRequestCookies) => { + const eventTypesCaller = await createRouterCaller( + eventTypesRouter, + await getTRPCContext(headers, cookies) + ); + return eventTypesCaller.getUserEventGroups({ filters }); + }, + ["viewer.eventTypes.getUserEventGroups", `u:${userId}`, JSON.stringify(filters ?? {})], + { revalidate: 3600, tags: ["viewer.eventTypes.getUserEventGroups", `user:${userId}`] } + );And update the call site (see below).
60-61: Update call site for the new per-user cache.Use the factory to bind
userIdandfilters, then passheaders/cookies.Apply:
- const userEventGroupsData = await getCachedEventGroups(_headers, _cookies, filters); + const userEventGroupsData = await getCachedEventGroupsFor(session.user.id, filters)(_headers, _cookies);
🧹 Nitpick comments (6)
apps/web/next.config.js (1)
260-269: Make cacheDirectory cross-platform.Hardcoding /tmp breaks on Windows. Use os.tmpdir().
Apply this diff:
- cacheDirectory: "/tmp/.next-cache", // Use tmp directory for better performance + cacheDirectory: require("path").join(require("os").tmpdir(), "next-cache"),If preferred, hoist imports once:
const os = require("os"); const path = require("path");and use path.join(os.tmpdir(), "next-cache").
apps/web/app/(use-page-wrapper)/(main-nav)/bookings/[status]/page.tsx (1)
21-28: Fix generateMetadata param types (no Promise).Next passes params as a plain object. Current typing works at runtime but is inaccurate.
Apply this diff:
-export const generateMetadata = async ({ params }: { params: Promise<{ status: string }> }) => +export const generateMetadata = async ({ params }: { params: { status: string } }) => await _generateMetadata( (t) => t("bookings"), (t) => t("bookings_description"), undefined, undefined, - `/bookings/${(await params).status}` + `/bookings/${params.status}` );apps/web/app/(use-page-wrapper)/(main-nav)/bookings/[status]/BookingsClient.tsx (1)
13-26: Avoid double fallbacks (dynamic.loading + Suspense).You’re providing both dynamic’s loading and a Suspense fallback. Pick one to prevent redundant UI and potential flicker. Easiest: keep loading and drop Suspense.
Apply this diff:
return ( <div className="space-y-6"> - {/* Main bookings list */} - <Suspense - fallback={ - <div className="space-y-4"> - <div className="h-32 animate-pulse rounded bg-gray-200" /> - <div className="h-32 animate-pulse rounded bg-gray-200" /> - <div className="h-32 animate-pulse rounded bg-gray-200" /> - </div> - }> - <BookingsList status={status} userId={userId} /> - </Suspense> + {/* Main bookings list */} + <BookingsList status={status} userId={userId} /> </div> );Alternative: set suspense: true on dynamic and remove the loading option, then keep the Suspense wrapper.
Also applies to: 37-46
apps/web/app/(use-page-wrapper)/(main-nav)/event-types/EventTypesClient.tsx (2)
61-75: Remove redundant Suspense wrappers (dynamic already handles loading).You’re using
next/dynamicwithloadingandssr: false, but also wrapping in<Suspense>. Sincesuspense: trueisn’t enabled ondynamic, these Suspense fallbacks won’t render. Keep one mechanism.Option A (remove Suspense):
-import { Suspense } from "react"; @@ - <Suspense fallback={<div className="h-12 w-48 animate-pulse rounded bg-gray-200" />}> - <EventTypesCTA userEventGroupsData={userEventGroupsData} /> - </Suspense> - <Suspense - fallback={ - <div className="space-y-4"> - <div className="h-8 w-1/3 animate-pulse rounded bg-gray-200" /> - <div className="h-24 animate-pulse rounded bg-gray-200" /> - <div className="h-24 animate-pulse rounded bg-gray-200" /> - </div> - }> - <EventTypes userEventGroupsData={userEventGroupsData} user={user} /> - </Suspense> + <EventTypesCTA userEventGroupsData={userEventGroupsData} /> + <EventTypes userEventGroupsData={userEventGroupsData} user={user} />Option B (prefer Suspense): set
suspense: trueon both dynamic imports and remove theirloadingprops; keep the Suspense fallbacks.Also applies to: 4-4, 29-41, 43-49
33-40: Prefer design-system Skeleton over custom Tailwind placeholders.For consistency and theming, use
Skeletonfrom@calcom/ui(if available) instead of hand-rolled gray divs.Example:
- loading: () => ( - <div className="space-y-4"> - <div className="h-8 w-1/3 animate-pulse rounded bg-gray-200" /> - <div className="h-24 animate-pulse rounded bg-gray-200" /> - <div className="h-24 animate-pulse rounded bg-gray-200" /> - </div> - ), + loading: () => ( + <div className="space-y-4"> + <Skeleton className="h-8 w-1/3" /> + <Skeleton className="h-24" /> + <Skeleton className="h-24" /> + </div> + ),Also applies to: 47-48
apps/web/app/(use-page-wrapper)/(main-nav)/event-types/page.tsx (1)
66-67: Minor: omitCTAwhen null if prop is optional.If
ShellMainAppDirmarksCTA?: ReactNode, you can drop the prop entirely for brevity.- CTA={null} // We'll handle CTA in the client component - > + >
📜 Review details
Configuration used: Path: .coderabbit.yaml
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 (5)
apps/web/app/(use-page-wrapper)/(main-nav)/bookings/[status]/BookingsClient.tsx(1 hunks)apps/web/app/(use-page-wrapper)/(main-nav)/bookings/[status]/page.tsx(3 hunks)apps/web/app/(use-page-wrapper)/(main-nav)/event-types/EventTypesClient.tsx(1 hunks)apps/web/app/(use-page-wrapper)/(main-nav)/event-types/page.tsx(3 hunks)apps/web/next.config.js(7 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:
apps/web/app/(use-page-wrapper)/(main-nav)/bookings/[status]/BookingsClient.tsxapps/web/app/(use-page-wrapper)/(main-nav)/event-types/EventTypesClient.tsxapps/web/app/(use-page-wrapper)/(main-nav)/bookings/[status]/page.tsxapps/web/app/(use-page-wrapper)/(main-nav)/event-types/page.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:
apps/web/app/(use-page-wrapper)/(main-nav)/bookings/[status]/BookingsClient.tsxapps/web/app/(use-page-wrapper)/(main-nav)/event-types/EventTypesClient.tsxapps/web/app/(use-page-wrapper)/(main-nav)/bookings/[status]/page.tsxapps/web/app/(use-page-wrapper)/(main-nav)/event-types/page.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:
apps/web/app/(use-page-wrapper)/(main-nav)/bookings/[status]/BookingsClient.tsxapps/web/app/(use-page-wrapper)/(main-nav)/event-types/EventTypesClient.tsxapps/web/app/(use-page-wrapper)/(main-nav)/bookings/[status]/page.tsxapps/web/next.config.jsapps/web/app/(use-page-wrapper)/(main-nav)/event-types/page.tsx
🧠 Learnings (6)
📓 Common learnings
Learnt from: mdm317
PR: calcom/cal.com#23221
File: packages/features/schedules/components/NewScheduleButton.tsx:1-1
Timestamp: 2025-08-20T17:34:35.004Z
Learning: In the Cal.com codebase, server actions like `revalidateAvailabilityList()` from `app/(use-page-wrapper)/(main-nav)/availability/actions` are imported and called directly in client components within mutation `onSuccess` callbacks. This pattern is consistently used across availability-related components (both in availability-view.tsx for updates and NewScheduleButton.tsx for creates) to ensure the availability list cache is fresh after mutations.
Learnt from: mdm317
PR: calcom/cal.com#23221
File: packages/features/schedules/components/NewScheduleButton.tsx:1-1
Timestamp: 2025-08-20T17:34:35.004Z
Learning: In the Cal.com codebase, it's an established pattern for client components to directly import and call server actions like `revalidateAvailabilityList()` from `app/(use-page-wrapper)/(main-nav)/availability/actions` within mutation `onSuccess` callbacks. This pattern is consistently used across availability-view.tsx for delete, update, and bulk update operations, and should be followed for create operations as well to maintain consistency.
Learnt from: bandhan-majumder
PR: calcom/cal.com#23454
File: packages/features/bookings/Booker/components/EventMeta.tsx:16-16
Timestamp: 2025-08-29T22:57:31.407Z
Learning: In Cal.com's Booker architecture, components become client-side through BookerStoreProvider.tsx which has "use client". Any component using useBookerStoreContext automatically runs on the client and should use client-appropriate utilities like markdownToSafeHTMLClient, regardless of whether they have explicit "use client" directives.
Learnt from: bandhan-majumder
PR: calcom/cal.com#23454
File: packages/features/bookings/Booker/components/EventMeta.tsx:16-16
Timestamp: 2025-08-29T22:57:31.407Z
Learning: Components in Cal.com codebase may run on the client without explicit "use client" directive if they are imported by parent client components. The presence of browser APIs, hooks, or interactive features indicates client-side execution regardless of the directive.
Learnt from: bandhan-majumder
PR: calcom/cal.com#23454
File: packages/features/bookings/Booker/components/EventMeta.tsx:16-16
Timestamp: 2025-08-29T22:57:31.407Z
Learning: In Cal.com's Booker component hierarchy, the "use client" directive is placed at a higher level parent component, making all child components in packages/features/bookings/Booker/components/ run on the client side without needing individual "use client" directives. Components should still use client-appropriate utilities like markdownToSafeHTMLClient even without explicit "use client" directives.
📚 Learning: 2025-08-20T17:34:35.004Z
Learnt from: mdm317
PR: calcom/cal.com#23221
File: packages/features/schedules/components/NewScheduleButton.tsx:1-1
Timestamp: 2025-08-20T17:34:35.004Z
Learning: In the Cal.com codebase, server actions like `revalidateAvailabilityList()` from `app/(use-page-wrapper)/(main-nav)/availability/actions` are imported and called directly in client components within mutation `onSuccess` callbacks. This pattern is consistently used across availability-related components (both in availability-view.tsx for updates and NewScheduleButton.tsx for creates) to ensure the availability list cache is fresh after mutations.
Applied to files:
apps/web/app/(use-page-wrapper)/(main-nav)/bookings/[status]/BookingsClient.tsx
📚 Learning: 2025-08-29T22:57:31.407Z
Learnt from: bandhan-majumder
PR: calcom/cal.com#23454
File: packages/features/bookings/Booker/components/EventMeta.tsx:16-16
Timestamp: 2025-08-29T22:57:31.407Z
Learning: In Cal.com's Booker architecture, components become client-side through BookerStoreProvider.tsx which has "use client". Any component using useBookerStoreContext automatically runs on the client and should use client-appropriate utilities like markdownToSafeHTMLClient, regardless of whether they have explicit "use client" directives.
Applied to files:
apps/web/app/(use-page-wrapper)/(main-nav)/bookings/[status]/BookingsClient.tsx
📚 Learning: 2025-08-29T22:57:30.382Z
Learnt from: bandhan-majumder
PR: calcom/cal.com#23454
File: packages/features/form-builder/FormBuilder.tsx:11-11
Timestamp: 2025-08-29T22:57:30.382Z
Learning: FormBuilder.tsx in packages/features/form-builder/ does not have "use client" directive at the top despite using client-side React hooks and event handlers, which suggests it should be a client component.
Applied to files:
apps/web/app/(use-page-wrapper)/(main-nav)/bookings/[status]/BookingsClient.tsxapps/web/app/(use-page-wrapper)/(main-nav)/event-types/EventTypesClient.tsxapps/web/app/(use-page-wrapper)/(main-nav)/event-types/page.tsx
📚 Learning: 2025-08-29T22:57:31.407Z
Learnt from: bandhan-majumder
PR: calcom/cal.com#23454
File: packages/features/bookings/Booker/components/EventMeta.tsx:16-16
Timestamp: 2025-08-29T22:57:31.407Z
Learning: In Cal.com's Booker component hierarchy, the "use client" directive is placed at a higher level parent component, making all child components in packages/features/bookings/Booker/components/ run on the client side without needing individual "use client" directives. Components should still use client-appropriate utilities like markdownToSafeHTMLClient even without explicit "use client" directives.
Applied to files:
apps/web/app/(use-page-wrapper)/(main-nav)/bookings/[status]/BookingsClient.tsx
📚 Learning: 2025-08-04T13:14:39.218Z
Learnt from: hariombalhara
PR: calcom/cal.com#22840
File: packages/app-store/webex/api/add.ts:4-4
Timestamp: 2025-08-04T13:14:39.218Z
Learning: Cal.com integrations: `WEBAPP_URL_FOR_OAUTH` intentionally falls back to `http://localhost:3000` when neither production nor development, matching the pattern used by other apps in the repo.
Applied to files:
apps/web/next.config.js
🧬 Code graph analysis (3)
apps/web/app/(use-page-wrapper)/(main-nav)/event-types/EventTypesClient.tsx (1)
apps/web/modules/event-types/views/event-types-listing-view.tsx (1)
EventTypesCTA(952-968)
apps/web/app/(use-page-wrapper)/(main-nav)/bookings/[status]/page.tsx (2)
apps/web/app/(use-page-wrapper)/(main-nav)/ShellMainAppDir.tsx (1)
ShellMainAppDir(8-66)apps/web/app/(use-page-wrapper)/(main-nav)/bookings/[status]/BookingsClient.tsx (1)
BookingsClient(33-49)
apps/web/app/(use-page-wrapper)/(main-nav)/event-types/page.tsx (1)
apps/web/app/(use-page-wrapper)/(main-nav)/event-types/EventTypesClient.tsx (1)
EventTypesClient(58-76)
⏰ 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 (12)
apps/web/next.config.js (6)
177-178: Good: single source of truth for dev mode.Using isDev derived from NODE_ENV is fine here.
211-213: Source maps only in production: LGTM.productionBrowserSourceMaps tied to !isDev keeps dev snappy.
222-228: Quieter dev logging: LGTM.Reduces noise without affecting prod.
415-418: Rewrite moved to afterFiles: LGTM.Keeps avatar rewrite while preserving beforeFiles priorities.
636-636: Regex tweak: LGTM.The negative lookahead for callbackUrl now reads cleaner and remains correct.
215-216: CI typecheck confirmed. The CI workflow (.github/workflows/check-types.yml) already runsyarn type-check:ci, so retainingignoreBuildErrors: !!process.env.CIis safe.apps/web/app/(use-page-wrapper)/(main-nav)/bookings/[status]/page.tsx (3)
15-15: Client handoff via BookingsClient: LGTM.Named import keeps tree-shaking friendly.
38-40: Passing userId as string: LGTM.Avoids unnecessary coercion and matches client prop.
43-43: Client-rendered list with server-fetched context: LGTM.Props are minimal and typed; keeps SSR concerns on the page.
apps/web/app/(use-page-wrapper)/(main-nav)/bookings/[status]/BookingsClient.tsx (1)
6-11: Status typing via typeof tuple: LGTM.import type keeps value out of the client bundle.
apps/web/app/(use-page-wrapper)/(main-nav)/event-types/EventTypesClient.tsx (1)
28-49: LGTM: dynamic client-splitting aligns with the PR’s perf goals.Using
next/dynamicwithssr: falsefor heavy views is appropriate here and matches the pattern used elsewhere in this PR.apps/web/app/(use-page-wrapper)/(main-nav)/event-types/page.tsx (1)
28-46:unstable_cacheauto-includes function arguments; static keyParts usage is safe.
No changes required—the cache key is built from your function plus serialized args, and the provided keyParts is only for extra identifiers.
apps/web/app/(use-page-wrapper)/(main-nav)/event-types/EventTypesClient.tsx
Outdated
Show resolved
Hide resolved
apps/web/app/(use-page-wrapper)/(main-nav)/event-types/EventTypesClient.tsx
Outdated
Show resolved
Hide resolved
apps/web/app/(use-page-wrapper)/(main-nav)/event-types/page.tsx
Outdated
Show resolved
Hide resolved
- Unify userId prop types (string ↔ number) between BookingsClient and BookingsList - Correct userEventGroupsData shape (object vs. array) - Fix user.id type mismatch (string ↔ number) Resolves TS2322, TS2739, TS2740
BenraouaneSoufiane
force-pushed
the
fix-lazy-local-dev
branch
from
9bd36db to
5c9a0b8
Compare
|
@coderabbitai, can you provide commitable changes for your suggesions? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainI need to find the previous suggestions I made to provide committable changes. Let me search for any previous CodeRabbit review comments on this PR. 🏁 Script executed: Length of output: 140 Let me try a different approach to find the review comments, including both reviews and individual line comments. 🏁 Script executed: Length of output: 28619 @BenraouaneSoufiane Based on my previous review comments, here are the committable changes to address the identified issues: 1. Fix EventTypesClient.tsx - Prop name mismatch and typesFile: import dynamic from "next/dynamic";
import { Suspense } from "react";
// Use canonical types from the view to avoid drift
import type { Props as EventTypesViewProps } from "~/event-types/views/event-types-listing-view";
const EventTypes = dynamic(() => import("~/event-types/views/event-types-listing-view"), {
ssr: false,
loading: () => (
<div className="animate-pulse">
<div className="mb-4 h-8 w-48 rounded bg-gray-200"></div>
<div className="space-y-4">
{[...Array(3)].map((_, i) => (
<div key={i} className="h-20 rounded border bg-gray-50"></div>
))}
</div>
</div>
),
});
const EventTypesCTA = dynamic(
() => import("~/event-types/views/event-types-listing-view").then((mod) => ({ default: mod.EventTypesCTA })),
{
ssr: false,
loading: () => (
<div className="animate-pulse">
<div className="h-24 rounded border bg-gray-50"></div>
</div>
),
}
);
interface EventTypesClientProps extends Pick<EventTypesViewProps, "userEventGroupsData" | "user"> {
heading?: string;
subtitle?: string;
}
export function EventTypesClient({ userEventGroupsData, user, heading, subtitle }: EventTypesClientProps) {
return (
<div className="space-y-6">
<Suspense
fallback={
<div className="animate-pulse">
<div className="h-24 rounded border bg-gray-50"></div>
</div>
}>
<EventTypesCTA userEventGroupsData={userEventGroupsData} />
</Suspense>
<Suspense
fallback={
<div className="animate-pulse">
<div className="mb-4 h-8 w-48 rounded bg-gray-200"></div>
<div className="space-y-4">
{[...Array(3)].map((_, i) => (
<div key={i} className="h-20 rounded border bg-gray-50"></div>
))}
</div>
</div>
}>
<EventTypes userEventGroupsData={userEventGroupsData} user={user} />
</Suspense>
</div>
);
}2. Fix next.config.js - Multiple configuration issuesFile: Replace the relevant sections with these fixes: Remove duplicate @calcom/ui transform (around line 250-254): modularizeImports: {
// Other imports can stay, but remove the @calcom/ui entry since optimizePackageImports handles it
},Move swcMinify to top-level (around line 203-210): experimental: {
// Disable turbopack due to compatibility issues with Cal.com
// externalize server-side node_modules with size > 1mb, to improve dev mode performance/RAM usage
optimizePackageImports: ["@calcom/ui"],
webpackMemoryOptimizations: true,
webpackBuildWorker: true,
},
// Enable SWC minification for faster builds
swcMinify: true,Fix header path for avatar API (around line 518): {
source: "/api/user/avatar",
headers: [CORP_CROSS_ORIGIN_HEADER],
},
{
source: "/avatar.svg",
headers: [CORP_CROSS_ORIGIN_HEADER],
},
{
source: "/icons/sprite.svg(\\?v=[0-9a-zA-Z\\-\\.]+)?",
headers: [
CORP_CROSS_ORIGIN_HEADER,
ACCESS_CONTROL_ALLOW_ORIGIN_HEADER,
{
key: "Cache-Control",
value: "public, max-age=31536000, immutable",
},
],
},These changes address:
The changes will resolve the TypeScript errors, optimize the Next.js configuration, and ensure proper CORS headers are applied to the avatar endpoints. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
apps/web/app/(use-page-wrapper)/(main-nav)/bookings/[status]/BookingsClient.tsx (2)
6-11: Derive props from the canonical view to avoid drift (and fix potential userId type mismatch).Bind this client’s props to the source view’s props so status/userId stay in sync with the actual component. This also resolves any string vs number drift for userId.
Apply:
+import type { ComponentProps } from "react"; -// Import the valid status types - use import type for type-only imports -import type { validStatuses } from "~/bookings/lib/validStatuses"; +import type BookingsListingView from "~/bookings/views/bookings-listing-view"; -// Create a type for the status -type BookingStatus = (typeof validStatuses)[number]; +type BookingsListProps = ComponentProps<typeof BookingsListingView>; @@ -interface BookingsClientProps { - status: BookingStatus; - userId?: number; -} +type BookingsClientProps = Pick<BookingsListProps, "status" | "userId">;Also applies to: 28-31
3-4: Remove redundant Suspense or switch tosuspense: trueinnext/dynamic(don’t mix both).You already provide a
loadingfallback indynamic(...). The extra Suspense boundary won’t trigger and just adds noise.Apply:
-import { Suspense } from "react"; +// Suspense not required when using next/dynamic with a loading fallback @@ export function BookingsClient({ status, userId }: BookingsClientProps) { return ( <div className="space-y-6"> - {/* Main bookings list */} - <Suspense - fallback={ - <div className="space-y-4"> - <div className="h-32 animate-pulse rounded bg-gray-200" /> - <div className="h-32 animate-pulse rounded bg-gray-200" /> - <div className="h-32 animate-pulse rounded bg-gray-200" /> - </div> - }> - <BookingsList status={status} userId={userId} /> - </Suspense> + <BookingsList status={status} userId={userId} /> </div> ); }Also applies to: 33-47
apps/web/app/(use-page-wrapper)/(main-nav)/event-types/EventTypesClient.tsx (2)
33-36: Drop unused_heading/_subtitleprops.They aren’t used here; keeping them adds noise and can confuse call sites.
Apply:
-interface EventTypesClientProps extends Pick<EventTypesViewProps, "userEventGroupsData" | "user"> { - _heading?: string; - _subtitle?: string; -} +type EventTypesClientProps = Pick<EventTypesViewProps, "userEventGroupsData" | "user">; @@ -export function EventTypesClient({ userEventGroupsData, user, _heading, _subtitle }: EventTypesClientProps) { +export function EventTypesClient({ userEventGroupsData, user }: EventTypesClientProps) {Also applies to: 38-38
3-4: Avoid mixingnext/dynamicloading with Suspense fallbacks.You already provide
loadingin both dynamic imports. The Suspense boundaries are redundant.Apply:
-import { Suspense } from "react"; +// Suspense not required when next/dynamic supplies loading fallbacks @@ - return ( - <> - <Suspense fallback={<div className="h-12 w-48 animate-pulse rounded bg-gray-200" />}> - <EventTypesCTA userEventGroupsData={userEventGroupsData} /> - </Suspense> - <Suspense - fallback={ - <div className="space-y-4"> - <div className="h-8 w-1/3 animate-pulse rounded bg-gray-200" /> - <div className="h-24 animate-pulse rounded bg-gray-200" /> - <div className="h-24 animate-pulse rounded bg-gray-200" /> - </div> - }> - <EventTypes userEventGroupsData={userEventGroupsData} user={user} /> - </Suspense> - </> - ); + return ( + <> + <EventTypesCTA userEventGroupsData={userEventGroupsData} /> + <EventTypes userEventGroupsData={userEventGroupsData} user={user} /> + </> + );Also applies to: 41-53
apps/web/app/(use-page-wrapper)/(main-nav)/event-types/page.tsx (1)
66-73: Stop passing unused_heading/_subtitleafter simplifying EventTypesClient.After dropping these props in EventTypesClient, remove them here too.
Apply:
- <EventTypesClient - userEventGroupsData={userEventGroupsData} - user={session.user} - _heading={t("event_types_page_title")} - _subtitle={t("event_types_page_subtitle")} - /> + <EventTypesClient userEventGroupsData={userEventGroupsData} user={session.user} />
📜 Review details
Configuration used: Path: .coderabbit.yaml
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 (3)
apps/web/app/(use-page-wrapper)/(main-nav)/bookings/[status]/BookingsClient.tsx(1 hunks)apps/web/app/(use-page-wrapper)/(main-nav)/event-types/EventTypesClient.tsx(1 hunks)apps/web/app/(use-page-wrapper)/(main-nav)/event-types/page.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:
apps/web/app/(use-page-wrapper)/(main-nav)/event-types/EventTypesClient.tsxapps/web/app/(use-page-wrapper)/(main-nav)/bookings/[status]/BookingsClient.tsxapps/web/app/(use-page-wrapper)/(main-nav)/event-types/page.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:
apps/web/app/(use-page-wrapper)/(main-nav)/event-types/EventTypesClient.tsxapps/web/app/(use-page-wrapper)/(main-nav)/bookings/[status]/BookingsClient.tsxapps/web/app/(use-page-wrapper)/(main-nav)/event-types/page.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:
apps/web/app/(use-page-wrapper)/(main-nav)/event-types/EventTypesClient.tsxapps/web/app/(use-page-wrapper)/(main-nav)/bookings/[status]/BookingsClient.tsxapps/web/app/(use-page-wrapper)/(main-nav)/event-types/page.tsx
🧠 Learnings (6)
📓 Common learnings
Learnt from: mdm317
PR: calcom/cal.com#23221
File: packages/features/schedules/components/NewScheduleButton.tsx:1-1
Timestamp: 2025-08-20T17:34:35.004Z
Learning: In the Cal.com codebase, server actions like `revalidateAvailabilityList()` from `app/(use-page-wrapper)/(main-nav)/availability/actions` are imported and called directly in client components within mutation `onSuccess` callbacks. This pattern is consistently used across availability-related components (both in availability-view.tsx for updates and NewScheduleButton.tsx for creates) to ensure the availability list cache is fresh after mutations.
Learnt from: mdm317
PR: calcom/cal.com#23221
File: packages/features/schedules/components/NewScheduleButton.tsx:1-1
Timestamp: 2025-08-20T17:34:35.004Z
Learning: In the Cal.com codebase, it's an established pattern for client components to directly import and call server actions like `revalidateAvailabilityList()` from `app/(use-page-wrapper)/(main-nav)/availability/actions` within mutation `onSuccess` callbacks. This pattern is consistently used across availability-view.tsx for delete, update, and bulk update operations, and should be followed for create operations as well to maintain consistency.
Learnt from: bandhan-majumder
PR: calcom/cal.com#23454
File: packages/features/bookings/Booker/components/EventMeta.tsx:16-16
Timestamp: 2025-08-29T22:57:31.407Z
Learning: In Cal.com's Booker architecture, components become client-side through BookerStoreProvider.tsx which has "use client". Any component using useBookerStoreContext automatically runs on the client and should use client-appropriate utilities like markdownToSafeHTMLClient, regardless of whether they have explicit "use client" directives.
Learnt from: bandhan-majumder
PR: calcom/cal.com#23454
File: packages/features/bookings/Booker/components/EventMeta.tsx:16-16
Timestamp: 2025-08-29T22:57:31.407Z
Learning: In Cal.com's Booker component hierarchy, the "use client" directive is placed at a higher level parent component, making all child components in packages/features/bookings/Booker/components/ run on the client side without needing individual "use client" directives. Components should still use client-appropriate utilities like markdownToSafeHTMLClient even without explicit "use client" directives.
📚 Learning: 2025-08-29T22:57:30.382Z
Learnt from: bandhan-majumder
PR: calcom/cal.com#23454
File: packages/features/form-builder/FormBuilder.tsx:11-11
Timestamp: 2025-08-29T22:57:30.382Z
Learning: FormBuilder.tsx in packages/features/form-builder/ does not have "use client" directive at the top despite using client-side React hooks and event handlers, which suggests it should be a client component.
Applied to files:
apps/web/app/(use-page-wrapper)/(main-nav)/event-types/EventTypesClient.tsxapps/web/app/(use-page-wrapper)/(main-nav)/bookings/[status]/BookingsClient.tsxapps/web/app/(use-page-wrapper)/(main-nav)/event-types/page.tsx
📚 Learning: 2025-08-20T17:34:35.004Z
Learnt from: mdm317
PR: calcom/cal.com#23221
File: packages/features/schedules/components/NewScheduleButton.tsx:1-1
Timestamp: 2025-08-20T17:34:35.004Z
Learning: In the Cal.com codebase, server actions like `revalidateAvailabilityList()` from `app/(use-page-wrapper)/(main-nav)/availability/actions` are imported and called directly in client components within mutation `onSuccess` callbacks. This pattern is consistently used across availability-related components (both in availability-view.tsx for updates and NewScheduleButton.tsx for creates) to ensure the availability list cache is fresh after mutations.
Applied to files:
apps/web/app/(use-page-wrapper)/(main-nav)/bookings/[status]/BookingsClient.tsxapps/web/app/(use-page-wrapper)/(main-nav)/event-types/page.tsx
📚 Learning: 2025-08-29T22:57:31.407Z
Learnt from: bandhan-majumder
PR: calcom/cal.com#23454
File: packages/features/bookings/Booker/components/EventMeta.tsx:16-16
Timestamp: 2025-08-29T22:57:31.407Z
Learning: In Cal.com's Booker architecture, components become client-side through BookerStoreProvider.tsx which has "use client". Any component using useBookerStoreContext automatically runs on the client and should use client-appropriate utilities like markdownToSafeHTMLClient, regardless of whether they have explicit "use client" directives.
Applied to files:
apps/web/app/(use-page-wrapper)/(main-nav)/bookings/[status]/BookingsClient.tsx
📚 Learning: 2025-08-29T22:57:31.407Z
Learnt from: bandhan-majumder
PR: calcom/cal.com#23454
File: packages/features/bookings/Booker/components/EventMeta.tsx:16-16
Timestamp: 2025-08-29T22:57:31.407Z
Learning: In Cal.com's Booker component hierarchy, the "use client" directive is placed at a higher level parent component, making all child components in packages/features/bookings/Booker/components/ run on the client side without needing individual "use client" directives. Components should still use client-appropriate utilities like markdownToSafeHTMLClient even without explicit "use client" directives.
Applied to files:
apps/web/app/(use-page-wrapper)/(main-nav)/bookings/[status]/BookingsClient.tsx
📚 Learning: 2025-08-20T17:34:35.004Z
Learnt from: mdm317
PR: calcom/cal.com#23221
File: packages/features/schedules/components/NewScheduleButton.tsx:1-1
Timestamp: 2025-08-20T17:34:35.004Z
Learning: In the Cal.com codebase, it's an established pattern for client components to directly import and call server actions like `revalidateAvailabilityList()` from `app/(use-page-wrapper)/(main-nav)/availability/actions` within mutation `onSuccess` callbacks. This pattern is consistently used across availability-view.tsx for delete, update, and bulk update operations, and should be followed for create operations as well to maintain consistency.
Applied to files:
apps/web/app/(use-page-wrapper)/(main-nav)/bookings/[status]/BookingsClient.tsx
🧬 Code graph analysis (2)
apps/web/app/(use-page-wrapper)/(main-nav)/event-types/EventTypesClient.tsx (1)
apps/web/modules/event-types/views/event-types-listing-view.tsx (1)
EventTypesCTA(952-968)
apps/web/app/(use-page-wrapper)/(main-nav)/event-types/page.tsx (1)
apps/web/app/(use-page-wrapper)/(main-nav)/event-types/EventTypesClient.tsx (1)
EventTypesClient(38-56)
⏰ 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 (4)
apps/web/app/(use-page-wrapper)/(main-nav)/bookings/[status]/BookingsClient.tsx (1)
13-26: Default export and props verified
The view fileapps/web/modules/bookings/views/bookings-listing-view.tsxexports a defaultBookingscomponent with aBookingsPropstype defining bothstatusand an optionaluserIdprop, matching the dynamic import usage. No changes required.apps/web/app/(use-page-wrapper)/(main-nav)/event-types/EventTypesClient.tsx (1)
7-9: Good: props derived from source view.Using
ComponentProps<typeof EventTypesListingView>ties this client to the canonical prop contract and prevents type drift.Also applies to: 33-33
apps/web/app/(use-page-wrapper)/(main-nav)/event-types/page.tsx (2)
16-18: Importing the client wrapper is correct.Shifting CTA/rendering to the client component aligns with the new lazy-load pattern.
66-73: No other EventTypesClient call sites pass heading/subtitle props.
|
@BenraouaneSoufiane can you add some actual recordings / screenshot of logs before vs after? |
Timing is too short/often varies without any changes so I could not exactly figure out the differences, also depends on activity on RAM/CPU (process running on the background,..) |
dosubot
bot
added
the
performance
…imports for avalability & teams, simplify main page as it only check session
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/app/(use-page-wrapper)/(main-nav)/availability/page.tsx (1)
50-61: Fix user-scoped caching to prevent cross-user data leakage.
unstable_cachekeys are static; passing headers/cookies as args does NOT key the cache. With a global key, one user’s availability can be served to another. Key by userId (and tag if you plan invalidation).Apply:
-const getCachedAvailabilities = unstable_cache( - async (headers: ReadonlyHeaders, cookies: ReadonlyRequestCookies) => { - const availabilityCaller = await createRouterCaller( - availabilityRouter, - await getTRPCContext(headers, cookies) - ); - return await availabilityCaller.list(); - }, - ["viewer.availability.list"], - { revalidate: 3600 } // Cache for 1 hour -); +const getCachedAvailabilitiesFor = (userId: string) => + unstable_cache( + async (headers: ReadonlyHeaders, cookies: ReadonlyRequestCookies) => { + const availabilityCaller = await createRouterCaller( + availabilityRouter, + await getTRPCContext(headers, cookies) + ); + return await availabilityCaller.list(); + }, + ["viewer.availability.list", userId], + { revalidate: 3600, tags: [`user:${userId}:availability`] } // Cache per user for 1 hour + );- const cachedAvailabilities = await getCachedAvailabilities(_headers, _cookies); + const cachedAvailabilities = await getCachedAvailabilitiesFor(session.user.id)(_headers, _cookies);I can propagate the same fix to other viewer-scoped caches in this PR if you want.
Also applies to: 72-72
apps/web/app/(use-page-wrapper)/(main-nav)/event-types/page.tsx (1)
37-55: Scope the event-groups cache by user and filters to prevent data mix-ups.Current
unstable_cachekey is static. Different users/filters can receive each other’s data. IncludeuserIdand a normalizedfilterskey.Apply:
-const getCachedEventGroups = unstable_cache( - async ( - headers: ReadonlyHeaders, - cookies: ReadonlyRequestCookies, - filters?: { - teamIds?: number[] | undefined; - userIds?: number[] | undefined; - upIds?: string[] | undefined; - } - ) => { - const eventTypesCaller = await createRouterCaller( - eventTypesRouter, - await getTRPCContext(headers, cookies) - ); - return await eventTypesCaller.getUserEventGroups({ filters }); - }, - ["viewer.eventTypes.getUserEventGroups"], - { revalidate: 3600 } // seconds -); +const getCachedEventGroupsFor = ( + userId: string, + filters?: { + teamIds?: number[] | undefined; + userIds?: number[] | undefined; + upIds?: string[] | undefined; + } +) => + unstable_cache( + async (headers: ReadonlyHeaders, cookies: ReadonlyRequestCookies) => { + const eventTypesCaller = await createRouterCaller( + eventTypesRouter, + await getTRPCContext(headers, cookies) + ); + return await eventTypesCaller.getUserEventGroups({ filters }); + }, + ["viewer.eventTypes.getUserEventGroups", userId, JSON.stringify(filters ?? {})], + { revalidate: 3600, tags: [`user:${userId}:event-groups`] } + );- const userEventGroupsData = await getCachedEventGroups(_headers, _cookies, filters); + const userEventGroupsData = await getCachedEventGroupsFor(session.user.id, filters)(_headers, _cookies);Want me to add cache invalidation tags where you mutate event types so this stays fresh?
Also applies to: 69-69
🧹 Nitpick comments (5)
apps/web/app/(use-page-wrapper)/(main-nav)/teams/page.tsx (1)
31-31: Avoid unnecessary awaits for headers()/cookies().
headers()andcookies()are sync in the App Router. Minor perf/readability win.Apply:
- const session = await getServerSession({ req: buildLegacyRequest(await headers(), await cookies()) }); + const session = await getServerSession({ req: buildLegacyRequest(headers(), cookies()) });apps/web/app/(use-page-wrapper)/(main-nav)/availability/page.tsx (1)
26-28: Add a lightweight loading fallback (accessibility, consistency).CTA pops in after hydration; a tiny, non-text status avoids layout shift and strings.
Apply:
-const AvailabilityCTA = dynamic(() => - import("~/availability/availability-view").then((mod) => mod.AvailabilityCTA) -); +const AvailabilityCTA = dynamic( + () => import("~/availability/availability-view").then((mod) => mod.AvailabilityCTA), + { loading: () => <div aria-busy="true" role="status" /> } +);apps/web/app/(use-page-wrapper)/(main-nav)/event-types/page.tsx (1)
24-26: Add a minimal loading fallback for CTA to avoid UI jank.Consistent with the main component’s fallback and improves perceived responsiveness.
Apply:
-const EventTypesCTA = dynamic(() => - import("~/event-types/views/event-types-listing-view").then((mod) => mod.EventTypesCTA) -); +const EventTypesCTA = dynamic( + () => import("~/event-types/views/event-types-listing-view").then((mod) => mod.EventTypesCTA), + { loading: () => <div aria-busy="true" role="status" /> } +);apps/web/app/page.tsx (2)
8-9: Broaden Auth cookie detection for reliability across environments.Include the secure cookie variant used in production.
Apply:
- const sessionCookie = _cookies.get("session") || _cookies.get("next-auth.session-token"); + const sessionCookie = + _cookies.get("session") || + _cookies.get("next-auth.session-token") || + _cookies.get("__Secure-next-auth.session-token");Confirm the exact cookie names used in your deployment (Auth.js/NextAuth version, prod vs. dev).
16-17: Fix stale comment.There’s no “dynamic import version above.” Suggest referencing getServerSession().
Apply:
- // Simple check - if you need full session data, use the dynamic import version above + // Simple check — if you need full session data, use getServerSession() instead
📜 Review details
Configuration used: Path: .coderabbit.yaml
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 (6)
apps/web/app/(use-page-wrapper)/(main-nav)/availability/page.tsx(1 hunks)apps/web/app/(use-page-wrapper)/(main-nav)/bookings/[status]/page.tsx(2 hunks)apps/web/app/(use-page-wrapper)/(main-nav)/event-types/page.tsx(2 hunks)apps/web/app/(use-page-wrapper)/(main-nav)/teams/page.tsx(2 hunks)apps/web/app/page.tsx(1 hunks)apps/web/next.config.js(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/app/(use-page-wrapper)/(main-nav)/bookings/[status]/page.tsx
- apps/web/next.config.js
🧰 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:
apps/web/app/page.tsxapps/web/app/(use-page-wrapper)/(main-nav)/availability/page.tsxapps/web/app/(use-page-wrapper)/(main-nav)/teams/page.tsxapps/web/app/(use-page-wrapper)/(main-nav)/event-types/page.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:
apps/web/app/page.tsxapps/web/app/(use-page-wrapper)/(main-nav)/availability/page.tsxapps/web/app/(use-page-wrapper)/(main-nav)/teams/page.tsxapps/web/app/(use-page-wrapper)/(main-nav)/event-types/page.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:
apps/web/app/page.tsxapps/web/app/(use-page-wrapper)/(main-nav)/availability/page.tsxapps/web/app/(use-page-wrapper)/(main-nav)/teams/page.tsxapps/web/app/(use-page-wrapper)/(main-nav)/event-types/page.tsx
🧠 Learnings (2)
📚 Learning: 2025-07-21T21:33:23.371Z
Learnt from: Anshumancanrock
PR: calcom/cal.com#22570
File: apps/web/modules/signup-view.tsx:253-253
Timestamp: 2025-07-21T21:33:23.371Z
Learning: In signup-view.tsx, when checking if redirectUrl contains certain strings, using explicit && checks (redirectUrl && redirectUrl.includes()) is preferred over optional chaining (redirectUrl?.includes()) to ensure the result is always a boolean rather than potentially undefined. This approach provides cleaner boolean contracts for downstream conditional logic.
Applied to files:
apps/web/app/page.tsx
📚 Learning: 2025-08-20T17:34:35.004Z
Learnt from: mdm317
PR: calcom/cal.com#23221
File: packages/features/schedules/components/NewScheduleButton.tsx:1-1
Timestamp: 2025-08-20T17:34:35.004Z
Learning: In the Cal.com codebase, server actions like `revalidateAvailabilityList()` from `app/(use-page-wrapper)/(main-nav)/availability/actions` are imported and called directly in client components within mutation `onSuccess` callbacks. This pattern is consistently used across availability-related components (both in availability-view.tsx for updates and NewScheduleButton.tsx for creates) to ensure the availability list cache is fresh after mutations.
Applied to files:
apps/web/app/(use-page-wrapper)/(main-nav)/availability/page.tsx
⏰ 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 (1)
apps/web/app/(use-page-wrapper)/(main-nav)/availability/page.tsx (1)
30-38: Consider disabling SSR for SliderTable if it touches window/document.If the slider uses browser-only APIs, set
ssr: falseto prevent server crashes.Apply (only if needed):
const AvailabilitySliderTable = dynamic( () => import("@calcom/features/timezone-buddy/components/AvailabilitySliderTable").then( (mod) => mod.AvailabilitySliderTable ), { - loading: () => <div>Loading team availability...</div>, + ssr: false, + loading: () => <div aria-busy="true" role="status" />, } );
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/app-store/googlecalendar/lib/__tests__/CalendarService.test.ts (2)
1313-1314: Fix mismatch: comment says 5x but assertion uses 2xUpdate the inline comment to reflect the relaxed threshold to avoid confusion for future readers.
- // Assert significant performance improvement (at least 5x faster) + // Assert significant performance improvement (at least 2x faster) expect(speedupRatio).toBeGreaterThan(2);
1305-1306: Harden ratio calc against near-zero timings to reduce flakinessGuard against extremely small nativeTime (e.g., JIT/warmup effects) producing inflated ratios.
- const speedupRatio = dayjsTime / nativeTime; + const speedupRatio = dayjsTime / Math.max(nativeTime, 0.1);
📜 Review details
Configuration used: Path: .coderabbit.yaml
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 (1)
packages/app-store/googlecalendar/lib/__tests__/CalendarService.test.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/app-store/googlecalendar/lib/__tests__/CalendarService.test.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/app-store/googlecalendar/lib/__tests__/CalendarService.test.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/app-store/googlecalendar/lib/__tests__/CalendarService.test.ts
🧠 Learnings (2)
📚 Learning: 2025-08-05T12:04:29.037Z
Learnt from: din-prajapati
PR: calcom/cal.com#21854
File: packages/app-store/office365calendar/__tests__/unit_tests/SubscriptionManager.test.ts:0-0
Timestamp: 2025-08-05T12:04:29.037Z
Learning: In packages/app-store/office365calendar/lib/CalendarService.ts, the fetcher method in Office365CalendarService class is public, not private. It was specifically changed from private to public in this PR to support proper testing and external access patterns.
Applied to files:
packages/app-store/googlecalendar/lib/__tests__/CalendarService.test.ts
📚 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:
packages/app-store/googlecalendar/lib/__tests__/CalendarService.test.ts
⏰ 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). (3)
- GitHub Check: Tests / Unit
- GitHub Check: Type check / check-types
- GitHub Check: Linters / lint
|
@keithwillcode, @hbjORbj I did some updates, if you can take a look. |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
@keithwillcode can you take a look? |
|
Hello @keithwillcode any updates on this? |
Hey @BenraouaneSoufiane lot's going on. Will provide feedback when I can |
Hhh, thinked you just forgot! |
|
@keithwillcode should you tip me please??! |
romitg2
left a comment
romitg2
left a comment
There was a problem hiding this comment.
@BenraouaneSoufiane Thank you for your contribution!
There seems to merge conflicts and the related issue is closed. can you please resolve merge conflicts and update your performance stats! if you think there is still scope of performance improvement then you can always create new PR!
Closing this one, Thanks again for PR!
5 participants
What does this PR do?
This PR significantly improves development performance by optimizing compilation times and reducing memory usage through strategic client-side component loading and build configuration enhancements.
Performance Improvements Achieved:
Key Changes Made:
1. Client-Side Component Optimization
EventTypesList,EventTypesCTA, andAvailabilitySettingsto client-side with dynamic importsBookingsListto client-side rendering with proper loading states2. Next.js Configuration Enhancements
@calcom/embed-core,@calcom/embed-react,@calcom/features)3. TypeScript and Architecture Improvements
anytypesVisual Demo
Before Optimization:
After Optimization:
Mandatory Tasks
How should this be tested?
Environment Setup:
/claim #23104