fix(routing): serialize route-scoped middleware as plain handlers - #4558
fix(routing): serialize route-scoped middleware as plain handlers#4558ShreeBohara wants to merge 1 commit into
Conversation
`findRoutedMiddleware` wrapped each match as `{ route, handler }` while the
app passed `match.data` straight to h3 as middleware, so any handler
registered with `middleware: true` and a route pattern failed with
`fn is not a function`. Serialize the bare handler, matching the
`MatchedRoute<Middleware>[]` contract, and cover the rules -> global ->
routed order in the fixture.
|
@ShreeBohara is attempting to deploy a commit to the Nitro Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe routing compiler now emits callable routed middleware handlers. The test fixture adds a route-scoped middleware endpoint and records execution order. The regression test verifies ChangesRoute-scoped middleware
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The change makes route-scoped middleware receive callable handlers and adds regression coverage for execution order and the prior failure. No actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
commit: |
1 participant
🔗 Linked issue
Resolves #4557
❓ Type of change
📚 Description
Route-scoped middleware (a
handlersentry withmiddleware: trueand a route pattern) returns a500 with
TypeError: fn is not a functionfor every matching request. Details and a minimalreproduction are in #4557.
findRoutedMiddlewareserialized each match withserializeHandler, somatch.datawas a routerecord
{ route, method, meta, handler }, while the generated app passes that value straight to h3as middleware. Global middleware in the same file already uses
serializeHandlerFn, which emits thebare handler — this makes routed middleware do the same, since the matcher's payload is middleware
rather than a route.
The alternative would be to unwrap on the consumer side in
virtual/app.ts(.map((r) => r.data.handler)),but the
route/method/metafields are never read for middleware, so emitting them at all is thepart that's wrong. Fixing the serializer also keeps the payload correct for any future consumer.
Regression test first, per
AGENTS.md: the fixture gains a global middleware and a route-scoped onethat record the order they run in, and
test/tests.tsasserts["rules", "global", "routed"], so thetest covers both the crash and the documented ordering. It runs against every preset. Reverting just
the one-word change in
routing.tsmakes it fail with the samefn is not a function.There was no test for route-scoped middleware anywhere in the repo before this, which is the reason
the breakage survived — the mismatch is present at least as far back as
d37caec(2025-12-11).Verified on this branch with
pnpm lint,pnpm typecheck, and the full suite against both builders:882 passed. The one failure,
test/unit/bump-version.test.ts, also fails on a cleanmaincheckoutin a non-UTC timezone and is unrelated.
📝 Checklist
already documented at https://nitro.build/docs/routing#route-scoped-middleware and this
only makes it behave as documented.