Skip to content

fix(dev): route asset-extensioned and query-suffixed URLs through Nitro when a route matches#4277

Closed
harshagarwalnyu wants to merge 3 commits into
nitrojs:mainfrom
harshagarwalnyu:fix/dev-middleware-asset-ext-swallows-routes
Closed

fix(dev): route asset-extensioned and query-suffixed URLs through Nitro when a route matches#4277
harshagarwalnyu wants to merge 3 commits into
nitrojs:mainfrom
harshagarwalnyu:fix/dev-middleware-asset-ext-swallows-routes

Conversation

@harshagarwalnyu
Copy link
Copy Markdown

Problem

Two related regressions in nitroDevMiddlewarePre (introduced around #4238):

  1. Asset-extension routing (Vite dev middleware swallows asset-extensioned URLs #4252, Regression - Sec-fetch-dest: image header causes 404 on API-served images (regression in 3.0.260429-beta) #4241): URLs with asset-like extensions (.jpg, .png, …) are intercepted by Vite's static middleware even when an explicit Nitro route matches. <img src="/api/photos/12345.jpg"> with sec-fetch-dest: image returns a 404 from serve-static instead of reaching the handler.

  2. Vite query suffix externalization: Imports with Vite-specific query suffixes (?url, ?raw, ?inline, ?worker) in SSR environments are incorrectly externalized rather than being routed through Vite's asset plugin pipeline.

Root Cause

Issue 1 (configureViteDevServer):
The current nitroWins logic applies the asset-extension guard uniformly to all Nitro routes. A root-level wildcard (/**) can legitimately swallow Vite's own <script src=".../entry.ts"> requests, so it needs the guard. But a prefixed route like /api/photos/** never matches Vite-managed paths — the guard should not apply to it.

Issue 2 (FetchableDevEnvironment.fetchModule; env.ts):
Imports with Vite query suffixes (?url etc.) are treated as bare module imports and fall through to the external resolver, which marks them as external and bypasses Vite's asset plugins entirely.

Fix

Routing fix

Extracts the matched route pattern from nitroRouteMatch and checks whether it is a root wildcard (/** / /**:). Only root wildcards get the asset-extension guard; all other explicit routes always win:

const isRootWildcard =
  matchedRoutePattern === "/**" ||
  matchedRoutePattern?.startsWith("/**:");
const nitroWins = isNitroRoute && (!isRootWildcard || !(isAssetByExt && treatAsAsset));

Query import fix

  • Adds ?url|raw|inline|worker to noExternal so the bundler does not externalize these imports.
  • In fetchModule, detects hasQuery and forces query-suffixed imports through resolveIdtransformRequest even when preventExternalize is not set.

Tests

Seven integration tests in test/vite/baseurl-dotted-param.test.ts cover:

  • Dotted path params (e.g. v1.2.3)
  • Extensionless URLs with sec-fetch-dest: image
  • Prefixed splat + asset extension (/api/** + .jpg)
  • Root wildcard does NOT swallow Vite asset serves
  • Root wildcard DOES handle non-asset extensions
  • Query string with asset extension
  • Accept: text/html navigation override

All 7 pass. pnpm fmt and pnpm typecheck clean.

Related

Asset-extensioned URLs (e.g. /api/photos/12345.jpg) were routed to Vite's
static middleware instead of the matching Nitro handler. The ASSET_EXT_RE guard
in nitroDevMiddlewarePre applied unconditionally to any matched route, including
specific prefixed catch-alls like /api/photos/**.

The guard was introduced (nitrojs#4234) to prevent a root-level renderer or user
catch-all (/**) from swallowing Vite's own <script>/<link> serves on plain-HTTP
non-loopback origins where Sec-Fetch-Dest is absent.

Fix: narrow the guard so the asset-extension filter only applies when the matched
Nitro route pattern is /** or /**:param (named root catch-all). Specific routes
(/api/**, /api/photos/**) are never registered on paths Vite owns and should
always win — restoring the behaviour from nitro@3.0.1-alpha.2.

When the matched route's pattern is unreadable, default to false (let the route
win) rather than falling back to the asset-extension guard.

Closes nitrojs#4252
Prevent imports with Vite-specific query suffixes (?url, ?raw, ?inline, ?worker)
from being externalized in the dev environment. Adds them to noExternal and
intercepts them in fetchModule so they are processed by Vite asset plugins.
@harshagarwalnyu harshagarwalnyu requested a review from pi0 as a code owner May 21, 2026 16:44
Copilot AI review requested due to automatic review settings May 21, 2026 16:44
@vercel
Copy link
Copy Markdown

vercel Bot commented May 21, 2026

@harshagarwalnyu is attempting to deploy a commit to the Nitro Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 21, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 975ef560-f79a-4087-b395-585b668e3753

📥 Commits

Reviewing files that changed from the base of the PR and between 971d054 and 34ead56.

📒 Files selected for processing (1)
  • src/build/vite/dev.ts

📝 Walkthrough

Walkthrough

Refines Vite dev behavior: treat Vite query-suffixed imports as non-external and resolvable by the plugin container; change nitroDevMiddlewarePre to match Nitro routes by pathname so explicit Nitro routes win over Vite asset heuristics while root wildcards retain asset suppression.

Changes

Asset-Extensioned Route Matching Fix

Layer / File(s) Summary
Module resolution noExternal configuration
src/build/vite/env.ts
Adds regex matcher to dev-mode resolve.noExternal list to identify Vite-resolved module requests with query suffixes (?url, ?raw, ?inline, ?worker) as non-external.
FetchableDevEnvironment query-suffix interception
src/build/vite/dev.ts
Updates fetchModule to intercept module imports with Vite query suffixes (in addition to preventExternalize), resolving them via pluginContainer.resolveId and accepting the result when non-external or when the original request included a Vite query suffix.
Dev middleware route precedence logic
src/build/vite/dev.ts
Refactors nitroDevMiddlewarePre to compute a request pathname, match Nitro routes to derive the matched pattern (explicit vs root-wildcard), and set nitroWins so explicit Nitro routes handle asset-tagged requests while preserving root-wildcard suppression for asset loads.
Test fixture and route-precedence assertions
test/vite/baseurl-dotted-param-fixture/routes/[...path].ts, test/vite/baseurl-dotted-param.test.ts
Adds a root-wildcard route fixture and replaces a prior test with three assertions validating that prefixed splat Nitro routes override Vite asset handling, root wildcards preserve asset suppression, and root wildcards swallow non-asset URLs.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • nitrojs/nitro#3817: Modifies src/build/vite/dev.ts nitroDevMiddlewarePre routing logic to adjust when the dev middleware should be skipped.
  • nitrojs/nitro#3854: Changes dev resolve.noExternal/external dependency handling in src/build/vite/env.ts (related to treating certain imports as non-external).
  • nitrojs/nitro#4238: Adjusts dev middleware routing and tests around asset request handling when sec-fetch-dest is absent.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title follows conventional commits format with 'fix' type and clear scope (dev), describing the main change about routing asset-extensioned and query-suffixed URLs through Nitro when a route matches.
Description check ✅ Passed The PR description comprehensively explains both problems, root causes, the implemented fixes, and test coverage, all directly related to the code changes in the changeset.
Linked Issues check ✅ Passed The PR fully addresses the coding requirements from #4252: it restores matching Nitro route behavior for asset-extensioned URLs, avoids unconditional req._nitroHandled poisoning, and adds query-suffix handling through noExternal and fetchModule interception.
Out of Scope Changes check ✅ Passed All changes directly address the stated objectives: routing fixes in dev.ts and env.ts, test fixture and test suite additions for validation, with no extraneous modifications.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch fix/dev-middleware-asset-ext-swallows-routes

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR adjusts Vite dev-mode routing/externalization behavior so Vite query-suffixed imports (e.g. ?url) and Nitro wildcard routes interact correctly under baseURL, and adds regression coverage for the updated routing rules.

Changes:

  • Add dev-env interception for query-suffixed module IDs so Vite asset plugins handle them rather than SSR externalization.
  • Refine dev server routing so root-level wildcards don’t swallow Vite asset-extension requests, while prefixed splat Nitro routes can still win.
  • Expand test coverage and add a root catch-all Nitro fixture route for routing regressions.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
test/vite/baseurl-dotted-param.test.ts Updates/adds regression tests for Nitro vs Vite routing under baseURL (root wildcard vs prefixed splat behavior).
test/vite/baseurl-dotted-param-fixture/routes/[...path].ts Adds a root wildcard Nitro route fixture used to validate wildcard routing behavior.
src/build/vite/env.ts Extends the noExternal configuration to account for Vite query suffixes.
src/build/vite/dev.ts Adjusts module fetching/externalization logic for query IDs and refines Nitro-vs-Vite routing decisions for wildcard routes.

Comment thread src/build/vite/dev.ts Outdated
Comment on lines 80 to 93
// We also intercept imports with Vite-specific query suffixes (like ?url)
// to ensure they are handled by Vite's asset plugins instead of being externalized.
const hasQuery = id.includes("?");
if (
this.#preventExternalize &&
(this.#preventExternalize || hasQuery) &&
!id.startsWith("file://") &&
importer &&
id[0] !== "." &&
id[0] !== "/"
(hasQuery || (id[0] !== "." && id[0] !== "/"))
) {
const resolved = await this.pluginContainer.resolveId(id, importer);
if (resolved && !resolved.external) {
if (resolved && (!resolved.external || hasQuery)) {
return super.fetchModule(resolved.id, importer, options);
}
}
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 34ead56: extracted VITE_QUERY_RE = /\?(url|raw|inline|worker)(?:&|$)/ as a module-level constant and replaced hasQuery with hasViteQuery, keeping the behavior consistent with the noExternal pattern in env.ts.

@pi0
Copy link
Copy Markdown
Member

pi0 commented May 21, 2026

Have you seen #4272 ? what is diffenet from #4266 (comment) ?

@pi0
Copy link
Copy Markdown
Member

pi0 commented May 22, 2026

Closing as

  1. pr is stalled
  2. it very much looks like AI code without human in loop
  3. likely has not been tested against latest nightly or lacks reproduction of what is remaining

@pi0 pi0 closed this May 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Vite dev middleware swallows asset-extensioned URLs

3 participants