fix(types): propagate middleware response types to app.on overloads#4906
Merged
yusukebe merged 1 commit intohonojs:mainfrom May 5, 2026
Merged
fix(types): propagate middleware response types to app.on overloads#4906yusukebe merged 1 commit intohonojs:mainfrom
yusukebe merged 1 commit intohonojs:mainfrom
Conversation
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 Report✅ All modified and coverable lines are covered by tests. 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. 🚀 New features to boost your workflow:
|
Member
|
Nice catch. This should be fixed! Thanks. |
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 appliedto
HandlerInterface(app.get/post/...) butOnHandlerInterfacewas not updated, so the sameroute declared with
app.on(method, path, ...)orapp.on(method[], path, ...)silently droppedmiddleware responses from the schema.
This is the same shape of asymmetry that #3852 and #4865 already fixed for narrower cases (
Rdiscard)—
OnHandlerInterfacekeeps drifting away fromHandlerInterfacewhenever the handler-side overloadsgain new type parameters.
Affected overloads
18 overloads in total:
app.on(method, path, handler x2..x10)— 9 overloadsapp.on(method[], path, handler x2..x10)— 9 overloadsThe single-handler variants (
x1) and the rest-array fallback overloads are unaffected because theyhave no preceding middleware position to capture.
Reproduction
Switching
app.on('GET', ...)↔app.get(...)flips the inference even though the routes aresemantically 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 mirroringHandlerInterface, 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 themethods: M[]overloads.
Test plan
Added regression tests under the existing
describe('RPC supports Middleware responses', ...)block insrc/types.test.ts, mirroring the correspondingapp.gettests. Coverage:app.on('GET', '/test', ...)with 1, 2, 5, and 9 middlewares before the response handlerapp.on(['GET'], '/test', ...)with 1, 2, 5, and 9 middlewares before the response handlerEach test asserts that every middleware status code (
400,401,403, ...) is reachable in theinferred RPC client and that
res.json()is narrowed correctly per status. These tests fail onmain(status union collapses to just
200, so every middleware-status branch becomesnever) and passafter this fix.
bun run format:fix && bun run lint:fixbun run test(full type check + vitest, 3400 passed)method[]variantsRelated history
Introduced
MergeMiddlewareResponse<M_k>forHandlerInterfaceonly.OnHandlerInterface#3852 (2025-01-25) — fix(types): missing response type onOnHandlerInterfaceEarlier
Rdiscard fix forOnHandlerInterface(different from this one).overloads
Closer cousin: same kind of
OnHandlerInterfacedrift, narrower scope.This PR completes the alignment so that
OnHandlerInterfacemirrorsHandlerInterfacefor bothRand
M_kpropagation. May also alleviate the open feedback in #3746.