Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughAdds a required app Possibly related PRs
✨ Finishing touches🧪 Generate unit tests (beta)
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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (08/19/25)1 reviewer was added to this PR based on Keith Williams's automation. |
E2E results are ready! |
|
Related Documentation No published documentation to review for changes on this repository. |
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 (2)
apps/web/app/api/social/og/image/route.tsx (2)
88-93: Add long-lived Cache-Control to leverage CDN/client caching (not just Next cache)
revalidate = falsecovers Next's server cache. Without explicitCache-Control, downstream caches (CDN/browser/social crawlers) may not retain the image as long as intended. Set a strong, immutable TTL to align with the “indefinite caching” objective and rely on URL param changes for busting.Apply this diff across the three 200 responses:
@@ return new Response(img.body, { status: 200, headers: { "Content-Type": "image/png", + "Cache-Control": "public, immutable, max-age=31536000, s-maxage=31536000" }, }); @@ return new Response(img.body, { status: 200, headers: { "Content-Type": "image/png", + "Cache-Control": "public, immutable, max-age=31536000, s-maxage=31536000" }, }); @@ return new Response(img.body, { status: 200, headers: { "Content-Type": "image/png", + "Cache-Control": "public, immutable, max-age=31536000, s-maxage=31536000" }, });Also applies to: 121-126, 154-159
95-107: Prevent caching of error responses (avoid poisoning the indefinite cache)With indefinite caching, error responses (400/404/500) risk being cached by intermediaries and served repeatedly. Mark them
no-store. Also set an explicit Content-Type for the text responses.Apply this diff:
@@ - return new Response( + return new Response( JSON.stringify({ error: "Invalid parameters for meeting image", message: "Required parameters: title, meetingProfileName. Optional: names, usernames, meetingImage", }), { status: 400, - headers: { "Content-Type": "application/json" }, + headers: { + "Content-Type": "application/json", + "Cache-Control": "no-store" + }, } ); @@ - return new Response( + return new Response( JSON.stringify({ error: "Invalid parameters for app image", message: "Required parameters: name, description, slug", }), { status: 400, - headers: { "Content-Type": "application/json" }, + headers: { + "Content-Type": "application/json", + "Cache-Control": "no-store" + }, } ); @@ - return new Response( + return new Response( JSON.stringify({ error: "Invalid parameters for generic image", message: "Required parameters: title, description", }), { status: 400, - headers: { "Content-Type": "application/json" }, + headers: { + "Content-Type": "application/json", + "Cache-Control": "no-store" + }, } ); @@ - default: - return new Response("What you're looking for is not here..", { status: 404 }); + default: + return new Response("What you're looking for is not here..", { + status: 404, + headers: { + "Content-Type": "text/plain; charset=utf-8", + "Cache-Control": "no-store" + } + }); @@ - } catch (error) { - return new Response("Internal server error", { status: 500 }); + } catch (error) { + return new Response("Internal server error", { + status: 500, + headers: { + "Content-Type": "text/plain; charset=utf-8", + "Cache-Control": "no-store" + } + }); }Also applies to: 127-139, 161-171, 177-179, 180-182
🧹 Nitpick comments (3)
apps/web/app/api/social/og/image/route.tsx (3)
38-57: Optional: fetch fonts once per process to cut first-render latencyEach unique URL’s first render pays the font fetch cost. Caching fonts at module scope avoids repeated upstream fetches and reduces tail latency under traffic.
You can refactor like this (outside the selected range, illustrative):
// at module top const fontsPromise: Promise<SatoriOptions["fonts"]> = (async () => { const results = await Promise.allSettled([ fetch(new URL("/fonts/cal.ttf", WEBAPP_URL)).then((r) => r.arrayBuffer()), fetch(new URL("/fonts/Inter-Regular.ttf", WEBAPP_URL)).then((r) => r.arrayBuffer()), fetch(new URL("/fonts/Inter-Medium.ttf", WEBAPP_URL)).then((r) => r.arrayBuffer()), ]); const fonts: SatoriOptions["fonts"] = []; if (results[1].status === "fulfilled") fonts.push({ name: "inter", data: results[1].value, weight: 400 }); if (results[2].status === "fulfilled") fonts.push({ name: "inter", data: results[2].value, weight: 500 }); if (results[0].status === "fulfilled") { fonts.push({ name: "cal", data: results[0].value, weight: 400 }); fonts.push({ name: "cal", data: results[0].value, weight: 600 }); } return fonts; })(); // then in handler, replace the per-request block with: const fonts = await fontsPromise;
177-179: Nit: hardcoded user-facing strings in a .tsx fileGuideline reminder: in frontend .tsx code use t() for text. These are API responses, so localization likely isn’t required, but consider centralizing strings or suppressing the lint in server-only files to avoid future drift.
Also applies to: 180-182
9-9: Add explicit Node runtime for consistent cache semanticsA search for any
export const runtime = "edge"underapps/web/appreturned no matches, so there’s no accidental Edge runtime inheritance.• In apps/web/app/api/social/og/image/route.tsx, before the existing
revalidateexport, add:+ export const runtime = "nodejs"; export const revalidate = false; // this results in indefinite caching, meaning that a new deployment will not invalidate the cache
📜 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 (1)
apps/web/app/api/social/og/image/route.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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/api/social/og/image/route.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/api/social/og/image/route.tsx
There was a problem hiding this comment.
2 issues found across 8 files
Prompt for AI agents (all 2 issues)
Understand the root cause of the following 2 issues and fix them.
<file name="packages/lib/OgImages.tsx">
<violation number="1" location="packages/lib/OgImages.tsx:220">
`config.avatarSize` holds "160", so this tailwind class expands to `w-[160]` / `h-[160]`, yielding invalid CSS and breaking the avatar sizing. Please append the pixel unit when composing these arbitrary value classes.</violation>
</file>
<file name="apps/web/app/api/social/og/image/route.tsx">
<violation number="1" location="apps/web/app/api/social/og/image/route.tsx:96">
Wrap the computed hash in quotes when setting the ETag header; unquoted entity tags violate RFC 7232 and may be ignored by caches.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| const svgHashesModule = await import("@calcom/web/public/app-store/svg-hashes.json"); | ||
| const SVG_HASHES = svgHashesModule.default ?? {}; | ||
| const svgHash = SVG_HASHES[slug] ?? undefined; |
There was a problem hiding this comment.
svg-hashes.json looks like:
{
"zoomvideo": "d1c78abf",
"zohocrm": "102d69df",
"zohocalendar": "a9a369fe",
"zoho-bigin": "d3ae97fd",
"zapier": "9a1570dd",
"wordpress": "d572db7e",
"wipemycalother": "e4bb0e21",
"whereby": "65fd921f",
"whatsapp": "4025a2c2",
"weather_in_your_calendar": "4111fee5",
"vital": "86b4bf1e",
"vimcal": "8001675b",
"umami": "0593be46",
"twipla": "86191ba1",
"templates/link-as-an-app": "506ff3da",
"templates/general-app-settings": "506ff3da",
"templates/event-type-location-video-static": "506ff3da",
"templates/event-type-app-card": "506ff3da",
"templates/booking-pages-tag": "506ff3da",
"templates/basic": "506ff3da",
"telli": "202f4d2a",
"telegram": "73f9a0ed",
"tandemvideo": "7eb7508a",
"synthflow": "283524ad",
"sylapsvideo": "3e48b068",
"stripepayment": "11b0b1ee",
"skype": "4176a41c",
"sirius_video": "7c49e446",
"signal": "0736faad",
"salesroom": "b5a77692",
"routing-forms": "d6cfc1dd",
"roam": "4c6094e4",
"riverside": "61e15a9d",
"retell-ai": "f90d2781",
"raycast": "d94a2a01",
"qr_code": "9efd6cef",
"posthog": "8681a845",
"plausible": "91049a3a",
"pipedrive-crm": "673e0931",
"pipedream": "5600de90",
"ping": "5bb01833",
"paypal": "a540c421",
"office365video": "97d4df35",
"office365calendar": "ac5dd392",
"nextcloudtalk": "d0266d7a",
"n8n": "c92f3b72",
"monobot": "6d601f42",
"mock-payment-app": "506ff3da",
"mirotalk": "f5298670",
"metapixel": "fa32781d",
"matomo": "7c2e329e",
"make": "ca519871",
"linear": "f78f9869",
"lindy": "c7ab2989",
"larkcalendar": "f93540ea",
"jitsivideo": "79fdec8b",
"jelly": "040ad0e1",
"intercom": "fd949dc0",
"insihts": "aa7e93d7",
"ics-feedcalendar": "44c4adaa",
"huddle01video": "81a0653b",
"hubspot": "a6e01fff",
"horizon-workrooms": "fa32781d",
"hitpay": "9f0a5120",
"gtm": "65847f31",
"greetmate-ai": "025e82b7",
"granola": "2829eb38",
"googlevideo": "e4bb0e21",
"googlecalendar": "062af390",
"giphy": "c67e5a9b",
"ga4": "9822cbf6",
"feishucalendar": "f93540ea",
"fathom": "9fde6e4c",
"facetime": "09f45b11",
"exchangecalendar": "a63ab3e4",
"exchange2016calendar": "a63ab3e4",
"exchange2013calendar": "a63ab3e4",
"elevenlabs": "9ae79100",
"element-call": "8c79e11f",
"eightxeight": "3a97ea08",
"dub": "db4e9834",
"discord": "147f41c9",
"dialpad": "aef13faa",
"demodesk": "fd72da43",
"deel": "5e1041f9",
"dailyvideo": "9567de52",
"closecom": "45302531",
"chatbase": "332dee04",
"campfire": "584fd592",
"caldavcalendar": "70757035",
"btcpayserver": "02235e9e",
"bolna": "6fd7aa5f",
"basecamp3": "de61c3ec",
"baa-for-hipaa": "735c46eb",
"autocheckin": "94ffa8f2",
"attio": "0bc3964f",
"applecalendar": "1fde27dd",
"amie": "f9c089a1",
"alby": "40854e92"
}The key for Zoom, for example, is zoomvideo, not zoom
What does this PR do?
Configuration set for OG images:
Mandatory Tasks (DO NOT REMOVE)
How to test
P.S. You can see that all the OG image URLs below now have
vparameter containing a hash :)1. App Type OG Image:
http://localhost:3000/api/social/og/image?type=app&name=Zoom+Video&slug=zoomvideo&description=Zoom+is+a+secure+and+reliable+video+platform+that+supports+all+of+your+online+communication+needs.+It+can+provide+everything+from+one+on+one+meetings%2C+chat%2C+phone%2C+webinars%2C+and+large-scale+online+events.+Available+with+both+desktop%2C+web%2C+and+mobile+versions.&logoUrl=%2Fapp-store%2Fzoomvideo%2Ficon.svg&v=aa1943a82. Meeting Type OG Image:
http://localhost:3000/api/social/og/image?type=meeting&title=quick+chat&meetingProfileName=Admin+Example&meetingImage=http%3A%2F%2Flocalhost%3A3000%2Favatar.svg&names=Admin+Example&usernames=admin&v=0825c61c3. Generic Type OG Image:
http://localhost:3000/api/social/og/image?type=generic&title=Event+Types+%7C+Cal.com&description=Create+events+to+share+for+people+to+book+on+your+calendar.&v=f637cf6b