docs: fix event.path usage and misleading middleware path in examples#4384
docs: fix event.path usage and misleading middleware path in examples#4384zhsj0089944 wants to merge 1 commit into
Conversation
1. lifecycle.md: event is an HTTPEvent, not H3Event, so event.path does not exist. Use event.url.pathname instead. 2. routing.md: The route-scoped middleware example used ./server/middleware/api-auth.ts as the handler path. Since files in the middleware/ directory are automatically registered as global middleware, this makes the route: '/api/**' config redundant and causes double-execution. Change to ./server/handlers/api-auth.ts to avoid the conflict. Fixes nitrojs#4375, Fixes nitrojs#4354
|
Someone is attempting to deploy a commit to the Nitro Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis PR updates two documentation examples in the routing guide to use ChangesRouting documentation examples
Lifecycle request hook example
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes 🚥 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 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/1.docs/50.lifecycle.md (1)
124-128: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUpdate
responsehook example for consistency.The
responsehook example still usesevent.path, but the hooks reference table types bothrequestandresponsehooks withevent: HTTPEvent. Ifpathis not available onHTTPEvent, this example should also useevent.url.pathnameto match therequesthook fix and avoid reader confusion.- console.log(`Response ${res.status} for ${event.path}`); + console.log(`Response ${res.status} for ${event.url.pathname}`);🤖 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 `@docs/1.docs/50.lifecycle.md` around lines 124 - 128, The `response` hook example in `definePlugin` is still using `event.path`, which is inconsistent with the `HTTPEvent`-based hooks reference and may not exist on that object. Update the example inside the `nitroApp.hooks.hook("response", ...)` callback to use `event.url.pathname`, matching the `request` hook example and keeping the docs consistent.
🤖 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.
Outside diff comments:
In `@docs/1.docs/50.lifecycle.md`:
- Around line 124-128: The `response` hook example in `definePlugin` is still
using `event.path`, which is inconsistent with the `HTTPEvent`-based hooks
reference and may not exist on that object. Update the example inside the
`nitroApp.hooks.hook("response", ...)` callback to use `event.url.pathname`,
matching the `request` hook example and keeping the docs consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 90896e77-764d-453d-b562-ee4a3f8d1731
📒 Files selected for processing (2)
docs/1.docs/5.routing.mddocs/1.docs/50.lifecycle.md
only when serverDir is set |
Changes
1. lifecycle.md: Fix
event.path→event.url.pathnameThe
requesthook receives anHTTPEvent(from h3), not anH3Event. TheHTTPEventinterface does not have apathproperty — the correct way to access the path isevent.url.pathname.Fixes #4375
2. routing.md: Fix misleading middleware handler path
The route-scoped middleware example used
./server/middleware/api-auth.tsas the handler path. Since files in themiddleware/directory are automatically registered as global middleware (matching/**), this:route: "/api/**"config completely redundantChanged to
./server/handlers/api-auth.tsto avoid the conflict.Fixes #4354