node-server icon indicating copy to clipboard operation
node-server copied to clipboard

Consider using serveStatic base implementation

Open exoego opened this issue 1 year ago • 5 comments

node-server implements its own serverStatic middleware. https://github.com/honojs/node-server/blob/74e86a28f375e5acd52e342519ff2b1110a95c16/src/serve-static.ts

However, serveStatic base implementation is provided in Hono main repo: https://github.com/honojs/hono/blob/c0d782cd649525ce40489906a24d83607deede29/src/middleware/serve-static/index.ts The base implementation is used by Bun adapter Deno adapter, which drastically eases implementing new feature like https://github.com/honojs/hono/pull/3461

It is great if node-server also uses the base implementation.

exoego avatar Oct 01 '24 03:10 exoego

Hi @exoego

I also think this is great to use the serveStatic middleware in the hono package. One thing I care about is we have to update the peerDependencies version of the hono package. It should be a higher version than the serveStatic middleware that was implemented. But it has been time after that; it's time to work on it.

yusukebe avatar Oct 01 '24 03:10 yusukebe

Hmmm. BUT, the serveStatic in hono package keeps updating. So, it may be hard to follow the update in this @hono/node-server adapter. Hmm.

yusukebe avatar Oct 01 '24 06:10 yusukebe

From a contributor perspective, it is much simpler if Node.js adapter is in Hono main repo. So my preference is:

  • Merge @hono/node-server into the core repo under the new package name hono/nodejs or something, as same as hono/bun and hono/deno does so. It allows Node.js adapter evolves with Hono and other adapters.
  • Then, deprecate @hono/node-server in favor of hono/nodejs
  • Then, transfer the issues from here to Hono main repo and archive this repo.

Sorry in advance, if that was already considered and abandoned before. I don't know the history why @honojs/node-server has its own repo.

exoego avatar Oct 01 '24 06:10 exoego

@exoego

Thank you for sharing your opinion! I can understand it well. We/I have some reasons and the historical stuff. I'll explain later.

yusukebe avatar Oct 01 '24 06:10 yusukebe

@exoego

Added the issue related to this discussion https://github.com/honojs/hono/issues/3483

Please take a look at it!

yusukebe avatar Oct 03 '24 07:10 yusukebe