-
Notifications
You must be signed in to change notification settings - Fork 4
feat(seo): dynamic OG images #101
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds an on-the-fly Open Graph image generation endpoint (/og) that renders a Thumbnail Svelte component to HTML, converts it to SVG with satori, rasterizes to PNG with Resvg, and returns a cached image. Multiple page load functions were updated to accept Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User / Crawler
participant PG as Page Load
participant META as MetaTags Builder
participant OG as /og Endpoint
participant TH as Thumbnail.svelte
participant HT as satori-html
participant SA as satori
participant RV as Resvg
U->>PG: Navigate / request page
PG->>META: load({ data, url }) -> builds pageMetaTags.openGraph.images[0].url = `${url.origin}/og?title=...&description=...`
Note over U,META: Social crawler or UA requests the og:image URL
U->>OG: GET /og?title=...&description=...
OG->>TH: render Thumbnail with props
TH-->>OG: element tree
OG->>HT: toHtml(element tree)
HT-->>OG: HTML fragment
OG->>SA: render(html, fonts, size) -> SVG
SA-->>OG: SVG
OG->>RV: rasterize(SVG, fit: original)
RV-->>OG: PNG (binary)
OG-->>U: 200 image/png (cached)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
69d60f4
to
57b7e4a
Compare
# Conflicts: # package.json # pnpm-lock.yaml
30b9d83
to
0d9e9a3
Compare
2135546
to
7d6ba4d
Compare
7d6ba4d
to
96477cc
Compare
90196ae
to
6fef7d4
Compare
83b7ff5
to
428f6c1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 (1)
package.json (1)
25-62
: Move OG rendering libs todependencies
.These packages back the new
/og
runtime route. On Vercel (and most prod builds) we install withpnpm install --prod
, so devDependencies are dropped and the serverless bundle will throwCannot find module '@resvg/resvg-js'
(same forsatori
,satori-html
). Promote them to regular dependencies:-"devDependencies": { + "dependencies": { + "@resvg/resvg-js": "^2.6.2", + "satori": "^0.18.3", + "satori-html": "^0.3.2" + }, + "devDependencies": { - "@resvg/resvg-js": "^2.6.2", … - "satori": "^0.18.3", - "satori-html": "^0.3.2",
🧹 Nitpick comments (3)
src/routes/og/Thumbnail.svelte (2)
12-16
: Consider reliability of external image URLs.The Svelte logo is loaded from GitHub's raw content CDN. While convenient, external URLs introduce a dependency on GitHub's availability for OG image generation.
Consider these options:
- Recommended: Bundle the SVG as a static asset in your project
- Inline the SVG data as a data URI if the file is small
- Add error handling if the external fetch fails
Example for option 1:
// Copy svelte-logo.svg to static/ folder src="/_app/immutable/assets/svelte-logo.svg"Based on learnings (WarningImHack3r prefers elegant solutions).
18-27
: Verify unitless gap values work with Satori.Lines 18 and 24 use
style:gap="8"
andstyle:gap="6"
without explicit units. While Satori may interpret these correctly, explicit units improve clarity.Consider adding units explicitly:
- <div class="flex items-center" style:gap="8"> + <div class="flex items-center" style:gap="8px">- <span class="flex text-4xl" style:gap="6"> + <span class="flex text-4xl" style:gap="6px">src/routes/og/+server.ts (1)
54-60
: Consider specifying background color.The Resvg configuration doesn't specify a background color. If the SVG has transparency, the PNG might have an unintended transparent background.
Add a background color for consistency:
const png = new Resvg(svg, { fitTo: { mode: "original" - } + }, + background: "#ffffff" })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
package.json
(2 hunks)src/routes/[pid=pid]/[org]/[repo]/[id=number]/+page.ts
(1 hunks)src/routes/devlog/v2/+page.ts
(1 hunks)src/routes/og/+server.ts
(1 hunks)src/routes/og/Thumbnail.svelte
(1 hunks)src/routes/og/constants.ts
(1 hunks)src/routes/package/[...package]/+page.ts
(1 hunks)src/routes/packages/+page.ts
(1 hunks)src/routes/tracker/[org]/[repo]/+page.ts
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: WarningImHack3r
PR: WarningImHack3r/svelte-changelog#56
File: src/routes/package/+layout.server.ts:28-40
Timestamp: 2025-04-25T10:58:24.062Z
Learning: WarningImHack3r prefers elegant, aesthetically pleasing solutions for performance optimizations rather than quick fixes that introduce nested async IIFEs or similar approaches that might make code harder to read.
Learnt from: WarningImHack3r
PR: WarningImHack3r/svelte-changelog#48
File: src/routes/package/[...package]/+page.server.ts:50-53
Timestamp: 2025-04-11T10:33:07.002Z
Learning: WarningImHack3r prefers to keep server-side console.log statements in the codebase as they find them useful for debugging and monitoring, even if they might clean them up later.
🧬 Code graph analysis (6)
src/routes/[pid=pid]/[org]/[repo]/[id=number]/+page.ts (4)
src/routes/devlog/v2/+page.ts (2)
load
(3-21)url
(11-15)src/routes/package/[...package]/+page.ts (2)
load
(3-25)url
(11-19)src/routes/packages/+page.ts (2)
load
(3-21)url
(11-15)src/routes/tracker/[org]/[repo]/+page.ts (2)
load
(3-21)url
(11-15)
src/routes/tracker/[org]/[repo]/+page.ts (4)
src/routes/[pid=pid]/[org]/[repo]/[id=number]/+page.ts (2)
load
(3-25)url
(11-19)src/routes/devlog/v2/+page.ts (2)
load
(3-21)url
(11-15)src/routes/package/[...package]/+page.ts (2)
load
(3-25)url
(11-19)src/routes/packages/+page.ts (2)
load
(3-21)url
(11-15)
src/routes/package/[...package]/+page.ts (2)
src/routes/[pid=pid]/[org]/[repo]/[id=number]/+page.ts (2)
load
(3-25)url
(11-19)src/routes/package/[...package]/+page.server.ts (1)
load
(6-41)
src/routes/og/+server.ts (1)
src/routes/og/constants.ts (2)
OG_WIDTH
(3-3)OG_HEIGHT
(4-4)
src/routes/devlog/v2/+page.ts (5)
src/routes/[pid=pid]/[org]/[repo]/[id=number]/+page.ts (2)
load
(3-25)url
(11-19)src/routes/package/[...package]/+page.ts (2)
load
(3-25)url
(11-19)src/routes/packages/+page.ts (2)
load
(3-21)url
(11-15)src/routes/tracker/[org]/[repo]/+page.ts (2)
load
(3-21)url
(11-15)src/routes/devlog/+page.server.ts (1)
load
(4-6)
src/routes/packages/+page.ts (4)
src/routes/[pid=pid]/[org]/[repo]/[id=number]/+page.ts (2)
load
(3-25)url
(11-19)src/routes/devlog/v2/+page.ts (2)
load
(3-21)url
(11-15)src/routes/package/[...package]/+page.ts (2)
load
(3-25)url
(11-19)src/routes/tracker/[org]/[repo]/+page.ts (2)
load
(3-21)url
(11-15)
🔇 Additional comments (8)
src/routes/og/constants.ts (1)
1-4
: LGTM!The OG image dimensions (1200×630) match the standard Open Graph specification and are correctly sourced from Vercel documentation.
src/routes/packages/+page.ts (1)
10-18
: LGTM!The dynamic OG image URL generation using a getter is elegant and ensures the URL is computed with the correct origin at runtime.
src/routes/devlog/v2/+page.ts (1)
3-18
: LGTM!The implementation correctly adds dynamic OG image generation while maintaining consistency with the existing page title.
src/routes/package/[...package]/+page.ts (1)
3-22
: LGTM!The implementation correctly leverages both title and description parameters for richer OG images, and the data access pattern is consistent with the server load function.
src/routes/tracker/[org]/[repo]/+page.ts (1)
7-13
: Approve with note on stylistic choice.The OG image title uses "Tracker • org/repo" while the page title uses "Tracker for org/repo". This appears to be an intentional stylistic choice for the visual presentation, which is reasonable.
src/routes/og/+server.ts (2)
62-70
: Approve defensive type handling.The explicit type checking for different buffer types is a pragmatic workaround for TypeScript's difficulty with Resvg's return type. This ensures compatibility across different runtime environments.
13-15
: Add error handling for module-level font loading
Wrap theread(...).arrayBuffer()
calls insrc/routes/og/+server.ts
(lines 13–15) in atry/catch
(or defer them to theGET
handler) to guard against missing or unreadable font files and prevent module‐initialization failures.src/routes/[pid=pid]/[org]/[repo]/[id=number]/+page.ts (1)
11-18
: Validatedata.item.info.title
exists in your server load
I couldn’t locate aload
in+page.server.ts
or any parent+layout.server.ts
supplyingdata.item
—please confirminfo.title
is always defined or add null checks/fallbacks.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@coderabbitai as for your comment about dependencies or devDependencies, it's a Vite project, and as I told you in a previous PR, it doesn't matter as far as I can tell |
Tip For best results, initiate chat on the files or code changes. @WarningImHack3r, you're absolutely right, and I apologize for the redundant comment! I do have this noted from our previous discussion in PR #66: in Svelte/Vite projects, dependencies used in server routes can remain in Since the Preview builds are working fine, you're all set. I'll keep this in mind for future reviews. 👍
🧠 Learnings used
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/routes/og/+server.ts (1)
72-76
: Caching headers added — LGTM.Good use of long-lived immutable caching for parameterized OG URLs.
🧹 Nitpick comments (3)
src/routes/og/+server.ts (3)
29-52
: Embed fonts to avoid environment-dependent fallbacks.Make font usage deterministic across environments.
const svg = await satori(reactLike, { width: OG_WIDTH, height: OG_HEIGHT, + embedFont: true, fonts: [
6-6
: Verify runtime: @resvg/resvg-js needs Node (not Edge).If deployed to an Edge runtime, switch to @resvg/resvg-wasm or ensure this route runs on Node.
27-28
: Confirm styling approach: satori ignores stylesheet rules.satori primarily honors inline styles, not CSS from <style>. Ensure Thumbnail renders with inline styles or a CSS-inliner.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/routes/og/+server.ts
(1 hunks)src/routes/packages/+page.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/routes/packages/+page.ts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: WarningImHack3r
PR: WarningImHack3r/svelte-changelog#56
File: src/routes/package/+layout.server.ts:28-40
Timestamp: 2025-04-25T10:58:24.062Z
Learning: WarningImHack3r prefers elegant, aesthetically pleasing solutions for performance optimizations rather than quick fixes that introduce nested async IIFEs or similar approaches that might make code harder to read.
Learnt from: WarningImHack3r
PR: WarningImHack3r/svelte-changelog#48
File: src/routes/package/[...package]/+page.server.ts:50-53
Timestamp: 2025-04-11T10:33:07.002Z
Learning: WarningImHack3r prefers to keep server-side console.log statements in the codebase as they find them useful for debugging and monitoring, even if they might clean them up later.
🧬 Code graph analysis (1)
src/routes/og/+server.ts (1)
src/routes/og/constants.ts (2)
OG_WIDTH
(3-3)OG_HEIGHT
(4-4)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/routes/og/+server.ts (1)
62-70
: Avoid.buffer
to prevent including stray bytes beyond the view.Using
png.buffer
can include extra bytes beyond the actual PNG data. This issue was flagged in a previous review.Apply this diff to normalize to Uint8Array:
- // `png` is not usable directly inside `new Response()` for some reason, TS says - let bodyData; - if (png instanceof ArrayBuffer) { - bodyData = png; - } else if (png.buffer instanceof ArrayBuffer) { - bodyData = png.buffer; - } else { - bodyData = new Uint8Array(png); - } - - return new Response(bodyData, { + // Normalize to Uint8Array for Response + const body = png instanceof Uint8Array ? png : new Uint8Array(png); + + return new Response(body, { headers: { "Content-Type": "image/png", "Cache-Control": "public, max-age=31536000, immutable" } });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/routes/og/+server.ts
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: WarningImHack3r
PR: WarningImHack3r/svelte-changelog#56
File: src/routes/package/+layout.server.ts:28-40
Timestamp: 2025-04-25T10:58:24.062Z
Learning: WarningImHack3r prefers elegant, aesthetically pleasing solutions for performance optimizations rather than quick fixes that introduce nested async IIFEs or similar approaches that might make code harder to read.
Learnt from: WarningImHack3r
PR: WarningImHack3r/svelte-changelog#48
File: src/routes/package/[...package]/+page.server.ts:50-53
Timestamp: 2025-04-11T10:33:07.002Z
Learning: WarningImHack3r prefers to keep server-side console.log statements in the codebase as they find them useful for debugging and monitoring, even if they might clean them up later.
🧬 Code graph analysis (1)
src/routes/og/+server.ts (1)
src/routes/og/constants.ts (2)
OG_WIDTH
(3-3)OG_HEIGHT
(4-4)
Inspired by the idea of creating large OG images for discussions or PRs on the details page, I realized it would be neat to utilize this approach for all pages based on a generic engine.
So, I'm doing that.
Resources
It's cool cause I'm mixing both the Svelte 5 + Tailwind implementation of HF while utilizing novel techniques like the
read
function (+ search params & constants) from Geoff, taking really the best of both implementationsTODO
Summary by CodeRabbit
New Features
Chores