feat: compat with Nitro 3#852
Conversation
commit: |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #852 +/- ##
==========================================
+ Coverage 68.01% 68.68% +0.67%
==========================================
Files 9 9
Lines 297 297
Branches 67 67
==========================================
+ Hits 202 204 +2
+ Misses 78 76 -2
Partials 17 17 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
📝 WalkthroughWalkthroughThis change modifies font asset handling in src/assets.ts. It adds a basename import from pathe, updates the dev event handler to derive the requested font filename using basename(event.path) rather than string slicing, broadens the dev server handler route to match all nested paths under the assets base URL, and sets a conditional fallthrough option on the Nitro publicAssets configuration based on the dev mode flag. Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant DevServerHandler
participant devEventHandler
Browser->>DevServerHandler: request asset path (**)
DevServerHandler->>devEventHandler: forward event
devEventHandler->>devEventHandler: basename(event.path)
devEventHandler-->>Browser: return matched font file
Related issues: None specified. Related PRs: None specified. Suggested labels: bug, assets Suggested reviewers: None specified. Poem 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/assets.ts`:
- Around line 32-33: The font lookup in the assets handler is using event.path
directly, so query strings from h3 requests can make basename(...) miss cached
entries and cause false 404s. Update the logic in the assets.ts flow around the
filename/url lookup to derive the name from event.url.pathname, or otherwise
strip event.url.search before calling basename, so renderedFontURLs keys match
requests like font.woff2?v=1.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| const filename = basename(event.path) | ||
| const url = context.renderedFontURLs.get(filename) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does h3's event.path getter include the query string from the request URL?
💡 Result:
Yes, the event.path getter in h3 includes the query string from the request URL [1]. In H3, event.path provides the full path of the incoming request, including both the path and the query parameters (for example, "/test?test=123") [1]. However, please note that event.path is deprecated in H3 v2 [2]. It is recommended to use event.url.pathname and event.url.search instead to access the path and query string components separately [2].
Citations:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant file and nearby context.
git ls-files src/assets.ts
wc -l src/assets.ts
cat -n src/assets.ts | sed -n '1,220p'Repository: nuxt/fonts
Length of output: 6001
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check for related path handling and any existing normalization helpers.
rg -n "basename\\(|event\\.path|renderedFontURLs|assetsBaseURL|pathname|search" srcRepository: nuxt/fonts
Length of output: 874
Strip query strings before deriving the font filename. event.path includes the search string in h3, so basename(event.path) will miss cached entries for requests like font.woff2?v=1 and 404. Use event.url.pathname (or strip event.url.search) here instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/assets.ts` around lines 32 - 33, The font lookup in the assets handler is
using event.path directly, so query strings from h3 requests can make
basename(...) miss cached entries and cause false 404s. Update the logic in the
assets.ts flow around the filename/url lookup to derive the name from
event.url.pathname, or otherwise strip event.url.search before calling basename,
so renderedFontURLs keys match requests like font.woff2?v=1.
|
|
||
| addDevServerHandler({ | ||
| route: joinURL(nuxt.options.runtimeConfig.app.baseURL || nuxt.options.app.baseURL, context.assetsBaseURL), | ||
| route: joinURL(nuxt.options.runtimeConfig.app.baseURL || nuxt.options.app.baseURL, context.assetsBaseURL, '**'), |
There was a problem hiding this comment.
Incompatible with Nitro 2 🧐
2 participants
🔗 Linked issue
no response
📚 Description
Add Nitro 3 support with backward compatibility.