honojs / hono

Web framework built on Web Standards
https://hono.dev
MIT License
20.38k stars 584 forks source link

Context<Env> from middlewares are not inferred in `factory.createHandlers` #3467

Closed mecab closed 3 days ago

mecab commented 1 month ago

What version of Hono are you using?

4.6.3

What runtime/platform is your app running on?

Node.js / Bun

What steps can reproduce the bug?

The Envs from middleware is inferred in the handlers which are created as app.get('/foo', middleware, ...), but it isn't when you create them using factory.createHandlers(middleware, ...).

Here's an example based on the doc:

import { Hono } from 'hono';
import { createFactory, createMiddleware } from 'hono/factory'
import { logger } from 'hono/logger'

const app = new Hono();
const factory = createFactory()

const middleware = createMiddleware<{ Variables: { foo: string } }>(async (c, next) => {
  c.set('foo', 'bar')
  await next()
})

const handlers = factory.createHandlers(logger(), middleware, async (c) => {
  // I suppose `c` is `Context<{ Variables: { foo: string } }, ...>`, but actually `Context<any, any, {}>`

  return c.json(c.get('foo'));
});

app
  .get('/api', ...handlers)
  .get('/api', logger(), middleware, async (c) => {
    // Here `c` is inferred as expected.
    return c.json(c.get('foo'));
  })

I attach how it looks in VSCode for reference image

What is the expected behavior?

As I wrote in the code comment, the context type of c in createHandlers is inferred as Context<{ Variables: { foo: string } }, ...> so I can use c.get('foo') or c.var.foo with the inferred type, as like as it works for the handler which directly attached to app.

What do you see instead?

The type of c is Context<any, any, {}> so I cannot get benefit from the type. Although it works as expected for the directly attached handler.

Additional information

I am not 100% sure if my assumption that the inference works for the createHandlers. If it is not expected as design (or TypeScript constraint), sorry for my misunderstanding.

I have searched the issue and found #3202 (and its dups - #3341, #3198) is discussing the inference for middlewares, but I guess it is actually not relevant.

yusukebe commented 1 month ago

Hi @mecab

Thank you for raising the issue. This is like a bug, but it is a not implemented thing rather than a bug. We can't fix it right now, but we may do it later.

mecab commented 1 month ago

Thanks @yusukebe, I see it is not supported at right now. I wish it comes at some point 🙂

rnldsalili commented 2 weeks ago

Hi @mecab

Thank you for raising the issue. This is like a bug, but it is a not implemented thing rather than a bug. We can't fix it right now, but we may do it later.

Great! This is reassuring. I also encountered the same issue.

sushichan044 commented 3 days ago

@yusukebe Hello, I'm currently working on this issue. I have confirmed that the Context type is calculated successfully. I will create PR later!