Skip to content

Commit ac26576

Browse files
pi0claude
andcommitted
fix(vite): route asset-tagged requests to opaque catch-alls in dev
Routes owned by a custom server entry or the SSR renderer are invisible to `nitro.routing.routes`, so the dev middleware pre-empted asset-tagged requests to them with `_nitroHandled` and they could never run (#4252). Dispatch such requests to nitro after Vite declines and decide from the response instead: a 2xx page/data content-type means the catch-all swallowed a missing asset (#4234) and falls through to the 404; anything else passes through verbatim. User-file root catch-alls keep the existing divert. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 7af4fee commit ac26576

7 files changed

Lines changed: 361 additions & 4 deletions

File tree

.agents/vite-dev.md

Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,187 @@
1+
# Vite Dev Middleware — Request Routing
2+
3+
> Scope: the request-routing logic in `src/build/vite/dev.ts` — the two
4+
> `server.middlewares.use(...)` registrations (`nitroDevMiddlewarePre` and the
5+
> catch-all `nitroDevMiddleware`): why the current design looks the way it does.
6+
7+
## 1. Where it lives & how it is wired
8+
9+
- Logic: `src/build/vite/dev.ts``configureViteDevServer(ctx, server)`.
10+
- Entry: `src/build/vite/plugin.ts:266` — the plugin's `configureServer` hook
11+
`return`s the result of `configureViteDevServer`.
12+
13+
Vite's `configureServer` contract: middleware registered **synchronously**
14+
inside the hook runs **before** Vite's internal middlewares; a function
15+
**returned** from the hook runs **after** them (the "post" hook). Nitro uses
16+
both ends:
17+
18+
| Registration | Site | Runs | Role |
19+
|---|---|---|---|
20+
| `nitroDevMiddlewarePre` (`function` form) | `dev.ts:281` | **before** Vite static/transform | Classifier. Route explicit-Nitro + definite navigations to Nitro immediately; let definite assets fall through to Vite, marking them `_nitroHandled` (transparent catch-all) or `_nitroAssetCheck` (opaque catch-all / no match). |
21+
| `nitroDevMiddleware` | `dev.ts:374`, inside the returned `() => { ... }` | **after** Vite static/transform | Catch-all fallback. Wraps req as web `Request`, tries `ctx.devApp.fetch` then `nitroEnv.dispatchFetch`, honoring `baseURL`; inspects the response for `_nitroAssetCheck` requests. Skipped for `_nitroHandled`. |
22+
23+
**Why two, and why pre?** Without the pre-pass, Vite's static/transform
24+
middleware serves files from the project root and would answer server routes
25+
before Nitro sees them (see upstream **vitejs/vite#20866**, which made Vite
26+
consult `sec-fetch-dest` to let document requests fall through — the same
27+
header Nitro's classifier leans on).
28+
29+
Facts about Vite's side (verified against vite@8.1.4): Nitro forces
30+
`appType: "custom"` (`plugin.ts:199`), so Vite registers none of
31+
`htmlFallback`/`indexHtml`/`notFound` middlewares, and **every plain miss
32+
`next()`s** (sirv and transformMiddleware never self-emit a 404 for a missing
33+
file/module). The only terminal 404 is connect's `finalhandler`, which sits
34+
**after** the Nitro post-hook. So Vite genuinely hands off every request it
35+
cannot serve — the post catch-all is the last handler before `finalhandler`.
36+
37+
## 2. Classification
38+
39+
`nitro.routing.routes` in an SSR app **always** contains a catch-all `/**`, so
40+
"does a Nitro route match?" is nearly always *yes*. The design distinguishes:
41+
42+
- **Explicit route**`route` set, `!== "/**"`, not `startsWith("/**:")` (a
43+
prefixed splat like `/api/photos/**` is explicit). Deterministic → **always
44+
Nitro**, no heuristic may override (#4108, #4241, #4252, #4270).
45+
- **Explicit public asset** — a public-asset dir under a non-root `baseURL`
46+
with `fallthrough: false` owns its subtree → Nitro, like an explicit route.
47+
- **Transparent catch-all** — a *user route file* at root level
48+
(`routes/[...].ts``/**`, `routes/[...slug].ts``/**:slug`). Nitro sees
49+
everything it can handle, so Vite stays the definitive asset handler: an
50+
asset-tagged request is marked `_nitroHandled` and a Vite miss must **not**
51+
fall back into it (#4234/#4266).
52+
- **Opaque catch-all** — the SSR renderer `/**` or a custom `serverEntry` `/**`
53+
(both registered in `routing.ts` with identifiable `handler` paths). Their
54+
*real* routes are invisible to the host (`server.ts` H3 app routes, framework
55+
routers like TanStack Start), so an asset-tagged miss from Vite must still be
56+
**dispatched** — marked `_nitroAssetCheck`, decided by response inspection.
57+
- **None** — no match. Extensionless → page navigation → Nitro; asset-tagged →
58+
same as opaque (`_nitroAssetCheck`, dispatch after Vite).
59+
60+
## 3. The asset heuristic (catch-all / none case only)
61+
62+
Set `Vary: sec-fetch-dest, accept`, then:
63+
64+
- **`Sec-Fetch-Dest` present & concrete** (not `empty`): `document`/`iframe`/
65+
`frame` = navigation → Nitro; anything else (`image`, `video`, `style`, …) =
66+
asset.
67+
- **Absent or `empty`**: fall back to extension — `ASSET_EXT_RE.test(ext)`
68+
**and** no `text/html` in `Accept` ⇒ asset. (`empty` = fetch/XHR is ambiguous:
69+
it tags both API calls and `fetch()`ed assets.)
70+
- Non-asset + (matched or extensionless) → Nitro immediately (pre-Vite).
71+
72+
Two regexes to keep intact:
73+
- `ASSET_EXT_RE` (`dev.ts:24`) — narrow on purpose, so dotted Nitro params like
74+
`/foo.bar.1` still reach Nitro (#4108).
75+
- Extension is extracted from the path only (query/hash stripped) so
76+
`?file=bar.png` does not misclassify.
77+
78+
## 4. Response inspection (`_nitroAssetCheck`)
79+
80+
An asset-tagged request that only an opaque catch-all could handle cannot be
81+
classified **a priori**`/image.png` may be a real custom-entry route (#4252)
82+
or a genuinely missing asset that a naive SSR `/**` would render as a 200 page
83+
(#4234). It *can* be classified from the **response**: after Vite declines and
84+
the post catch-all dispatches to Nitro, a 2xx with a page-ish content-type
85+
(`PAGE_CONTENT_RE`, `dev.ts:33`: `text/html` | `application/json`) means the
86+
catch-all swallowed a missing asset → the response is discarded via `next()`
87+
(connect `finalhandler` 404, same as before). Anything else — real asset types,
88+
`text/plain`, no content-type, non-2xx (framework 404 pages, redirects) —
89+
passes through verbatim.
90+
91+
Deny-list rationale (all verified empirically):
92+
93+
- `application/json` is included because a naive SSR entry can swallow with
94+
JSON, not just HTML (the `app-fixture` entry does exactly that).
95+
- `text/plain` is excluded because a bare string returned from an h3 handler
96+
has **no** content-type at the h3 layer but crosses the worker bridge as a
97+
`Response` with the fetch-spec default `text/plain;charset=UTF-8` — it must
98+
pass through (custom-entry handlers returning strings).
99+
- Transparent (user-file) catch-alls can **not** use inspection instead of the
100+
divert: their string 200s arrive with no distinguishing content-type.
101+
102+
This keeps everything zero-config, host-side only (no runtime/bundle change,
103+
no production behavior change). Known leftover: **production** SSR still
104+
renders 200 HTML for missing-asset URLs — pre-existing behavior; changing it
105+
is a separate, deliberate breaking-change discussion.
106+
107+
## 5. Issue / PR lineage (chronological)
108+
109+
- **#3649** first crude "has extension ⇒ Vite" rule; **#3804/#3805/#3817**
110+
middleware improvements, mounted-path skip, internal-prefix skip (survives as
111+
the `^\/(?:__|@)` guard); **#4098** `sec-websocket-protocol` to tell Vite HMR
112+
sockets from Nitro websockets (the `upgrade` handler).
113+
- **#4108** baseURL-aware matching for dotted Nitro routes
114+
(TanStack/router#6903).
115+
- **#4234** non-loopback plain-HTTP origins omit `Sec-Fetch-*` → splat swallowed
116+
`<script src>` loads. Fixed by **#4238**: `Accept` + `ASSET_EXT_RE` fallback +
117+
the `_nitroHandled` marker.
118+
- **#4241** #4238 over-eager: `sec-fetch-dest: image` on a real route
119+
(`/api/image`) sent to Vite → 404. Fixed by `7d49dcae` + `08f2ec69`
120+
(query-string stripped before extension matching).
121+
- **#4252 / #4270** even explicit routes lost when the URL had an asset
122+
extension. Fixed by **#4272** (`5b7e152b`): explicit vs catch-all vs none
123+
classification — **an explicit route is a deterministic win no heuristic can
124+
touch**.
125+
- `7765bcb7` added `isExplicitPublicAsset`.
126+
- **#4252 follow-ups** (katywings, jantimon/TanStack/router#7403): routes the
127+
classifier cannot see — a custom `server.ts` H3 app serving `/image.png`, or
128+
an SSR framework serving assets — were pre-empted by `_nitroHandled` and
129+
could never run. Fixed by the opaque-catch-all + response-inspection design
130+
above (§2, §4): the pre-emption now applies only to transparent user-file
131+
catch-alls, and opaque dispatches are judged by their response content-type.
132+
133+
## 6. Runtime dispatch flow
134+
135+
The catch-all `nitroDevMiddleware` dispatches in two stages:
136+
137+
1. **`ctx.devApp.fetch(req)`**`NitroDevApp` (`src/dev/app.ts`), host-side:
138+
`devHandlers`, `/_vfs/**`, `/_nitro/tasks`, public asset dirs, `devProxy`.
139+
No catch-all → a 404 means "not mine, hand off"; any non-404 is returned
140+
directly (never inspected — deterministic host serves are authoritative).
141+
2. **`nitroEnv.dispatchFetch(req)`** → env-runner → worker → `nitroApp.fetch`
142+
full route table, middleware, catch-alls.
143+
144+
Catch-all registration (`src/routing.ts`): a custom `serverEntry` is pushed as
145+
`/**` with `handler = nitro.options.serverEntry.handler`; the SSR renderer as
146+
`/**` with `handler = nitro.options.renderer.handler` (set by `plugin.ts`
147+
`configResolved` to `internal/vite/ssr-renderer` when an `ssr` service exists).
148+
These handler paths are how the pre-pass identifies opaque catch-alls.
149+
150+
## 7. Test coverage map
151+
152+
All under `test/vite/`; run via `test:rollup` and `test:rolldown`. Each starts
153+
a real Vite dev server and `fetch()`es with hand-set headers.
154+
155+
- `app.test.ts` (`app-fixture/`) — SSR `/**` path. #4234 swallow contracts
156+
(missing `.css`/`.js` under `style`/absent/`empty` dests must not 200 — the
157+
fixture entry swallows with **JSON**, keeping the deny-list honest), #4252
158+
deliberate asset serve (`/dynamic-asset.png``image/png` passes
159+
inspection), `HTTPError` propagation, navigations, storage/config sharing.
160+
- `server-entry.test.ts` (`server-entry-fixture/`) — #4252 custom `server.ts`
161+
H3 app: asset-extensioned routes reachable under `image`/absent/`script`
162+
dests (including a no-content-type string return), missing-asset 404
163+
contract, navigation.
164+
- `root-wildcard.test.ts` — transparent root catch-all `/**:path`: must never
165+
200 for `/entry-client.ts` under `script`/`style`/`image`/absent dests
166+
(#4234/#4266), navigations still reach it.
167+
- `baseurl-dotted-param.test.ts` — explicit splat routes under `baseURL:
168+
/subdir/`: #4241, #4252, #4270, query-string extension, unmatched asset →
169+
Vite.
170+
- Tangential: `hmr.test.ts` (existing module served by Vite under `script`
171+
dest), `openapi.test.ts` (explicit routes), others (build/env, not routing).
172+
173+
## 8. Gotchas
174+
175+
- The `server.middlewares.stack` scan in `nitroDevMiddleware` skips requests
176+
whose URL starts with any other middleware's mounted `base` (#3805).
177+
- `_nitroHandled` is a one-way latch set by the pre-pass (transparent
178+
catch-all asset) and by the post catch-all itself (re-entry guard). Never set
179+
it for a request a real or opaque handler should still see (#4252 root
180+
cause).
181+
- Extension detection must stay path-only (strip `?#`) and `ASSET_EXT_RE` must
182+
stay narrow, or dotted Nitro params regress (#4108).
183+
- `PAGE_CONTENT_RE` must not grow `text/plain` (bridge default for string
184+
returns) and inspection must only apply to `envRes.ok` — framework 404 pages
185+
and redirects pass through verbatim.
186+
- The websocket `upgrade` handler is a separate concern sharing the "Vite's or
187+
Nitro's?" theme.

src/build/vite/dev.ts

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,14 @@ import { getEnvRunner } from "./env.ts";
2424
const ASSET_EXT_RE =
2525
/^(?:[jt]sx?|mjs|cjs|css|s[ac]ss|less|styl|vue|svelte|astro|mdx?|map|wasm|png|jpe?g|gif|svg|webp|avif|ico|bmp|woff2?|ttf|otf|eot|mp[34]|webm|wav|ogg|m4a)$/i;
2626

27+
// Content types that mean "a page/data response was rendered" rather than "an asset was served".
28+
// When an asset-tagged request that only an opaque catch-all (SSR renderer / custom server entry)
29+
// could handle comes back with one of these, the catch-all swallowed a genuinely missing asset
30+
// (#4234) and the response is discarded in favor of a plain 404. `text/plain` is deliberately
31+
// not included: a bare string returned from a handler arrives with the fetch-spec default
32+
// `text/plain;charset=UTF-8` and must pass through.
33+
const PAGE_CONTENT_RE = /^(?:text\/html|application\/json)\b/i;
34+
2735
// workerd built-in module namespaces (`cloudflare:workers`, `cloudflare:sockets`, `workerd:...`).
2836
// These are provided natively by the runtime and have no host-side representation, so they must be
2937
// externalized for the in-worker module runner to `import()` them directly instead of being fetched
@@ -206,7 +214,7 @@ export async function configureViteDevServer(ctx: NitroPluginContext, server: Vi
206214
});
207215

208216
const nitroDevMiddleware = async (
209-
nodeReq: IncomingMessage & { _nitroHandled?: boolean },
217+
nodeReq: IncomingMessage & { _nitroHandled?: boolean; _nitroAssetCheck?: boolean },
210218
nodeRes: ServerResponse,
211219
next: (error?: unknown) => void
212220
) => {
@@ -246,6 +254,18 @@ export async function configureViteDevServer(ctx: NitroPluginContext, server: Vi
246254
if (nodeRes.writableEnded || nodeRes.headersSent) {
247255
return;
248256
}
257+
// An asset-tagged request Vite already declined that only an opaque catch-all could
258+
// handle: a page/data response means the catch-all swallowed a missing asset (#4234) —
259+
// fall through to the 404 instead. A deliberate asset serve (any other or no
260+
// content-type) passes through untouched (#4252).
261+
if (
262+
nodeReq._nitroAssetCheck &&
263+
envRes.ok &&
264+
PAGE_CONTENT_RE.test(envRes.headers.get("content-type") || "")
265+
) {
266+
await envRes.body?.cancel();
267+
return next();
268+
}
249269
return await sendNodeResponse(nodeRes, envRes);
250270
} catch (error) {
251271
return next(error);
@@ -323,9 +343,29 @@ export async function configureViteDevServer(ctx: NitroPluginContext, server: Vi
323343
}
324344

325345
if (isAsset) {
326-
// Vite is the definitive handler — mark the request so the catch-all `nitroDevMiddleware`
327-
// registered after Vite doesn't fall back into a splat Nitro route on a 404.
328-
(req as IncomingMessage & { _nitroHandled?: boolean })._nitroHandled = true;
346+
// Opaque catch-alls (the SSR renderer and a custom server entry) route requests Nitro
347+
// cannot see in `nitro.routing.routes` (#4252), so an asset-tagged miss from Vite must
348+
// still be dispatched — the response content-type then decides (`_nitroAssetCheck`).
349+
// A user-file root catch-all is transparent: Nitro sees everything it can handle, so
350+
// Vite stays the definitive asset handler and a Vite miss must not fall back into it.
351+
const opaqueHandlers = new Set(
352+
[
353+
nitro.options.renderer?.handler,
354+
nitro.options.serverEntry && nitro.options.serverEntry.handler,
355+
].filter(Boolean)
356+
);
357+
const onlyOpaqueCatchAll = matchedHandlers.every(
358+
(h) => h?.handler && opaqueHandlers.has(h.handler)
359+
);
360+
const nodeReq = req as IncomingMessage & {
361+
_nitroHandled?: boolean;
362+
_nitroAssetCheck?: boolean;
363+
};
364+
if (onlyOpaqueCatchAll) {
365+
nodeReq._nitroAssetCheck = true;
366+
} else {
367+
nodeReq._nitroHandled = true;
368+
}
329369
}
330370
next();
331371
});

test/vite/app-fixture/app/entry-server.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@ export default {
77
if (req.url.includes("?error")) {
88
throw new HTTPError({ status: 418, headers: { "x-test": "123" } });
99
}
10+
if (new URL(req.url).pathname === "/dynamic-asset.png") {
11+
return new Response("PNGDATA", { headers: { "content-type": "image/png" } });
12+
}
1013
const storage = useStorage();
1114
const config = useRuntimeConfig();
1215
await storage.set("test:key", "value-from-ssr");

test/vite/app.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,23 @@ describe("vite:app", () => {
8282
expect(res.status).not.toBe(200);
8383
});
8484

85+
// #4252: an asset-tagged request the SSR catch-all deliberately serves (non-page content-type)
86+
// must reach the renderer and pass through, even though the URL looks like an asset.
87+
test("SSR catch-all can serve asset-tagged requests", async () => {
88+
for (const headers of [
89+
{ "sec-fetch-dest": "image", accept: "image/*" },
90+
{ accept: "*/*" },
91+
] as Record<string, string>[]) {
92+
const res = await fetch(`${serverURL}/dynamic-asset.png`, {
93+
headers,
94+
redirect: "manual",
95+
});
96+
expect(res.status, JSON.stringify(headers)).toBe(200);
97+
expect(res.headers.get("content-type")).toBe("image/png");
98+
expect(await res.text()).toBe("PNGDATA");
99+
}
100+
});
101+
85102
// HTTPError thrown from the SSR entry must propagate to the nitro app so the h3
86103
// error handler preserves its status and headers (consistent with production).
87104
test("propagates HTTPError status and headers from the SSR entry", async () => {
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import { H3 } from "h3";
2+
3+
const app = new H3();
4+
5+
app.get("/", (event) => {
6+
event.res.headers.set("content-type", "text/html");
7+
return `<img src="/image.png" />`;
8+
});
9+
10+
app.get("/image.png", (event) => {
11+
event.res.headers.set("content-type", "image/png");
12+
return "PNGDATA";
13+
});
14+
15+
app.get("/generated.js", () => "console.log('generated')");
16+
17+
export default app;
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import { defineConfig } from "vite";
2+
import { nitro } from "nitro/vite";
3+
4+
export default defineConfig({
5+
plugins: [nitro()],
6+
});

0 commit comments

Comments
 (0)