Move Node.js serveStatic to this repo

yusukebe commented 6 days ago

I'll propose to move the Node.js Server's Serve Static Middleware in honojs/node-server to this honojs/hono repo.

Create `serveStatic` in `hono/nodejs`

As mentioned in the comment: https://github.com/honojs/node-server/issues/203, we have a base implementation of Serve Static Middleware, but Node.js Server(@hono/node-server developed in honojs/node-server) does not use it. That implements the same logic, and there are lots of duplicated stuff.

We can use the base Serve Static Middleware in the honojs/node-server, but there are problems:

The base Serve Static will be modified in the future.
The Serve Static in honojs/node-server should follow it.
If so, we have to update the peerDependecies(hono package) in package.json of honojs/node-server.

This is the pain point. So I considered and and found that it's good to move the Node.js Server's Server Static to this repo like:

import { serve } from '@hono/node-server'
import { Hono } from 'hono'
import { serveStatic } from 'hono/nodejs'

const app = new Hono()

app.use('/static/*', serveStatic({ root: './' }))

serve(app)

This means we create a Node.js Adapter (not an "Adapter Server") like the adapters for Cloudflare Workers, Deno, and Bun.

Should The Node.js Server be in the independent repo?

@exoego said in the comment:

it is much simpler if Node.js adapter is in Hono main repo. *In this sentence, Node.js adapter means Node.js (Adapter) Server in honojs/node-server.

I can understand that opinion, but the Node.js Server should be honojs/node-server though the Serve Static for Node.js will be in this main repo.

Node.js almost completely has Web Standards APIs like fetch, Request,Respoense, etc. But Node.js does not have an "entry point" for Web Standards APIs.

For Cloudflare Workers, Deno, and Bun, we can run the following app with one command:

export default {
  fetch: () => new Response("Hi!")
}

Below is Bun case:

bun run hello.ts

However, how to run the app on Node.js?? The answer is we have to convert Node.js's HTTP API IncomingMessage and OutgoingMessage to Web Standards' Request and Reponse. The Node.js Server is doing that.

How about making it an adapter in this repo like hono/cloudflare-workers or hono/bun or hono/deno? They are for adapting between differences in runtimes. For example, Serve Static and WebSockets. However, the role of converting Node.js's API between Web Standards is improper. It assumes a greater role. So, keep using hono/node-server is good.

Plus, @hono/node-server is used in other packages like @hono/vite-dev-server as an independent library against hono package.

Next Actions

Shall we move the Server Static in hono/node-server to this repo?

Stop developing the Serve Static in hono/node-server. Close https://github.com/honojs/node-server/pull/202
Mark the Serve Static in hono/node-server as deprecated.
Create serveStatic in hono/nodejs, which uses Serve Static base implementation in this repo.
We probably have to re-design and add implementation to the base Serve Static Middleware.
If it succeeds, it will be easy to follow the base Serve Static Middleware change. We don't have to care about the peerDependcies of @hono/node-server.

yusukebe commented 6 days ago

Hey @usualoma @exoego @sor4chi @nakasyou @watany-dev @ryuapp ant others!

Please share your opinion if you have it. And if you want to implement it, tell us!

nakasyou commented 6 days ago

I basically agree to move Node.js serveStatic to this repo.

As you said, we can change serveStatic API easier. Currently, we have to create at least 2 PR (this repo, Node.js) to change serveStatic interface. I guess one of the reasons to putting Node.js serveStatic adapter on other repo is that the serveStatic adapter depends node: API. However, Bun serveStatic adapter also depends node: API.

Or, to make creating serveStatic easy, I have an idea.

export const serveStatic: ServeStatic = defineServeStatic({
  async open(path: string): ReadableStream<Uint8Array> {
    // ...
  }
})

In this idea, we can bundle common processes in on one place. And modifying API interface is easy.

And, in my opinion, I don't like to move Node.js adapter to this repo. Runtimes which this repo supports as adapter such as Deno, Cloudflare Workers, and Bun are using Request/Response API. In contrast it's necessary to translate Request/Response for using Node.js. The process can't be called as small. So I think it's good that Node.js adapter is in other repo.

exoego commented 6 days ago

I think it is more natural to have the entire Node.js adapter here, since

Translating Node.js's HTTP API to Web Standards' API sounds like the exact role expected for adapter.
This repo already have adapters for aws-lambda, lambda-edge, netlify and so on, which are basically Node.js, IIUC.
It is much simpler for contributors if a common change is needed to many adapters.

But it's OK for me to move only serveStatic middleware, which seems a quickly agreeable option at this moment.

honojs / hono