fix(types): replace schema-based path tracking with VoidMiddlewarePath parameter

[kosei28]

• ref: #4529

The author should do the following, if applicable

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

[codecov[bot]]

Codecov Report

:x: Patch coverage is 95.00000% with 1 line in your changes missing coverage. Please review. :white_check_mark: Project coverage is 91.53%. Comparing base (ee52b25) to head (a1bf08e).

Files with missing lines	Patch %	Lines
src/hono-base.ts	95.00%	1 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##             next    #4552      +/-   ##
==========================================
- Coverage   91.53%   91.53%   -0.01%     
==========================================
  Files         172      172              
  Lines       11228    11232       +4     
  Branches     3263     3262       -1     
==========================================
+ Hits        10278    10281       +3     
- Misses        949      950       +1     
  Partials        1        1              

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[kosei28]

I found a part that works differently than expected outside of the test cases, so I'll put the PR in draft.

[kosei28]

I fixed it according to the original specifications and added tests for them.

[kosei28]

The number of instantiations is now 694,649.

[kosei28]

I fixed the pathless handler's path to be a union to match the original type. However, in the actual routing, this.#path (which stores the last registered path) is used for the pathless handler, resulting in the following behavior. The CurrentPath implementation before the fix might have been better.

Also, even if the handler's return value isn't Promise<void>, calling next will route to the subsequent handler, so I don't think type branching based on the return type is necessary.

import { Hono } from "hono";

const app = new Hono()
  .use(async (c, next) => {
    await next();
    console.log(`status: ${c.res.status}`);
  })
  .get("/a", async (c, next) => {
    console.log("get /a middleware");
    await next();
  })
  .get("/b", async (c, next) => {
    console.log("get /b middleware");
    await next();
    return c.res;
  })
  .get((c) => {
    console.log(`get ${c.req.url}`);
    return c.text("");
  })
  .post(async (c) => {
    console.log(`post ${c.req.url}`);
    return c.text("");
  });

await app.request("/a", { method: "GET" });
// get /a middleware
// status: 404

await app.request("/b", { method: "GET" });
// get /b middleware
// get http://localhost/b
// status: 200

await app.request("/a", { method: "POST" });
// status: 404

await app.request("/b", { method: "POST" });
// post http://localhost/b
// status: 200

[kosei28]

For reference, the schema generated by the code above looks like this:

CurrentPath approach

const app: Hono<{}, {
    "/b": {
        $get: {
            input: {};
            output: {};
            outputFormat: string;
            status: StatusCode;
        };
    };
} & {
    "/b": {
        $get: {
            input: {};
            output: "";
            outputFormat: "text";
            status: ContentfulStatusCode;
        };
    };
} & {
    "/b": {
        $post: {
            input: {};
            output: "";
            outputFormat: "text";
            status: ContentfulStatusCode;
        };
    };
}, "/", "/b">

VoidMiddlewarePath approach

const app: Hono<{}, {
    "/b": {
        $get: {
            input: {};
            output: {};
            outputFormat: string;
            status: StatusCode;
        };
    };
} & {
    "/a": {
        $get: {
            input: {};
            output: "";
            outputFormat: "text";
            status: ContentfulStatusCode;
        };
    };
} & {
    "/a": {
        $post: {
            input: {};
            output: "";
            outputFormat: "text";
            status: ContentfulStatusCode;
        };
    };
}, "/", "/a">

ExtractStringKey approach (from #4529)

const app: Hono<{}, {
    "/a": {};
} & {
    "/b": {
        $get: {
            input: {};
            output: {};
            outputFormat: string;
            status: StatusCode;
        };
    };
} & {
    "/a": {
        $get: {
            input: {};
            output: "";
            outputFormat: "text";
            status: ContentfulStatusCode;
        };
    };
} & {
    "/a": {
        $post: {
            input: {};
            output: "";
            outputFormat: "text";
            status: ContentfulStatusCode;
        };
    };
    "/b": {
        $post: {
            input: {};
            output: "";
            outputFormat: "text";
            status: ContentfulStatusCode;
        };
    };
}, "/">

[kosei28]

The number of instantiations for the CurrentPath approach is 684,676.

[yusukebe]

Hi @kosei28

Thank you for your work! I want to take a look at this deeply, but I'm a little busy. Though I said "please help me", I can't work on it immediately, sorry, but wait! It will help us. You are great!

[yusukebe]

Hey @kosei28 !

Sorry for being late. Awesome! Let's go with the VoidMiddlewarePath approach. I've left some comments. Please check them.

[kosei28]

I think the CurrentPath approach is simpler to implement and aligns better with the actual behavior. Could you tell me the reason for adopting the VoidMiddlewarePath approach?

CurrentPath approach: https://github.com/honojs/hono/compare/51765d1...98df8fa

[yusukebe]

@kosei28

Ah, you are right! The CurrentPath approach is best. The problem was that types did not know the "current path". Let's go with it.

[kosei28]

Reverted to CurrentPath approach and fixed type updates for pathless app.use() and multi-path app.on().

Note: Tests pass locally, but CI is failing due to const type parameters conflicting with the old esbuild version. #4571 will fix the build.

[yusukebe]

@kosei28

Looks good! You solve the big problem. Awesome. Thank you for your contribution!!

[yusukebe]

I'll merge this to the next branch for the next minor version and release it soon.