fix(build): emit assets referenced via new URL(...,import.meta.url) in SSR#1640
fix(build): emit assets referenced via new URL(...,import.meta.url) in SSR#1640james-elicx wants to merge 2 commits into
Conversation
…n SSR (#1346) Vite's built-in `vite:asset-import-meta-url` plugin only runs in the `client` environment, so server-side URL dependencies (e.g. an edge API route with `import(new URL('./style.css', import.meta.url).href)`) were left untransformed in the SSR/server bundle and the referenced file was never emitted. At runtime, `new URL('./style.css', import.meta.url)` resolved to a non-existent path under the build output and Node crashed with `ERR_MODULE_NOT_FOUND`. Add a `vinext:server-asset-import-meta-url` plugin that: - Runs in non-client environments only (Pages Router SSR, App Router SSR, RSC, Cloudflare worker). - In `transform`, emits each referenced asset via `this.emitFile` and rewrites the expression to a plugin-private placeholder wrapped in `new URL(..., import.meta.url)`. - In `renderChunk`, resolves the placeholder to a chunk-relative URL (e.g. `./_next/static/<hash>.css`). A relative URL is required because Vite's default `toOutputFilePathInJS` for SSR returns the root- absolute path, which `new URL(..., import.meta.url)` would resolve to `file:///_next/...` at Node runtime. Mirrors the Next.js deploy-suite fixture `test/e2e/react-version/pages/api/pages-api-edge-url-dep.js` which adds a URL dependency to an edge API route purely to validate that it does not break the build. Adds a regression test in `tests/ssr-css-assets.test.ts` that builds a Pages Router fixture with the URL-dependency pattern, asserts the CSS file is emitted under the SSR output, and that the rewritten `new URL(...)` points at a resolvable on-disk path.
|
/bigbonk review |
commit: |
There was a problem hiding this comment.
Review: fix(build): emit assets referenced via new URL(…, import.meta.url) in SSR
Overall this is a well-designed plugin that fills a real gap — Vite's built-in vite:asset-import-meta-url gates on consumer === "client", so server environments genuinely need this. The two-phase transform → renderChunk approach with a private placeholder is sound, the test is thorough, and the code is clean. A few things to address:
1. Plugin ordering conflict with og-inline-fetch-assets (bug)
Both plugins are enforce: "pre", and server-asset-import-meta-url is registered before og-inline-fetch-assets in the plugin array (index.ts:3822 vs 3825). This means server-asset-import-meta-url transforms first.
For code like:
fetch(new URL("./font.ttf", import.meta.url)).then(res => res.arrayBuffer())The new plugin's regex matches the inner new URL("./font.ttf", import.meta.url) (because \b matches at (n) and rewrites it to:
fetch(new URL("__VINEXT_SERVER_ASSET__abc123__", import.meta.url)).then(res => res.arrayBuffer())Now og-inline-fetch-assets runs and tries to match fetch(new URL("./...", import.meta.url)) — but the \.\/ in its regex no longer matches the placeholder. The OG inlining is silently skipped in all non-client environments during builds.
This matters for Cloudflare Workers where import.meta.url is "worker" and the fetch() pattern must be inlined as base64 — emitting the file and rewriting the URL won't help when import.meta.url isn't a real file URL.
Fix options:
- (a) Swap registration order: put
og-inline-fetch-assetsbeforeserver-asset-import-meta-urlso inlining wins first. The OG plugin's regex is more specific (requiresfetch(...)wrapper), so if it matches and replaces, the barenew URL(...)pattern is gone and the new plugin doesn't fire. If the OG plugin doesn't match (nofetch()wrapper), the new plugin handles it. - (b) Add a negative lookbehind to the new plugin's regex to skip
new URL(...)that's insidefetch(new URL(...)). Something like checking whether the match is preceded byfetch(. - (c) Have the new plugin skip matches where the
new URL(...)is immediately preceded byfetch(— a simplecode.slice(matchStart - 6, matchStart) === 'fetch('check in the loop body.
I'd go with (a) — it's the simplest and most robust.
2. Missing source map (nit, non-blocking)
Both transform and renderChunk return { code: result, map: null }. This means any downstream source map is invalidated. Vite's upstream uses MagicString for proper source map support.
For server-side code this is unlikely to cause real debugging pain, but if you ever want to add it: MagicString with .update(start, end, replacement) and .generateMap({ hires: "boundary" }) is the pattern used everywhere else in the codebase.
Not blocking, just noting for posterity.
3. The transform filter uses a global regex (correctness note)
const ASSET_IMPORT_META_URL_RE = /\bnew\s+URL\s*\(...\)/g;
// ...
filter: { code: ASSET_IMPORT_META_URL_RE },Passing a global (g) regex to filter.code works because Vite calls .test() which advances lastIndex, but since a new filter check happens per-module the state leak doesn't matter in practice. I see the handler also correctly creates a fresh copy via new RegExp(ASSET_IMPORT_META_URL_RE) on line 91. Just want to confirm this is intentional — it's fine, but removing the g flag from the module-level constant and only using it in the handler copy would be clearer about intent.
4. Test uses vite.build() instead of createBuilder().buildApp() — is this intentional?
The test calls viteBuild(...) directly with build.ssr set, which mirrors the Pages Router CLI path. But the AGENTS.md notes that App Router requires createBuilder() + builder.buildApp() for multi-environment builds. Is the Pages Router SSR path the only scenario that needs this fix? If App Router server components can also have new URL(...) patterns, it might be worth adding a createBuilder-based test too (or noting that the existing App Router CSS test already covers it).
5. Regex doesn't handle ../ parent traversal with multiple segments
The regex (\.\.?\/[^'"\x60]+) matches ../foo.css (single parent) and ./foo.css. But it would also match ../../deeply/nested/asset.css, which is fine. Just double-checking: \.\.? matches either . or .., and the [^'"\x60]+ eats the rest. A path like ../../../etc/passwd would match and path.resolve(moduleDir, url) would resolve it — but fs.promises.readFile would succeed only if the file exists, and it's the user's own source code, so this isn't a security concern. Just noting it's handled correctly.
Summary
The core approach is solid. The main actionable item is #1 (plugin ordering) — in the current registration order, the new plugin breaks og-inline-fetch-assets for @vercel/og assets in Cloudflare Workers builds. Swapping the registration order is a one-line fix. The rest are minor observations.
|
Review posted on PR #1640. Here's a summary of the key findings: Main issue (bug): The new Minor observations:
|
…sets The new `vinext:server-asset-import-meta-url` plugin's regex matches the inner `new URL(...)` inside the `fetch(new URL(...))` and `readFileSync(fileURLToPath(new URL(...)))` patterns that `vinext:og-inline-fetch-assets` rewrites to inline base64. Both plugins use `enforce: "pre"`, so within that bucket the order in the plugin array determines transform order. With the previous order, the new plugin rewrote the inner `new URL(...)` to a `__VINEXT_SERVER_ASSET__<id>__` placeholder before OG inlining could match its full pattern, silently breaking @vercel/og font inlining on Cloudflare Workers (where `import.meta.url` is the literal string "worker" and the only correct codepath is OG's inline-base64 IIFE). Move the new plugin after the OG plugins so OG transforms run first. The new plugin then sees the already-inlined IIFE (no remaining `new URL(...)` to match) and is a no-op on those modules. Spotted by @ask-bonk during review of #1640.
|
/bigbonk review |
|
There was a problem hiding this comment.
Re-review: fix(build): emit assets referenced via new URL(…, import.meta.url) in SSR
The main issue from the first review — plugin ordering conflict with og-inline-fetch-assets — is fixed in 10b1780. createServerAssetImportMetaUrlPlugin() is now registered after createOgInlineFetchAssetsPlugin() at index.ts:3834, and the 6-line comment at lines 3828–3833 documents the ordering constraint clearly. The OG plugin's more-specific fetch(new URL(...)) regex gets first pass, so if it matches and inlines as base64, the bare new URL(...) is gone before the new plugin sees it. Correct.
Walked through the rest of the plugin a second time — no new issues.
transform phase: The two-pass design (read file → emitFile → rewrite to placeholder) is clean. The @vite-ignore check correctly scans the slice between new URL( and the opening quote. The readFile catch-and-skip for missing files is the right fallback. The new RegExp(ASSET_IMPORT_META_URL_RE) copy in the handler avoids lastIndex leaking from the filter.code check.
renderChunk phase: The JSON.stringify(replacement).slice(1, -1) substitution is correct — the placeholder sits inside an already-quoted string literal (produced by JSON.stringify in transform), so we need to replace only the inner content. The path.posix relative-path computation avoids the Vite SSR default of root-absolute paths that would resolve to file:///_next/... at runtime.
PLACEHOLDER_RE: [\w$-] is slightly broader than needed (both Rollup and Rolldown replace - with $ in reference IDs, so - never appears), but the prefix/suffix anchoring prevents false matches. Matches Vite's own [\w$] pattern in spirit.
Test: The regression test is well-structured — creates the exact Next.js fixture pattern (import(new URL('./style.css', import.meta.url).href)), runs a Pages Router SSR build, and verifies both the rewritten URL shape and that the emitted file exists on disk. The assertions are specific enough to catch the failure mode described in #1346.
Minor nits from first review, status:
- Source maps (
map: null): Still null, still fine for server code. Noted, not blocking. - Global
gflag on module-level constant: Harmless, handler makes a fresh copy. Noted, not blocking. - App Router
createBuildertest coverage: The existing App Router CSS test (every CSS import in dist/server/ssr/*.js resolves to a real file on disk) exercises thecreateBuilderpath, and the new plugin'sapplyToEnvironmentcovers all non-client envs, so the gap is small. Not blocking.
LGTM — the ordering fix was the only actionable item from the first review and it's addressed.
|
Review posted on PR #1640. Here's the summary: Approved. The main actionable item from the first review — the plugin ordering conflict with The rest of the plugin holds up on re-review:
No new issues found. The minor nits from the first review (source maps, global regex flag, App Router test coverage) remain non-blocking observations. |
Summary
Fixes #1346.
Vite's built-in
vite:asset-import-meta-urlplugin only runs in theclientenvironment, so server-side URL dependencies (e.g. an edge API route withimport(new URL('./style.css', import.meta.url).href)) were left untransformed in the SSR/server bundle and the referenced file was never emitted. At runtime,new URL('./style.css', import.meta.url)resolved to a non-existent path under the build output and Node crashed withERR_MODULE_NOT_FOUND.Adds a
vinext:server-asset-import-meta-urlplugin that:transform, emits each referenced asset viathis.emitFileand rewrites the expression to a plugin-private placeholder wrapped innew URL(..., import.meta.url).renderChunk, resolves the placeholder to a chunk-relative URL (e.g../_next/static/<hash>.css). A relative URL is required because Vite's defaulttoOutputFilePathInJSfor SSR returns the root-absolute path, whichnew URL(..., import.meta.url)would resolve tofile:///_next/...at Node runtime.Mirrors the Next.js deploy-suite fixture
test/e2e/react-version/pages/api/pages-api-edge-url-dep.jswhich adds a URL dependency to an edge API route purely to validate that it does not break the build.Test plan
tests/ssr-css-assets.test.tsthat builds a Pages Router fixture with the URL-dependency pattern, asserts the CSS file is emitted under the SSR output, and that the rewrittennew URL(...)points at a resolvable on-disk path.tests/ssr-css-assets.test.ts,tests/pages-router.test.ts,tests/app-router.test.ts,tests/font-google.test.tsall green.Notes / follow-ups
new URL("./X", import.meta.url)pattern. Template-literal forms with${...}interpolation (which Vite handles viaimport.meta.globfor client builds) are not covered — that path is rarely useful in server code; we can extend if a fixture requires it./* @vite-ignore */to match Vite's upstream contract.