-
-
Notifications
You must be signed in to change notification settings - Fork 778
fix(vite): remove extension check to support dotted API routes #3994
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
@kricsleo is attempting to deploy a commit to the Nitro Team on Vercel. A member of the Team first needs to authorize it. |
π WalkthroughWalkthroughModified the Vite dev middleware to add a 404 fallback mechanism that delegates unhandled requests to the next middleware (allowing Vite to process them) and removed extension-based filtering that previously blocked extension-less URLs from reaching the Nitro/dev pipeline. Changes
Estimated code review effortπ― 2 (Simple) | β±οΈ ~10 minutes 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)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
π€ Fix all issues with AI agents
In `@src/build/vite/dev.ts`:
- Around line 225-229: The handler that checks envRes.status === 404 should not
reset nodeReq._nitroHandled because that lets the request be processed again by
nitroDevMiddleware; instead, remove the line that sets nodeReq._nitroHandled =
false and simply call next() so Vite can handle the fallback while leaving the
Nitro-handled flag intact (change the branch in the envRes.status === 404 block
that references nodeReq._nitroHandled and next()).
| if (envRes.status === 404) { | ||
| // Allow Vite to try | ||
| nodeReq._nitroHandled = false; | ||
| return next(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid re-running Nitro middleware after 404 fallback.
Resetting nodeReq._nitroHandled allows the same request to hit the later nitroDevMiddleware (registered at the end), duplicating Nitro dispatch for 404s. Keeping the flag set still lets Vite handle via next().
π§ Suggested change
if (envRes.status === 404) {
// Allow Vite to try
- nodeReq._nitroHandled = false;
return next();
}π Committable suggestion
βΌοΈ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (envRes.status === 404) { | |
| // Allow Vite to try | |
| nodeReq._nitroHandled = false; | |
| return next(); | |
| } | |
| if (envRes.status === 404) { | |
| // Allow Vite to try | |
| return next(); | |
| } |
π€ Prompt for AI Agents
In `@src/build/vite/dev.ts` around lines 225 - 229, The handler that checks
envRes.status === 404 should not reset nodeReq._nitroHandled because that lets
the request be processed again by nitroDevMiddleware; instead, remove the line
that sets nodeReq._nitroHandled = false and simply call next() so Vite can
handle the fallback while leaving the Nitro-handled flag intact (change the
branch in the envRes.status === 404 block that references nodeReq._nitroHandled
and next()).
commit: |
π Linked issue
resolves #3990
β Type of change
π Description
Removed overly broad extension check that broke routes like
/api/foo.get. Added404fallback to Vite instead.π Checklist