Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PoC: feat: Allow handlers and middleware to return functions of the same type as themselves #4012

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

usualoma
Copy link
Member

This PR will add a convenient feature to hono, but it will also increase the complexity, so please be careful. It is currently at the PoC stage.

What does this enable us to do?

We can dynamically configure middleware with very clean code.
Of course, I know that authentication middleware has the ability to dynamically determine credentials, but with this feature, it is easy to pass settings dynamically even if the middleware does not support it.

const app = new Hono()
  .get('/shortcut-icon.svg', () => {
    const path = 'dynamic-deterministic-filename.svg'
    return serveStatic({ path })
  })
  .get('/', (c) => {
    return c.text('Hello World')
  })
const app = new Hono()
  .post('*', (c) => {
    const limit = new URL(c.req.url).host === 'localhost' ? 1024 * 1024 * 10 : 1024 * 1024
    return bodyLimit({ maxSize: limit })
  })
  .post('/upload', uploadHandler)
const app = new Hono()
  .post('*', (c) => {
    const username = c.req.header('Authorization')?.split(' ')[1] ?? ''
    return basicAuth({ username, password: passwordMap[username] })
  })
  .get('/', (c) => {
    return c.text('Hello World')
  })

Isn't this just syntax sugar?

Yes, we can call it with (c, next) and return it as it is now, so it is safe to think of it as syntax sugar that omits (c, next).

const app = new Hono()
  .get('/shortcut-icon.svg', (c, next) => {
    const path = 'dynamic-deterministic-filename.svg'
    return serveStatic({ path })(c, next)
  })
  .get('/', (c) => {
    return c.text('Hello World')
  })

When is it particularly useful?

We will be able to solve issues that can be solved simply by using existing middleware in a clean way.

honojs/node-server#205

Is there any degradation in performance?

I don't think there is any decrease in performance at runtime.
When I compared them in "benchmarks/handle-event", there was no decrease in node and bun.

itty-router x 376,693 ops/sec ±4.26% (85 runs sampled)
sunder x 465,962 ops/sec ±3.09% (86 runs sampled)
worktop x 224,003 ops/sec ±4.51% (85 runs sampled)
Hono current main x 513,444 ops/sec ±5.52% (81 runs sampled)
Hono feat:returns-middleware x 525,498 ops/sec ±5.83% (80 runs sampled)
Fastest is Hono feat:returns-middleware,Hono current main

There may also be an impact on performance for "types", but I don't know the details of that, so I haven't been able to measure it.

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

Copy link

codecov bot commented Mar 18, 2025

Codecov Report

Attention: Patch coverage is 93.67089% with 5 lines in your changes missing coverage. Please review.

Project coverage is 91.29%. Comparing base (035c2d7) to head (d5b878e).

Files with missing lines Patch % Lines
src/adapter/cloudflare-pages/handler.ts 90.90% 3 Missing ⚠️
src/compose.ts 91.66% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4012      +/-   ##
==========================================
- Coverage   91.32%   91.29%   -0.04%     
==========================================
  Files         168      168              
  Lines       10687    10706      +19     
  Branches     3021     3031      +10     
==========================================
+ Hits         9760     9774      +14     
- Misses        926      931       +5     
  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.

@usualoma
Copy link
Member Author

Hi @yusukebe.

I'm not sure whether this is a really cool feature or just something that makes things more complicated. What do you think?

@yusukebe
Copy link
Member

First, regarding the performance of TypeScript. I've measured the performance in the same way of the CI (it does not work on this PR because of a permission issue).

The main:

Files:              263
Lines:           118435
Identifiers:     116371
Symbols:         307723
Types:           216430
Instantiations: 3093926
Memory used:    709000K
I/O read:         0.02s
I/O write:        0.00s
Parse time:       0.28s
Bind time:        0.12s
Check time:       2.88s
Emit time:        0.00s
Total time:       3.27s

This PR #4012:

Files:              263
Lines:           118454
Identifiers:     116392
Symbols:         309125
Types:           217913
Instantiations: 3098926
Memory used:    703547K
I/O read:         0.02s
I/O write:        0.00s
Parse time:       0.29s
Bind time:        0.12s
Check time:       2.91s
Emit time:        0.00s
Total time:       3.32s

The result is Instantiations will increase 5000 from the main. As expected, the value increased, but it wasn't huge. If this feature is good, I think the performance degradation due to the type can be ignored.

@yusukebe
Copy link
Member

yusukebe commented Mar 20, 2025

Hi @usualoma

Thank you for the proposal and the PoC.

First of all, I like this feature.

Until now, I had always thought that handlers would always return a Response (if it's middleware, it should return Promise<void | Response>). That's reasonable, and the great thing about Hono is that you can return a response using the raw API, like new Response('Hello') instead of c.text('Hello') etc.

However, I realized the only didn't need to be to return a Response. Also, I've always thought forcing users to add (c, next) was not a good idea. It's particularly confusing for new users.

One point that concerns me is that, until now, handlers have been defined simply as returning a Response. However, this PR changes that definition. Therefore, we need to tell users the type of handler. The definition of the handler in the current type.ts is very complicated, so I thought it would be better to present users with a simplified type like the following (I added the definition of middleware).

// Simplified:
type Handler = (c: Context, next: Next) => Response | Promise<Response> | Handler
type MiddlewareHandler = (c: Context, next: Next) => Promise<Response | void | MiddlewareHandler>

Anyway, this is a big feature that will change the definition of Hono's handler and middleware. We may be able to leave it for a while.

@yusukebe
Copy link
Member

Another reason I like this feature is that it allows me to write neatly. This may be a matter of personal taste, but it makes sense because I have always placed importance on being able to write neatly when developing Hono.

// Verbose
app.get('/admin/*', (c, next) => {
  return basicAuth({ username, password })(c, next)
})

// Neat!
app.get('/admin/*', () => {
  return basicAuth({ username, password })
})

@usualoma
Copy link
Member Author

Hi @yusukebe, Thank you for your response!

As you say, describing it as ‘neatly’ might be better.

I think the following are two of Hono's core values (there are several).

  • middleware
    • easy to write
    • the core has all the essential things
    • there are many reusable middleware, including honojs/middleware and other than the core.
  • neatly
    • when using Hono, we can write application code neatly

This PR will expand the range of middleware use and further strengthen Hono's core values.

Will ‘neatness’ continue to be important?

In the Vibe Coding Era, when writing code with AI Agents, neatness may become less important than it is now. There is a possibility that, rather than being able to write concisely, people will prefer to write in a slightly more verbose but non-omitting way.

However, highly abstract and neat code will remain valuable because it is easy to read.

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