When chaining routes with middlewares, the context type might be incorrect.

[agatan]

What version of Hono are you using?

3.12.5

What runtime/platform is your app running on?

Node

What steps can reproduce the bug?

Run and type check the code below:

import { Hono, MiddlewareHandler } from 'hono'

const mw1: MiddlewareHandler<{ Variables: { foo1: string } }> = async (c, next) => { 
  c.set('foo1', 'foo1')
  await next()
}
const mw2: MiddlewareHandler<{ Variables: { foo2: string } }> = async (c, next) => {
  c.set('foo2', 'foo2')
  await next()
}

const app = new Hono()
  .get('/1', mw1, async (c) => {
    // this is correctly typed.
    return c.json({ foo1: c.var.foo1 })
  })
  .get('/1-then-2', mw1, mw2, async (c) => {
    // this is also correctly typed.
    return c.json({ foo1: c.var.foo1, foo2: c.var.foo2 })
  })
  .get('/nothing', async (c) => {
    // In this scope, c.var should be empty because no middlewares are applied.
    // However, c.var is typed as { foo1: string; foo2: string; }.
    // In runtime, v is {}.
    const v: { foo1: string, foo2: string } = c.var;
    return c.json(v);
  });


console.log(await (await app.request('/1')).json())
// => { foo1: 'foo1' }

console.log(await (await app.request('/1-then-2')).json())
// => { foo1: 'foo1', foo2: 'foo2' }

console.log(await (await app.request('/nothing')).json())
// => {}

What is the expected behavior?

In the handler for /nothing, no middlewares are applied.
So, the type of c.var should be inferred as an empty object.

What do you see instead?

In the handler for /nothing, the type of c.var is inferred as { foo1: string; foo2: string }.

Additional information

I believe the issue with Hono lies in the overloaded types of HandlerInterface.

The HandlerInterface returns Hono<E, S, P>, where E is extended by the Env definitions of handlers. However, this extension is limited to the same handler scope.

I attempted a fix (see commit), but then realized that this might conflict with issue #1666.

I believe that to achieve correct type inference, it's necessary to maintain information about the paths where middleware is applied at the type level. However, I think this could be quite challenging to implement.

[yusukebe]

Hi @agatan

Thanks for the issue! I will look at it later, but I will tell you one thing.

We are currently working on challenges for Types in the v4 branch, currently. For example, the following changes have been merged into v4.

#1995

If you create a PR, please make it based on v4.

[agatan]

Thank you for pointing that out.
I was aware of the branch, but I wasn't sure which one to target for my PR.
I'll make sure to do so in the future!

Just to be sure, I've confirmed that the same issue occurs in the v4 branch as well.

[yusukebe]

Hi @agatan !

Perhaps the specifications here are wrong.

hono/src/types.test.ts

Lines 841 to 845 in 1f46aa1

	new Hono().get('/', mw1).get('/', (c) => {
	expectTypeOf(c.get('foo1')).toEqualTypeOf<string>()
	expectTypeOf(c.var.foo1).toEqualTypeOf<string>()
	return c.json(0)
	})

This test must fail, and the following tests must pass:

type Env = {
  Variables: {
    init: number
  }
}

const app1 = new Hono<Env>().get('/', mw1, (c) => {
  expectTypeOf(c.get('init')).toEqualTypeOf<number>()
  expectTypeOf(c.var.init).toEqualTypeOf<number>()
  expectTypeOf(c.get('foo1')).toEqualTypeOf<string>()
  expectTypeOf(c.var.foo1).toEqualTypeOf<string>()
  return c.json(0)
})

app1.get('/', (c) => {
  expectTypeOf(c.get('init')).toEqualTypeOf<number>()
  expectTypeOf(c.var.init).toEqualTypeOf<number>()
  // @ts-expect-error foo1 is not typed
  c.get('foo1')
  // @ts-expect-error foo1 is not typed
  c.var.foo1
  return c.json(0)
})

To correct this, we can fix the following type definitions.

hono/src/types.ts

Lines 384 to 398 in 1f46aa1

	// app.get(path, handler)
	<
	P extends string,
	MergedPath extends MergePath<BasePath, P> = MergePath<BasePath, P>,
	R extends HandlerResponse<any> = any,
	I extends Input = {},
	E2 extends Env = E
	>(
	path: P,
	handler: H<E2, MergedPath, I, R>
	): Hono<
	IntersectNonAnyTypes<[E, E2]>,
	S & ToSchema<M, MergePath<BasePath, P>, I['in'], MergeTypedResponseData<R>>,
	BasePath
	>

The diff looks like this:

diff --git a/src/types.ts b/src/types.ts
index 5fdcccd..2d6ffd0 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -391,11 +391,7 @@ export interface HandlerInterface<
   >(
     path: P,
     handler: H<E2, MergedPath, I, R>
-  ): Hono<
-    IntersectNonAnyTypes<[E, E2]>,
-    S & ToSchema<M, MergePath<BasePath, P>, I['in'], MergeTypedResponseData<R>>,
-    BasePath
-  >
+  ): Hono<E, S & ToSchema<M, MergePath<BasePath, P>, I['in'], MergeTypedResponseData<R>>, BasePath>

   // app.get(path, handler x2)
   <

What do you think? I think we can fix this by the approach.

[agatan]

@yusukebe
Thank you for your reply. I agree with your approach.

I was trying it in this commit, but as you pointed out, the test at

hono/src/types.test.ts

Lines 841 to 845 in 1f46aa1

	new Hono().get('/', mw1).get('/', (c) => {
	expectTypeOf(c.get('foo1')).toEqualTypeOf<string>()
	expectTypeOf(c.var.foo1).toEqualTypeOf<string>()
	return c.json(0)
	})

failed, so I wanted to discuss this issue.

If we proceed with this solution, it looks like the commit I tried could be reused. I'll create a pull request.

[yusukebe]

@agatan

Yeah. I think the tests should fail.

I've left comments on your PR. Check them!

[yusukebe]

Fixed!