Skip to content

fix(types): propagate middleware response types to app.on overloads#4906

Merged
yusukebe merged 1 commit intohonojs:mainfrom
T4ko0522:fix/types-app-on-mw-response
May 5, 2026
Merged

fix(types): propagate middleware response types to app.on overloads#4906
yusukebe merged 1 commit intohonojs:mainfrom
T4ko0522:fix/types-app-on-mw-response

Conversation

@T4ko0522
Copy link
Copy Markdown
Contributor

@T4ko0522 T4ko0522 commented May 4, 2026

Summary

PR #4393 introduced MergeMiddlewareResponse<M_k> to capture middleware responses (e.g. c.json({ error }, 401) returned from a preceding handler) into the inferred RPC schema. The change was applied
to HandlerInterface (app.get/post/...) but OnHandlerInterface was not updated, so the same
route declared with app.on(method, path, ...) or app.on(method[], path, ...) silently dropped
middleware responses from the schema.

This is the same shape of asymmetry that #3852 and #4865 already fixed for narrower cases (R discard)
OnHandlerInterface keeps drifting away from HandlerInterface whenever the handler-side overloads
gain new type parameters.

Affected overloads

18 overloads in total:

  • app.on(method, path, handler x2..x10) — 9 overloads
  • app.on(method[], path, handler x2..x10) — 9 overloads

The single-handler variants (x1) and the rest-array fallback overloads are unaffected because they
have no preceding middleware position to capture.

Reproduction

import { Hono } from 'hono'
import { hc } from 'hono/client'
import { createMiddleware } from 'hono/factory'

const auth = createMiddleware(async (c, next) => {
  if (!authorized) return c.json({ error: 'unauthorized' }, 401)
  await next()
})

// Works correctly (already covered by #4393)
const a = new Hono().get('/secret', auth, (c) => c.json({ ok: true }, 200))

// Bug: `output` collapses to `{ ok: true }` only, dropping the 401 response
const b = new Hono().on('GET', '/secret', auth, (c) => c.json({ ok: true }, 200))

const client = hc<typeof b>('/')
const res = await client.secret.$get()
if (res.status === 401) {
  // Before this PR: TypeScript narrows to `never`, so this branch is unreachable
  // After this PR:  `res.json()` resolves to `{ error: string }`
  const err = await res.json()
}

Switching app.on('GET', ...)app.get(...) flips the inference even though the routes are
semantically identical. The same gap exists for the method[] form.

Fix

For each of the 18 affected overloads, add M1..M(n-1) middleware type parameters mirroring
HandlerInterface, intersect them into the handler tuple positions before the final response handler,
and union MergeMiddlewareResponse<M_k> into the schema's response type.

The same shape is applied for handler counts 3..10 (with M1..M(n-1)) and for the methods: M[]
overloads.

Test plan

Added regression tests under the existing describe('RPC supports Middleware responses', ...) block in
src/types.test.ts, mirroring the corresponding app.get tests. Coverage:

  • app.on('GET', '/test', ...) with 1, 2, 5, and 9 middlewares before the response handler
  • app.on(['GET'], '/test', ...) with 1, 2, 5, and 9 middlewares before the response handler

Each test asserts that every middleware status code (400, 401, 403, ...) is reachable in the
inferred RPC client and that res.json() is narrowed correctly per status. These tests fail on main
(status union collapses to just 200, so every middleware-status branch becomes never) and pass
after this fix.

  • bun run format:fix && bun run lint:fix
  • bun run test (full type check + vitest, 3400 passed)
  • Added regression tests for both single-method and method[] variants

Related history

This PR completes the alignment so that OnHandlerInterface mirrors HandlerInterface for both R
and M_k propagation. May also alleviate the open feedback in #3746.

PR honojs#4393 introduced `MergeMiddlewareResponse<M_k>` to capture middleware responses (e.g. `c.json({ error }, 401)` returned from a preceding handler) into the inferred RPC schema. The change was applied to `HandlerInterface` (`app.get/post/...`) but `OnHandlerInterface` was not updated, so the same route declared with `app.on(method, path, ...)` or `app.on(method[], path, ...)` silently dropped middleware responses from the schema.

Bring 18 overloads (handler counts 2..10 for both single-method and method[] variants) in line with `HandlerInterface` by introducing M1..M(n-1) middleware type parameters and unioning `MergeMiddlewareResponse<M_k>` into the schema's response type.

Tests covering 1, 2, 5, and 9 middlewares for each variant are added under the existing `RPC supports Middleware responses` describe.

Follow-up to honojs#4393 and honojs#4865.
@codecov
Copy link
Copy Markdown

codecov Bot commented May 5, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.00%. Comparing base (8f027e5) to head (0fd7a89).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4906   +/-   ##
=======================================
  Coverage   93.00%   93.00%           
=======================================
  Files         177      177           
  Lines       11900    11900           
  Branches     3568     3568           
=======================================
  Hits        11068    11068           
  Misses        832      832           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Member

@yusukebe yusukebe left a comment

Choose a reason for hiding this comment

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

LGTM!

@yusukebe
Copy link
Copy Markdown
Member

yusukebe commented May 5, 2026

@T4ko0522

Nice catch. This should be fixed! Thanks.

@yusukebe yusukebe merged commit 52aaaf9 into honojs:main May 5, 2026
20 checks passed
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.

2 participants