Skip to content

fix(routing): use serializeHandlerFn for routed middleware - #4563

Open
koding88 wants to merge 2 commits into
nitrojs:mainfrom
koding88:fix/routed-middleware-serialization
Open

fix(routing): use serializeHandlerFn for routed middleware#4563
koding88 wants to merge 2 commits into
nitrojs:mainfrom
koding88:fix/routed-middleware-serialization

Conversation

@koding88

Copy link
Copy Markdown

Summary

Fixes #4557

When registering a handler with middleware: true and a route pattern (route-scoped middleware), findRoutedMiddleware compiled with serialize: serializeHandler, which emits route record objects { route, method, meta, handler } as route data.

In the generated #nitro/virtual/app template:

middleware.push(...findRoutedMiddleware(method, pathname).map((r) => r.data));

the route record object was passed directly to H3 as a middleware function, causing runtime requests to fail with TypeError: fn is not a function (HTTP 500).

Background

Global middleware in src/build/virtual/routing.ts is serialized via serializeHandlerFn, emitting bare handler functions (wrapped in h3.toEventHandler). Route-scoped middleware in findRoutedMiddleware should also be serialized as handler functions so r.data yields executable middleware handlers for H3.

Verification

  • Added unit test in test/unit/virtual-routing.test.ts verifying findRoutedMiddleware compiles with serializeHandlerFn.
  • Full test suites passed locally: pnpm test:rolldown and pnpm test:rollup (55 test files, 923 tests passed).
  • Lint, format and typecheck clean (pnpm fmt, pnpm lint, pnpm typecheck).

@vercel

vercel Bot commented Aug 27, 2026

Copy link
Copy Markdown

@koding88 is attempting to deploy a commit to the Nitro Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 712b3a1a-b875-461a-9aef-f01d0179a16d

📥 Commits

Reviewing files that changed from the base of the PR and between eb3a4da and 69bb25d.

📒 Files selected for processing (1)
  • test/unit/virtual-routing.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • test/unit/virtual-routing.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The routing template now serializes routed middleware as direct handler references. The unit test remains within the virtual routing template test group and verifies the expected h3.toEventHandler expression.

Changes

Routed middleware serialization

Layer / File(s) Summary
Direct handler serialization and regression test
src/build/virtual/routing.ts, test/unit/virtual-routing.test.ts
findRoutedMiddleware now uses serializeHandlerFn instead of serializeHandler. The test verifies that routed middleware emits h3.toEventHandler(_mid123) in the generated template.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 69bb2

This change makes route-scoped middleware executable at request time instead of passing route metadata as a middleware function, addressing the reported HTTP 500 failure. No actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title follows Conventional Commits syntax with the fix type and clearly describes the routed middleware serialization change.
Description check ✅ Passed The description explains the route-scoped middleware failure, the serialization fix, and the verification performed. It is directly related to the changeset.
Linked Issues check ✅ Passed The change addresses issue #4557 by serializing routed middleware with serializeHandlerFn, which emits executable handler functions instead of route records. The test covers the corrected serializat…
Out of Scope Changes check ✅ Passed The changes are limited to the serialization fix and related test organization. No unrelated code changes are present.
Full details: Linked Issues check

Explanation

The change addresses issue #4557 by serializing routed middleware with serializeHandlerFn, which emits executable handler functions instead of route records. The test covers the corrected serialization behavior.

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a runtime failure when using route-scoped middleware (middleware: true + route pattern) by ensuring routed middleware is serialized as executable handler functions (via serializeHandlerFn) rather than route-record objects, aligning the generated #nitro/virtual/routing output with how #nitro/virtual/app consumes findRoutedMiddleware(...).map(r => r.data).

Changes:

  • Switch findRoutedMiddleware compilation to use serializeHandlerFn so r.data is a callable middleware handler.
  • Add a unit test ensuring routed middleware serialization emits h3.toEventHandler(...) rather than a { route, method, meta, handler } record.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/build/virtual/routing.ts Adjusts routed-middleware serialization so generated middleware entries are executable handler functions.
test/unit/virtual-routing.test.ts Adds regression coverage asserting the routed-middleware serializer output uses serializeHandlerFn semantics.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +56 to +80

it("serializes routed middleware with serializeHandlerFn instead of route record", () => {
const handler: NitroEventHandler & { _importHash: string } = {
route: "/api/**",
handler: "/path/to/middleware.ts",
_importHash: "_mid123",
middleware: true,
};
const nitroStub = {
options: { baseURL: "/", routeRules: {} },
routing: {
routes: { routes: [], compileToString: () => "{}" },
routedMiddleware: {
routes: [{ route: "/api/**", data: handler }],
compileToString: ({ serialize }: { serialize: (h: unknown) => string }) =>
`{"/api/**":${serialize(handler)}}`,
},
globalMiddleware: [],
},
} as unknown as Nitro;
const template = routing(nitroStub).template();
expect(template).toContain(
'export const findRoutedMiddleware = {"/api/**":h3.toEventHandler(_mid123)};'
);
});
@nitrojs nitrojs temporarily blocked koding88 Aug 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Route-scoped middleware throws fn is not a function and returns 500

2 participants