honojs / hono

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

Add NestJS Hono Adapter #2817

Open drdreo opened 1 month ago

drdreo commented 1 month ago

What is the feature you are proposing?

Hono Adapter for NestJs

https://hono.dev

Similar to https://docs.nestjs.com/techniques/performance#adapter

Reason

It would be amazing to have the choice for honojs in nestjs applications. Not only would it make hono more recognized in the JavaScript world, but also improve the NestJS ecosystem i believe.

Goal

The goal is to have a simple NestJS application adapter as we have for express and fastify. E.g.:

const app = await NestFactory.create<NestFastifyApplication>(AppModule, new FastifyAdapter());
await app.listen(3000);

and the ideal equivalent:

const app = await NestFactory.create<NestHonoApplication>(AppModule, new HonoAdapter());
await app.listen(3000);

Ideas From

Unfortunately, NestJS is not officially working on a Hono integration:

https://github.com/nestjs/nest/issues/13013

https://github.com/nestjs/nest/issues/13073#issuecomment-1902730322

but they are welcoming community contributions.

My attempt

My repo for testing, any help is appreciated: https://github.com/drdreo/nest-hono-adapter

I tried to get an adapter going but ran into roadblocks so i hope someone from here or the community can contribute and help me solve it.

Problems

For some reason, the response / hono context is not correctly propagated when calling the adapter methods. Instead, an async function is passed around, e.g. to the Nest reply(). Seems to be something like the next() handler function. I have also aligned to use the exact same underlying http.server as the express adapter with no luck.

getRequestHostname and co. seem to get the correct HonoContext.

EdamAme-x commented 1 month ago

I have been interested in implementing this for some time, thanks for create the issue!

EdamAme-x commented 1 month ago

https://github.com/nestjs/nest/blob/master/packages/platform-fastify/adapters/fastify-adapter.ts

EdamAme-x commented 1 month ago

Is it type safe because the code contains so many any...? Either way I would like to work on this.

EdamAme-x commented 1 month ago
[6:14:45 AM] Starting compilation in watch mode...

[6:14:49 AM] Found 0 errors. Watching for file changes.

[Nest] 5374  - 05/30/2024, 6:14:49 AM   DEBUG [HonoAdapter] constructor
[Nest] 5374  - 05/30/2024, 6:14:49 AM     LOG [NestFactory] Starting Nest application... +3ms
[Nest] 5374  - 05/30/2024, 6:14:49 AM     LOG [InstanceLoader] AppModule dependencies initialized +13ms
[Nest] 5374  - 05/30/2024, 6:14:49 AM   DEBUG [HonoAdapter] initHttpServer
[Nest] 5374  - 05/30/2024, 6:14:49 AM VERBOSE [HonoAdapter] skipping registerParserMiddleware
[Nest] 5374  - 05/30/2024, 6:14:49 AM     LOG [RoutesResolver] AppController {/}: +1ms
[Nest] 5374  - 05/30/2024, 6:14:49 AM     LOG [RouterExplorer] Mapped {/, GET} route +3ms
[Nest] 5374  - 05/30/2024, 6:14:49 AM     LOG [RouterExplorer] Mapped {/hono, GET} route +1ms
[Nest] 5374  - 05/30/2024, 6:14:49 AM   DEBUG [HonoAdapter] setting not found handler
[Nest] 5374  - 05/30/2024, 6:14:49 AM   DEBUG [HonoAdapter] setting error handler
[Nest] 5374  - 05/30/2024, 6:14:49 AM     LOG [NestApplication] Nest application successfully started +0ms
[Nest] 5374  - 05/30/2024, 6:14:49 AM   DEBUG [HonoAdapter] listening on port: 3000
[Nest] 5374  - 05/30/2024, 6:14:49 AM     LOG [Bootstrap] Application is running on: http://[::1]:3000
[Nest] 5374  - 05/30/2024, 6:15:00 AM   DEBUG [HonoAdapter] status: 200
[Nest] 5374  - 05/30/2024, 6:15:00 AM   DEBUG [HonoAdapter] reply: undefined
[Nest] 5374  - 05/30/2024, 6:15:00 AM   DEBUG [HonoAdapter] isHeadersSent
[Nest] 5374  - 05/30/2024, 6:15:00 AM   ERROR [ExceptionsHandler] Cannot read properties of undefined (reading 'get')
TypeError: Cannot read properties of undefined (reading 'get')
    at HonoAdapter.getHeader (/workspaces/nest-hono-adapter/adapter/hono-adapter.ts:246:33)
    at HonoAdapter.reply (/workspaces/nest-hono-adapter/adapter/hono-adapter.ts:143:42)
    at RouterResponseController.apply (/workspaces/nest-hono-adapter/node_modules/@nestjs/core/router/router-response-controller.js:15:36)
    at /workspaces/nest-hono-adapter/node_modules/@nestjs/core/router/router-execution-context.js:176:48
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at /workspaces/nest-hono-adapter/node_modules/@nestjs/core/router/router-execution-context.js:47:13
    at Array.<anonymous> (/workspaces/nest-hono-adapter/node_modules/@nestjs/core/router/router-proxy.js:9:17)
    at responseViaResponseObject (/workspaces/nest-hono-adapter/node_modules/@hono/node-server/dist/index.js:368:13)

