-
Notifications
You must be signed in to change notification settings - Fork 1.5k
perf(og): render blog OG/Instagram images on-demand, not at build #4709
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?
Changes from all commits
16fbf07
5f63ef0
ee6dc35
ba4793b
59a6998
708ef63
fdde436
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,23 +1,43 @@ | ||
| // Dynamic 4:5 Instagram image for a blog post, as a Route Handler. See | ||
| // app/blog/og/[...slug] for the on-demand-ISR / static-export split. | ||
|
|
||
| import { generateBlogStaticParams } from 'utils/blog/generateBlogStaticParams'; | ||
| import { getBlogPost } from 'utils/blog/getBlogPost'; | ||
| import { renderBlogInstagramImage } from 'utils/og/blogInstagramImage'; | ||
|
|
||
| const IS_EXPORT = process.env.EXPORT_MODE === 'static'; | ||
|
|
||
| export const dynamic = 'force-static'; | ||
| export const dynamicParams = false; | ||
| export const dynamicParams = !IS_EXPORT; | ||
|
|
||
| export function generateStaticParams() { | ||
| return generateBlogStaticParams('en'); | ||
| return IS_EXPORT ? generateBlogStaticParams('en') : []; | ||
| } | ||
|
|
||
| export async function GET( | ||
| _request: Request, | ||
| { params }: { params: { slug: string[] } }, | ||
| ) { | ||
| const slugPath = params.slug.join('/'); | ||
| const { post } = await getBlogPost('en', slugPath); | ||
| // Tina's client defaults to errorPolicy: 'throw', so an unknown slug throws | ||
| // rather than returning { post: null } (same convention as app/blog/[...slug]). | ||
| let post: Awaited<ReturnType<typeof getBlogPost>>['post']; | ||
| try { | ||
| ({ post } = await getBlogPost('en', slugPath)); | ||
| } catch (error) { | ||
| console.error( | ||
| `Error fetching post for Instagram image: ${slugPath}`, | ||
| error, | ||
| ); | ||
| post = null; | ||
| } | ||
| if (!post) { | ||
| // Unknown slug: 404 rather than render + ISR-cache a generic fallback image. | ||
| return new Response(null, { status: 404 }); | ||
| } | ||
| return renderBlogInstagramImage({ | ||
| title: post?.title ?? 'TinaCMS Blog', | ||
| author: post?.author, | ||
| title: post.title, | ||
| author: post.author, | ||
| seed: slugPath, | ||
| }); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,29 +6,47 @@ | |
| // So we serve the image from a distinct prefix (/blog/og/<slug>) and point | ||
| // `openGraph.images` at it from the post's generateMetadata. | ||
| // | ||
| // force-static + generateStaticParams pre-renders one image per post at build | ||
| // (where the fonts/photos under public/ are readable via fs). | ||
| // Rendered on-demand and cached (ISR): the first request for a post's image | ||
| // renders it, then it's served from cache. A static export (output: 'export') | ||
| // can't generate on-demand, so it must prebuild every image instead — gated on | ||
| // the same EXPORT_MODE switch next.config.js uses. Fonts/photos load from | ||
| // public/ on disk with a live production-host fallback (see utils/og/ogAssets), | ||
| // so they resolve at build and at on-demand runtime. | ||
|
|
||
| import { generateBlogStaticParams } from 'utils/blog/generateBlogStaticParams'; | ||
| import { getBlogPost } from 'utils/blog/getBlogPost'; | ||
| import { renderBlogOgImage } from 'utils/og/blogOgImage'; | ||
|
|
||
| const IS_EXPORT = process.env.EXPORT_MODE === 'static'; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Heads up: this gate can never be true — pre-existing bug, not introduced here.
The comma makes that a single shell assignment, so So Worth knowing though: the PR description's "a static export still prebuilds every image" describes a path that currently can't execute or be tested. It also implies
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed, pre-existing and self-consistent with next.config.js, so leaving it here. Separate follow-up to fix the comma ( |
||
|
|
||
| export const dynamic = 'force-static'; | ||
| export const dynamicParams = false; | ||
| export const dynamicParams = !IS_EXPORT; | ||
|
|
||
| export function generateStaticParams() { | ||
| return generateBlogStaticParams('en'); | ||
| return IS_EXPORT ? generateBlogStaticParams('en') : []; | ||
| } | ||
|
|
||
| export async function GET( | ||
| _request: Request, | ||
| { params }: { params: { slug: string[] } }, | ||
| ) { | ||
| const slugPath = params.slug.join('/'); | ||
| const { post } = await getBlogPost('en', slugPath); | ||
| // Tina's client defaults to errorPolicy: 'throw', so an unknown slug throws | ||
| // rather than returning { post: null } (same convention as app/blog/[...slug]). | ||
| let post: Awaited<ReturnType<typeof getBlogPost>>['post']; | ||
| try { | ||
| ({ post } = await getBlogPost('en', slugPath)); | ||
| } catch (error) { | ||
| console.error(`Error fetching post for OG image: ${slugPath}`, error); | ||
| post = null; | ||
| } | ||
| if (!post) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This 404 guard is unreachable — unknown slugs will 500 instead.
So the throw propagates out of This path didn't exist before:
let post;
try {
({ post } = await getBlogPost('en', slugPath));
} catch {
post = null;
}
if (!post) {
return new Response(null, { status: 404 });
}Alternatively, validating
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✅ Done. Wrapped |
||
| // Unknown slug: 404 rather than render + ISR-cache a generic fallback image. | ||
| return new Response(null, { status: 404 }); | ||
| } | ||
| return renderBlogOgImage({ | ||
| title: post?.title ?? 'TinaCMS Blog', | ||
| author: post?.author, | ||
| title: post.title, | ||
| author: post.author, | ||
| seed: slugPath, | ||
| }); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,22 +2,39 @@ import { generateBlogStaticParams } from 'utils/blog/generateBlogStaticParams'; | |
| import { getBlogPost } from 'utils/blog/getBlogPost'; | ||
| import { renderBlogInstagramImage } from 'utils/og/blogInstagramImage'; | ||
|
|
||
| const IS_EXPORT = process.env.EXPORT_MODE === 'static'; | ||
|
|
||
| export const dynamic = 'force-static'; | ||
| export const dynamicParams = false; | ||
| export const dynamicParams = !IS_EXPORT; | ||
|
|
||
| export function generateStaticParams() { | ||
| return generateBlogStaticParams('zh'); | ||
| return IS_EXPORT ? generateBlogStaticParams('zh') : []; | ||
| } | ||
|
|
||
| export async function GET( | ||
| _request: Request, | ||
| { params }: { params: { slug: string[] } }, | ||
| ) { | ||
| const slugPath = params.slug.join('/'); | ||
| const { post } = await getBlogPost('zh', slugPath); | ||
| // Tina's client defaults to errorPolicy: 'throw', so an unknown slug throws | ||
| // rather than returning { post: null } (same convention as app/blog/[...slug]). | ||
| let post: Awaited<ReturnType<typeof getBlogPost>>['post']; | ||
| try { | ||
| ({ post } = await getBlogPost('zh', slugPath)); | ||
| } catch (error) { | ||
| console.error( | ||
| `Error fetching post for Instagram image: ${slugPath}`, | ||
| error, | ||
| ); | ||
| post = null; | ||
| } | ||
| if (!post) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as the og route: this guard is unreachable because
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✅ Done, same fix. |
||
| // Unknown slug: 404 rather than render + ISR-cache a generic fallback image. | ||
| return new Response(null, { status: 404 }); | ||
| } | ||
| return renderBlogInstagramImage({ | ||
| title: post?.title ?? 'TinaCMS Blog', | ||
| author: post?.author, | ||
| title: post.title, | ||
| author: post.author, | ||
| seed: slugPath, | ||
| }); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,22 +5,36 @@ import { generateBlogStaticParams } from 'utils/blog/generateBlogStaticParams'; | |
| import { getBlogPost } from 'utils/blog/getBlogPost'; | ||
| import { renderBlogOgImage } from 'utils/og/blogOgImage'; | ||
|
|
||
| const IS_EXPORT = process.env.EXPORT_MODE === 'static'; | ||
|
|
||
| export const dynamic = 'force-static'; | ||
| export const dynamicParams = false; | ||
| export const dynamicParams = !IS_EXPORT; | ||
|
|
||
| export function generateStaticParams() { | ||
| return generateBlogStaticParams('zh'); | ||
| return IS_EXPORT ? generateBlogStaticParams('zh') : []; | ||
| } | ||
|
|
||
| export async function GET( | ||
| _request: Request, | ||
| { params }: { params: { slug: string[] } }, | ||
| ) { | ||
| const slugPath = params.slug.join('/'); | ||
| const { post } = await getBlogPost('zh', slugPath); | ||
| // Tina's client defaults to errorPolicy: 'throw', so an unknown slug throws | ||
| // rather than returning { post: null } (same convention as app/blog/[...slug]). | ||
| let post: Awaited<ReturnType<typeof getBlogPost>>['post']; | ||
| try { | ||
| ({ post } = await getBlogPost('zh', slugPath)); | ||
| } catch (error) { | ||
| console.error(`Error fetching post for OG image: ${slugPath}`, error); | ||
| post = null; | ||
| } | ||
| if (!post) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as the og route: this guard is unreachable because
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✅ Done, same fix. |
||
| // Unknown slug: 404 rather than render + ISR-cache a generic fallback image. | ||
| return new Response(null, { status: 404 }); | ||
| } | ||
| return renderBlogOgImage({ | ||
| title: post?.title ?? 'TinaCMS Blog', | ||
| author: post?.author, | ||
| title: post.title, | ||
| author: post.author, | ||
| seed: slugPath, | ||
| }); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| // Covers the fail-loud asset loading. A genuinely-absent optional asset (404) | ||
| // resolves to null so the caller can fall back (llama), while a transient load | ||
| // failure (any non-404 / network error) rejects — so the render 500s and isn't | ||
| // ISR-cached, instead of silently shipping a degraded image that sticks for the | ||
| // life of the deployment. The module memoizes at module scope and derives its | ||
| // fetch origin from env at import, so each case runs in an isolated registry. | ||
| // | ||
| // Paths under /__missing__ don't exist in public/ on disk, so loadPublic falls | ||
| // through to the (mocked) origin fetch — no fs mock needed. | ||
|
|
||
| function loadOgAssets() { | ||
| // jest 25 doesn't recognise `node:`-prefixed builtins as core modules, so the | ||
| // real fs/path can't resolve under test. doMock both (non-hoisted, so ts-jest | ||
| // 25 doesn't choke on the hoist transform) before the module loads, so | ||
| // readFileSync always misses and every load goes through the origin fetch. | ||
| jest.doMock('node:fs', () => ({ | ||
| readFileSync: () => { | ||
| throw new Error('ENOENT'); | ||
| }, | ||
| })); | ||
| jest.doMock('node:path', () => ({ join: (...p: string[]) => p.join('/') })); | ||
| // Set the production alias so the origin-fetch fallback is enabled. | ||
| process.env.VERCEL_PROJECT_PRODUCTION_URL = 'tina.io'; | ||
| let mod: typeof import('./ogAssets'); | ||
| jest.isolateModules(() => { | ||
| mod = require('./ogAssets'); | ||
| }); | ||
| return mod; | ||
| } | ||
|
|
||
| // AbortSignal.timeout exists on the Vercel runtime (Node 20+) but not jest 25's | ||
| // env; the mocked fetch ignores the signal anyway, so a no-op stub is enough. | ||
| beforeAll(() => { | ||
| if (typeof AbortSignal.timeout !== 'function') { | ||
| (AbortSignal as unknown as { timeout: () => AbortSignal }).timeout = () => | ||
| new AbortController().signal; | ||
| } | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| jest.resetModules(); | ||
| global.fetch = undefined as unknown as typeof fetch; | ||
| }); | ||
|
|
||
| describe('asset loading fail-loud semantics', () => { | ||
| it('resolves null for a genuinely-absent asset (404) so the caller can fall back', async () => { | ||
| global.fetch = jest.fn( | ||
| async () => ({ ok: false, status: 404 }) as Response, | ||
| ); | ||
| const { pngDataUri } = loadOgAssets(); | ||
| await expect(pngDataUri('/__missing__/nobody.png')).resolves.toBeNull(); | ||
| }); | ||
|
|
||
| it('rejects on a transient (non-404) fetch failure rather than degrading silently', async () => { | ||
| global.fetch = jest.fn( | ||
| async () => ({ ok: false, status: 503 }) as Response, | ||
| ); | ||
| const { pngDataUri } = loadOgAssets(); | ||
| await expect(pngDataUri('/__missing__/blip.png')).rejects.toThrow( | ||
| /asset fetch failed/i, | ||
| ); | ||
| }); | ||
| }); |
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.
Same as the og route: this guard is unreachable because
getBlogPostthrows on an unknown slug (Tina's client defaults toerrorPolicy: 'throw'), so the request 500s rather than 404s. Needs the same try/catch thatapp/blog/[...slug]/page.tsx:42-56already uses.Uh oh!
There was an error while loading. Please reload this page.
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.
✅ Done, same fix.