fix(og): lazy-load @vercel/og to keep it out of the main worker entry - #1774
Conversation
The next/og shim statically imported @vercel/og, so a top-level
`import { ImageResponse } from "next/og"` inlined ~800 KB of satori + resvg +
wasm/fonts into the always-loaded server entry. Import it via dynamic import()
inside the async stream callback so it is always a separate chunk.
app-router-cloudflare dist/server/index.js: ~1.67 MB -> ~875 KB.
commit: |
|
|
/bigbonk review for issues |
There was a problem hiding this comment.
Review: lazy-load @vercel/og to keep it out of the main worker entry
Verdict: LGTM. Clean, minimal, well-targeted change. Verified the fix works end-to-end on the PR preview deployment.
What I verified
- No behavioral regression in error handling. On
main,new VercelImageResponse(...)was already inside the asyncReadableStream.start()callback. This PR only addsawait import("@vercel/og")immediately before that existing call. So error surfacing semantics are unchanged — both before and after, a failure in@vercel/og(construction or now also the dynamic import) propagates as a stream error during body consumption, not atnew ImageResponse()construction. Good. - No resolveId loop. The
resolveIdhook atpackages/vinext/src/index.ts:2342redirects@vercel/ogimports to the shim unless the importer is the shim itself (isVinextOgShimImporter, index.ts:308). The dynamicimport("@vercel/og")insideog.tsxcorrectly resolves to the real package, not back to the shim. Confirmedog.tsxis matched by the importer check. optimizeDeps.excludealready lists@vercel/og(build-optimization.test.ts:160), so it isn't pre-bundled and the dynamic import code-splits cleanly into its own chunk.- No per-request overhead. ESM caches the module, so
import("@vercel/og")only pays the load cost once per isolate, then returns cached. - End-to-end on the preview deploy:
GET https://pr-1774-app-router-cloudflare.vinext.workers.dev/api/og?title=test→200,content-type: image/png, ~64 KB PNG. OG generation still works with the staticimport { ImageResponse } from "next/og"and lazy runtime load (workerd WASM path + base64-inlined font intact). - Tests pass:
tests/og-inline.test.ts,tests/og-font-patch.test.ts,tests/shims.test.ts(1040 passed),tests/build-optimization.test.ts(82 passed).vp checkclean on the changed file.
Minor follow-ups (non-blocking)
-
vinext:og-assetssafety net may now be skipped on the Node.js target. That plugin (packages/vinext/src/plugins/og-assets.ts:183-192) only scans<rsc-out>/index.jsfor theresvg.wasmstring before copying the WASM file. Now that@vercel/ogis code-split into a separate chunk, theresvg.wasmreference may no longer appear inindex.js, socontent.includes("resvg.wasm")could returnfalseand the disk-read fallback copy would be skipped. This only affects the Node.js fallback path — on workerd (the primary target) resvg.wasm is loaded via?moduleand bundled by Wrangler, which is exactly why the preview deploy above works. Worth a quick check that the scan covers all emitted chunks (or scans the chunk that actually references the asset), if the Node.js target is still supported for OG. -
Dead workaround left in an example. The PR's premise is that apps no longer need to manually write
await import("next/og").examples/app-router-playground/app/api/og/route.tsx:6-8still uses that workaround with a now-stale comment. Not in scope for this PR, but a candidate cleanup to demonstrate the fix.
Neither item blocks merge.
|
Review posted to PR #1774. SummaryVerdict: LGTM — clean, minimal, well-targeted change with no behavioral regression. Key findingsThe fix is correct. The change converts a static
Two non-blocking follow-ups I flagged
|
Address review feedback on the lazy @vercel/og change:
- vinext:og-assets scanned only index.js for the resvg.wasm reference to
decide whether to copy the Node.js disk-read fallback asset. Now that
@vercel/og is code-split into its own chunk, that reference is no longer in
index.js, so the copy was skipped (breaking the Node target's OG fallback).
Scan all emitted chunks via the writeBundle bundle arg instead.
- app-router-playground used a manual `await import("next/og")` workaround
that is no longer needed; switch it to the idiomatic static import.
|
Thanks @ask-bonk — addressed both follow-ups in
Targeted tests still green: og-inline, og-font-patch, shims, build-optimization (1122 passed). /bigbonk review for issues |
There was a problem hiding this comment.
Re-review of bfee2e2b — both follow-ups addressed
I re-verified both fixes end-to-end (built app-router-cloudflare and app-router-playground from this branch, ran targeted tests, vp check). LGTM — both follow-ups are correctly resolved.
1. vinext:og-assets resvg.wasm copy — fix is correct and necessary
Reproduced the regression and confirmed the fix:
- Built
app-router-cloudflareon this branch.dist/server/index.js(the only thing the old code scanned) contains 0 references toresvg.wasmand 0 satori/Resvg references — so the pre-fixcontent.includes("resvg.wasm")check onindex.jswould have returnedfalseand skipped the copy. Regression confirmed. - The og runtime is now code-split into
dist/server/_next/static/index.edge-*.js, which is where theresvg.wasmliteral lives (asnew URL("./resvg.wasm", import.meta.url)injected byvinext:og-font-patch). - The new
writeBundle(options, bundle)scan over alltype === "chunk"outputs correctly finds that nested chunk, anddist/server/resvg.wasm(1378357 bytes) is copied again. Verified present on disk after build.
One robustness note worth calling out (in the fix's favor): the scan is reliable specifically because vinext:og-font-patch (index.ts:4741-4749) always injects the new URL("./resvg.wasm", import.meta.url) string literal into the Node.js catch fallback. So even though the bundler may rewrite the import("./resvg.wasm?module") specifier, the literal resvg.wasm string is guaranteed to survive in the chunk. Good — the two transforms are consistent.
2. Dead workaround in app-router-playground — clean
app/api/og/route.tsx now uses idiomatic import { ImageResponse } from "next/og" with the stale comment removed. Built it on this branch:
index.jssatori/Resvg count = 0 (still code-split via the shim's lazy import)dist/server/resvg.wasmcopied
This is a good demonstration of the PR's premise (apps no longer need the manual await import("next/og")).
Verification summary
vp test run tests/og-font-patch.test.ts tests/og-inline.test.ts→ 19 passedvp test run tests/shims.test.ts tests/build-optimization.test.ts→ 1103 passedvp checkon both changed source files → clean (format + lint + types)
Non-blocking follow-up
There is still no unit test for the vinext:og-assets writeBundle handler (neither before nor after this PR). This regression shipped silently precisely because the chunk-scanning logic is untested — the only thing catching it is a manual example build. Per AGENTS.md ("If you move behavior out of a template string / change runtime behavior, add a focused unit test"), a small test that feeds a synthetic bundle (a chunk whose code references resvg.wasm under a non-index.js filename) and asserts referencedAssets resolves would lock in this fix and prevent a future "scan only index.js" style regression. Not blocking this PR.
|
Review posted to PR #1774. SummaryVerdict: LGTM — both follow-ups in 1.
|
Problem
The
next/ogshim (packages/vinext/src/shims/og.tsx) statically imported@vercel/og. As a result, the idiomatic Next.js usageat the top of a route module pulled the entire ~800 KB
@vercel/ogruntime (satori + resvg + embedded wasm + base64-inlined fonts) into the always-loaded server entry (dist/server/index.js), where it is parsed on every cold start — even for requests that never generate an OG image.Apps could only avoid this by manually writing
await import("next/og")(asexamples/app-router-playgrounddoes, with a code comment explaining the workaround). The framework shouldn't require that.Fix
Import
@vercel/ogvia a dynamicimport()inside the shim's already-asyncReadableStream.start()callback. The heavy runtime is now always emitted as its own code-split chunk, regardless of whether the app importsnext/ogstatically or dynamically. Type-only imports stay static (erased at build, no runtime pull-in).Impact
On
examples/app-router-cloudflare(which uses a staticimport { ImageResponse } from "next/og"),dist/server/index.jsdrops from ~1.67 MB to ~875 KB. satori/resvg move into a separate chunk; the og chunk still contains the base64-inlined font, so OG generation still works. The already-dynamicapp-router-playgroundcase is unaffected.Tests
Verified locally with targeted runs:
tests/og-inline.test.ts,tests/og-font-patch.test.ts,tests/shims.test.ts— 1040 passedtests/build-optimization.test.ts— 82 passed