/workspaces/nest-hono-adapter/node_modules/@hono/node-server/dist/index.js:207
  for (const [k, v] of headers) {
                       ^
TypeError: headers is not iterable
    at buildOutgoingHttpHeaders (/workspaces/nest-hono-adapter/node_modules/@hono/node-server/dist/index.js:207:24)
    at responseViaResponseObject (/workspaces/nest-hono-adapter/node_modules/@hono/node-server/dist/index.js:374:27)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)

hmm..

EdamAme-x commented 1 month ago

fixing...

EdamAme-x commented 1 month ago

image setted at Async function

EdamAme-x commented 1 month ago

Since there is no typed, it is difficult to determine the cause...

EdamAme-x commented 1 month ago

@drdreo Do you have any docs of this?

drdreo commented 1 month ago

Not sure this issue is the best place to have the debug conversation. But thanks for starting to look into it.

Is it type safe because the code contains so many any

From how they've done it with the express and fastify adapter, i am not sure Nest provides the types (defaults to any), but rather the adapter should https://github.com/nestjs/nest/blob/master/packages/core/adapters/http-adapter.ts#L12

So far i have relied on the old console.log and Node debugger to find the types of objects which are passed around. But as you figured out, there is something odd going on with the objects passed to the adapter (Async Function). Therefore, I have copied the express adapter as well, to compare with a working solution. And that one has completely different objects passed to the same interface functions. Even if the http server under the hood is the same.

Do you have any docs of this?

Unfortunately not. I'm trying to do some 'reverse engineering' to figure out how the adapter is supposed to work.

Perhaps some misconfiguration that i'm missing to tell Nest this adapter is valid. I only found that the notFound handler works and serves requests properly.

EdamAme-x commented 1 month ago

Thanks for your reply. @drdreo It's difficut for me to understand, but I try it for now.

yusukebe commented 1 month ago

Hi @drdreo

It would be amazing to have the choice for honojs in nestjs applications. Not only would it make hono more recognized in the JavaScript world, but also improve the NestJS ecosystem i believe.

This makes sense to me. Great.

What we have to consider it should we include the adapter into the core package like hono/vercel or hono/cloudflare-pages. This is ideal, but it should not dependent on other external libraries if you want to put it into the core. So, if you cant do that, it will be a nice idea to make it as 3rd party middleware or the community middleware which is hosted by the community, not in honojs org.

If I have time, I'll check the code. Thanks!

wenerme commented 1 month ago

I'd like to make this happen, is there any latest code I can copy & paste to try this out ?

I thinks the other way is to make nestjs work like a fetch handler, so I can honoApp.use('/api',createNestJSHandler(nestApp))

EdamAme-x commented 1 month ago

I thinks the other way is to make nestjs work like a fetch handler, so I can honoApp.use('/api',createNestJSHandler(nestApp))

Nest.js is based on express, so it's slow. If we use .use, it will still be slow.

wenerme commented 1 month ago

For me, the usability > speed, I don't want to have tow nest application, one for hono+yoga+type-graphql, one for nestjs+fastify+rest, I hope I can use one unified fetch like application. Event there is hono nestjs adapter, I think I still have to start a hono for yoga, I prefer type-graphql over nestjs and Request&Response like api, just like golang's http handler.

wenerme commented 1 month ago

But if new HonoAdapter() can accept an exiting instance like new HonoAdapter(myHonoApp), maybe this can solve the problem.

EdamAme-x commented 1 month ago

For me, the usability > speed, I don't want to have tow nest application, one for hono+yoga+type-graphql, one for nestjs+fastify+rest, I hope I can use one unified fetch like application. Event there is hono nestjs adapter, I think I still have to start a hono for yoga, I prefer type-graphql over nestjs and Request&Response like api, just like golang's http handler.

Thank for your reply. hmm... I'll measure the speed of "Nest.js on Hono".

drdreo commented 1 month ago

I'd like to make this happen, is there any latest code I can copy & paste to try this out ?

Sorry for my late response, @wenerme you can experiment all you like in my repo https://github.com/drdreo/nest-hono-adapter it includes a test hono app, test nest app and the adapter code

kiyasov commented 1 month ago

If anyone needs an adapter https://github.com/nestjs/nest/issues/13013#issuecomment-2147586323

EdamAme-x commented 1 month ago

If anyone needs an adapter nestjs/nest#13013 (comment)

image

it works and great works bro!!!

EdamAme-x commented 1 month ago

In gitpod (Large) and AutoCannon.

Benchmark Results

Hello World Application

Even for applications that only return "hello world" as a response, the difference is 1.15x. I would like to know the benchmarks for other apps.

yusukebe commented 1 month ago

@EdamAme-x

If you run it on Node.js with @hono/node-server, the speed performance is not so different between Hono and Express. Especially if the application handles the request body like using c.req.json(). But Hono will be fast on non-Node.js environments like Cloudflare, Deno, or Bun. So, I am interested in whether NextJS will work on these runtimes.

EdamAme-x commented 1 month ago

Thank for your reply @yusukebe I'll try it on bun

a4arpon commented 3 weeks ago

Thank you for mentioning my issue. I will do some tests on your repo. Have a great day. But if you want to contact with me in discord based on this issue my username @a4arpon Feel free to knock me.