feat(types): passing middleware types

[slawekkolodziej]

This PR fixes #2719
It's somewhat based off this draft PR: #4252 (I copied majority of tests from there, thanks!)

After some more changes in validator middlewares (I already adjusted @hono/zod-validator and @hono/valibot-validator) it gives proper error codes through RPC.

The code here already works & should be passing all tests. The only outstanding thing I am aware of is adjusting types for app.use. From type like this:

<P extends string, MergedPath extends MergePath<BasePath, P>, E2 extends Env = E>(
  path: P,
  handler: MiddlewareHandler<E2, MergedPath, any>
): HonoBase<IntersectNonAnyTypes<[E, E2]>, ChangePathOfSchema<S, MergedPath>, BasePath>

into one that passes 4 params to MiddlewareHandler:

<P extends string, MergedPath extends MergePath<BasePath, P>, E2 extends Env = E>(
  path: P,
  handler: MiddlewareHandler<E2, MergedPath, any, any>
): HonoBase<IntersectNonAnyTypes<[E, E2]>, ChangePathOfSchema<S, MergedPath>, BasePath>

Without it app.use is complaining about invalid type when you try to pass a 'typed middleware' to it. I should be able to adjust this soon.

The author should do the following, if applicable

• Add tests
• Run tests
• bun run format:fix && bun run lint:fix to format the code
• Add TSDoc/JSDoc to document the code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.26%. Comparing base (e3bf8c8) to head (bfa3912).
⚠️ Report is 9 commits behind head on next.

Additional details and impacted files

@@           Coverage Diff           @@
##             next    #4393   +/-   ##
=======================================
  Coverage   91.25%   91.26%           
=======================================
  Files         171      171           
  Lines       10915    10927   +12     
  Branches     3146     3149    +3     
=======================================
+ Hits         9961     9973   +12     
  Misses        953      953           
  Partials        1        1           

☔ 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.

[yusukebe]

Hi @slawekkolodziej !

Awesome! It comes true and all tests passed.

One thing we have to consider is the performance. I took the diagnostics with the same method of
https://github.com/honojs/hono/blob/main/.github/actions/perf-measures/action.yml

The main branch:

$ bun tsc -p tsconfig.build.json --diagnostics
Files:              300
Lines:           140207
Identifiers:     127056
Symbols:         264248
Types:           155887
Instantiations:  538082
Memory used:    291398K
I/O read:         0.02s
I/O write:        0.00s
Parse time:       0.24s
Bind time:        0.13s
Check time:       1.13s
Emit time:        0.00s
Total time:       1.50s

This PR branch:

$ bun tsc -p tsconfig.build.json --diagnostics
Files:              300
Lines:           140576
Identifiers:     128020
Symbols:         266050
Types:           197880
Instantiations:  638156
Memory used:    356616K
I/O read:         0.02s
I/O write:        0.00s
Parse time:       0.24s
Bind time:        0.10s
Check time:       1.24s
Emit time:        0.00s
Total time:       1.57s

The Instantiations will be 538082 to 638156, this means, +100,074 +18.6% increase. I think this is not small.

So, we should choose.

1. Make them behave as expected
2. Keep the performance

Both are important. It's a common problem with the performance of type inference. If the project becomes big, the PRC will be super slow, and the editor may hang. However, making behavior expected is also important for achieving better DX.

I think we can accept this PR, though bringing the performance degradation since making consistency is significant, and we may resolve the performance issue in other ways.

Anyway, I'll review the details later. Thanks!

[yusukebe]

Hey @m-shaka @NamesMT !

What do you think of this PR? Of course, this is super awesome. But should we merge this?

[slawekkolodziej]

Regarding performance, I like to think that TS7, once released, will make this problem go away completely.

[jkonowitch]

I’ve made no contribution here so take this with a grain of salt, but I am eagerly awaiting this PR. Typesafe errors/responses from middleware is a huge upgrade to the usefulness of RPC. It allows us to compose behaviors using the elegant Hono middleware system AND retain type safety end to end - it will result in a better user experience as devs will be able to easily understand via the type system what errors may result from a particular RPC call and handle them in the UI accordingly.

Both project references/composite builds and of course TSGO will likely more than compensate for any performance impact.

[yusukebe]

@slawekkolodziej @jkonowitch

Yes. This PR is great. I'll merge this later!

[m-shaka]

My bad for being late. I think you can merge it.
Increase of instantiations isn't a disaster and way much better compared to the past, for example in #3439

[yusukebe]

LGTM!

[yusukebe]

Hi @slawekkolodziej

I'll merge this into the next branch for the next minor release v4.10.0.

Without it app.use is complaining about invalid type when you try to pass a 'typed middleware' to it. I should be able to adjust this soon.

If you can fix it and create a PR for it. Please make a PR against the next branch.

Thank you for the great work!!

[slawekkolodziej]

Great news!

Regarding that fix - it's already there! I pushed the fix in one of my subsequent commits (when I removed the 'draft' label). My bad, I should have update original my MR message.

I'll create another PR for zod & valibot middlewares to have proper type inference with those two (I already have working code locally). I think those two can serve as good starting point for other middleware's maintainers.