fix(vite): handle dotted Nitro routes under baseURL in dev#4108
fix(vite): handle dotted Nitro routes under baseURL in dev#4108schiller-manuel wants to merge 3 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds baseURL normalization and Nitro-route detection to the Vite dev middleware so requests under a non-root base (including splat routes with dotted params) are matched as Nitro routes instead of being treated as static assets. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment |
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/vite/baseurl-dotted-param.test.ts (1)
15-29: Consider restoring the original working directory inafterAll.
process.chdir(rootDir)modifies global process state. While the test is markedsequential, restoring the cwd in the teardown would improve isolation and prevent potential side effects on subsequent test files.♻️ Suggested improvement
+ let originalCwd: string; + beforeAll(async () => { + originalCwd = process.cwd(); process.chdir(rootDir); server = await createServer({ root: rootDir }); await server.listen("0" as unknown as number); const addr = server.httpServer?.address() as { port: number; address: string; family: string; }; serverURL = `http://${addr.family === "IPv6" ? `[${addr.address}]` : addr.address}:${addr.port}`; }, 30_000); afterAll(async () => { await server?.close(); + if (originalCwd) { + process.chdir(originalCwd); + } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/vite/baseurl-dotted-param.test.ts` around lines 15 - 29, The test changes the process cwd in beforeAll via process.chdir(rootDir) but never restores it; capture the original working directory (e.g., const originalCwd = process.cwd()) before calling process.chdir in the beforeAll block and then restore it in afterAll (call process.chdir(originalCwd)) alongside closing the server in the existing afterAll to avoid leaking global state; update the beforeAll/afterAll in this test file (the hooks named beforeAll and afterAll in test/vite/baseurl-dotted-param.test.ts) accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@test/vite/baseurl-dotted-param.test.ts`:
- Around line 15-29: The test changes the process cwd in beforeAll via
process.chdir(rootDir) but never restores it; capture the original working
directory (e.g., const originalCwd = process.cwd()) before calling process.chdir
in the beforeAll block and then restore it in afterAll (call
process.chdir(originalCwd)) alongside closing the server in the existing
afterAll to avoid leaking global state; update the beforeAll/afterAll in this
test file (the hooks named beforeAll and afterAll in
test/vite/baseurl-dotted-param.test.ts) accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6033fe32-8363-4d31-9607-44c65e872049
📒 Files selected for processing (6)
src/build/vite/dev.tstest/vite/baseurl-dotted-param-fixture/api/proxy/[...param].tstest/vite/baseurl-dotted-param-fixture/index.htmltest/vite/baseurl-dotted-param-fixture/tsconfig.jsontest/vite/baseurl-dotted-param-fixture/vite.config.tstest/vite/baseurl-dotted-param.test.ts
| // No file extension (not /src/index.ts) | ||
| !req.url!.match(/\.([a-z0-9]+)(?:[?#]|$)/i)?.[1] && | ||
| // No file extension (not /src/index.ts) unless it is an explicit Nitro route | ||
| (!ext || isNitroRoute) && |
There was a problem hiding this comment.
Tests seem not covering this (pass also without change)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/build/vite/dev.ts`:
- Around line 203-207: The dev server applies withBase(nodeReq.url, baseURL) too
early which lets unbased requests like /api/proxy/... match routes that should
require the base (e.g., /subdir/api/proxy/...), causing dev/production
divergence; update the pre-check in src/build/vite/dev.ts to use the raw request
pathname (nodeReq.url or req.url) when testing route matches (i.e., do not call
withBase(...) for the initial match) so unbased URLs are rejected when
nitro.options.baseURL is set, and add a regression test (e.g., extend
test/vite/baseurl-dotted-param.test.ts) asserting that an unbased URL fails or
behaves identically to production when baseURL="/subdir".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bac1297a-d13a-4fe2-a9ce-452c0af4e2bf
📒 Files selected for processing (1)
src/build/vite/dev.ts
| const baseURL = nitro.options.baseURL || "/"; | ||
| const originalURL = nodeReq.url; | ||
| if (baseURL !== "/") { | ||
| nodeReq.url = withBase(nodeReq.url, baseURL); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the relevant sections of src/build/vite/dev.ts
cat -n src/build/vite/dev.ts | sed -n '200,250p'Repository: nitrojs/nitro
Length of output: 2188
🏁 Script executed:
# Look at the imports to understand what withBase does
cat -n src/build/vite/dev.ts | head -30Repository: nitrojs/nitro
Length of output: 1345
🏁 Script executed:
# Check the complete routing and route matching logic
cat -n src/routing.ts | head -100Repository: nitrojs/nitro
Length of output: 3879
🏁 Script executed:
# Look at how routes are registered with baseURL
rg -n "baseURL" src/routing.ts -A 3 -B 3Repository: nitrojs/nitro
Length of output: 1210
🏁 Script executed:
# Search for route registration and how baseURL is applied
rg -n "register\|baseURL" src/build/virtual.ts -A 2 -B 2Repository: nitrojs/nitro
Length of output: 117
🏁 Script executed:
# Check how Router.match() is implemented
cat -n src/routing.ts | sed -n '160,210p'Repository: nitrojs/nitro
Length of output: 1998
🏁 Script executed:
# Look for tests related to baseURL or route matching in dev
fd -e test.ts -e spec.ts -e test.js | xargs rg -l "baseURL" 2>/dev/null | head -5Repository: nitrojs/nitro
Length of output: 211
🏁 Script executed:
# Check if there are any existing tests for dev.ts
fd -e test.ts -e spec.ts -e test.js | xargs rg -l "dev.ts\|nitroDevMiddleware" 2>/dev/nullRepository: nitrojs/nitro
Length of output: 41
🏁 Script executed:
# Understand the Router.match() signature and behavior
rg -n "match\(" src/routing.ts -A 5 | head -40Repository: nitrojs/nitro
Length of output: 304
🏁 Script executed:
# Look at the baseURL dotted param test to understand the intended behavior
cat test/vite/baseurl-dotted-param.test.tsRepository: nitrojs/nitro
Length of output: 1467
🏁 Script executed:
# Check what the test expects and how baseURL should work in dev
rg -n "baseURL" test/vite/baseurl-dotted-param.test.ts -B 3 -A 3Repository: nitrojs/nitro
Length of output: 574
🏁 Script executed:
# Understand what withBase does - check ufo documentation or its usage
rg -n "withBase" src/build/vite/dev.ts -B 2 -A 2Repository: nitrojs/nitro
Length of output: 624
🏁 Script executed:
# Check if there's any discussion of this issue in git history or comments
rg -n "unbased\|diverge\|root URL\|baseURL" src/build/vite/dev.ts -B 2 -A 2Repository: nitrojs/nitro
Length of output: 39
🏁 Script executed:
# Check the fixture's nitro configuration
cat test/vite/baseurl-dotted-param-fixture/nitro.config.ts 2>/dev/null || cat test/vite/baseurl-dotted-param-fixture/nitro.config.js 2>/dev/null || ls -la test/vite/baseurl-dotted-param-fixture/Repository: nitrojs/nitro
Length of output: 402
🏁 Script executed:
# Look for API route definition in the fixture
find test/vite/baseurl-dotted-param-fixture -type f \( -name "*.ts" -o -name "*.js" \) | xargs cat 2>/dev/nullRepository: nitrojs/nitro
Length of output: 358
🏁 Script executed:
# Check ufo's withBase behavior documentation
rg -n "withBase\|export.*withBase" --type ts | head -20Repository: nitrojs/nitro
Length of output: 39
🏁 Script executed:
# Look at how withBase is used elsewhere in the codebase to understand its semantics
rg -n "withBase" src/ -B 1 -A 1 | head -50Repository: nitrojs/nitro
Length of output: 1771
🏁 Script executed:
# Let's see the complete logic after the isNitroRoute check
cat -n src/build/vite/dev.ts | sed -n '238,270p'Repository: nitrojs/nitro
Length of output: 1290
🏁 Script executed:
# Check if the test actually verifies that unbased URLs don't work
cat -n test/vite/baseurl-dotted-param.test.ts | sed -n '30,50p'Repository: nitrojs/nitro
Length of output: 627
🏁 Script executed:
# Let's see what ufo's withBase actually does
web_search withBase ufo documentationRepository: nitrojs/nitro
Length of output: 107
🏁 Script executed:
# Search for any tests that check unbased URLs should NOT work with a configured baseURL
rg -n "test\|describe" test/vite/baseurl-dotted-param.test.ts -A 10 | grep -A 10 "unbased\|without.*base\|root.*URL\|/api/proxy"Repository: nitrojs/nitro
Length of output: 39
🏁 Script executed:
# Check if there are any other test cases in the same file or related fixtures
cat -n test/vite/baseurl-dotted-param.test.tsRepository: nitrojs/nitro
Length of output: 1768
🏁 Script executed:
# Look for ufo package documentation or type definitions in node_modules or documentation
rg -n "export.*withBase" --type ts | grep -E "node_modules|types|\.d\.ts"Repository: nitrojs/nitro
Length of output: 39
🏁 Script executed:
# Check how withBase behaves by looking at its usage in scan.ts more closely
cat -n src/scan.ts | sed -n '90,100p'Repository: nitrojs/nitro
Length of output: 693
🏁 Script executed:
# Create a simple test to understand withBase behavior
python3 << 'EOF'
# Simulate withBase logic based on ufo's behavior
# withBase(path, baseURL) prepends baseURL to path
def with_base(path, base_url):
"""Simulates ufo's withBase: prepends baseURL to path"""
if not path:
return base_url or "/"
if not base_url or base_url == "/":
return path
# Remove trailing slash from base, leading slash from path is kept
base = base_url.rstrip("/")
if not path.startswith("/"):
path = "/" + path
return base + path
# Test cases
base = "/subdir"
print(f"withBase('/api/proxy/todos/X', '{base}') = {with_base('/api/proxy/todos/X', base)}")
print(f"withBase('/subdir/api/proxy/todos/X', '{base}') = {with_base('/subdir/api/proxy/todos/X', base)}")
print(f"Expected registered route path: /subdir/api/proxy/...")
print()
print("Analysis:")
print("- Routes registered at: /subdir/api/proxy/...")
print("- Unbased URL /api/proxy/... with withBase becomes: /subdir/api/proxy/...")
print("- This WILL match the registered route")
print("- Creating a dev-only alias for unbased URLs")
EOFRepository: nitrojs/nitro
Length of output: 479
🏁 Script executed:
# Check if there are any comments or related PRs discussing this issue
rg -n "pre-check\|dotted\|baseURL.*dev\|unbased" src/build/vite/dev.ts -B 2 -A 2Repository: nitrojs/nitro
Length of output: 39
🏁 Script executed:
# Look for any related issue discussions in comments
rg -n "TODO\|FIXME\|NOTE\|BUG" src/build/vite/dev.ts | head -20Repository: nitrojs/nitro
Length of output: 39
Add regression tests for unbased URLs when baseURL is configured.
The pre-check at line 244 uses withBase(req.url!, nitro.options.baseURL) before matching routes, which allows unbased URLs like /api/proxy/... to match routes registered with the baseURL prefix (e.g., /subdir/api/proxy/...). This creates a dev-only alias where both /api/proxy/... and /subdir/api/proxy/... resolve when baseURL="/subdir", diverging from production where only the prefixed path would work.
The test in test/vite/baseurl-dotted-param.test.ts only verifies baseURL-prefixed URLs (/subdir/api/proxy/...) work; it doesn't verify that unbased URLs either fail or maintain consistent behavior with production. To fix this, either: (1) check the raw pathname in the pre-check (line 244) without applying withBase(), ensuring unbased routes are rejected when baseURL is configured, or (2) add explicit test coverage documenting the intended behavior for unbased URLs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/build/vite/dev.ts` around lines 203 - 207, The dev server applies
withBase(nodeReq.url, baseURL) too early which lets unbased requests like
/api/proxy/... match routes that should require the base (e.g.,
/subdir/api/proxy/...), causing dev/production divergence; update the pre-check
in src/build/vite/dev.ts to use the raw request pathname (nodeReq.url or
req.url) when testing route matches (i.e., do not call withBase(...) for the
initial match) so unbased URLs are rejected when nitro.options.baseURL is set,
and add a regression test (e.g., extend test/vite/baseurl-dotted-param.test.ts)
asserting that an unbased URL fails or behaves identically to production when
baseURL="/subdir".
2 participants
fixes TanStack/router#6903