Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Consider using serveStatic base implementation #203

Open
exoego opened this issue Oct 1, 2024 · 5 comments
Open

Consider using serveStatic base implementation #203

exoego opened this issue Oct 1, 2024 · 5 comments

Comments

@exoego
Copy link

exoego commented Oct 1, 2024

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 honojs/hono#3461

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

@yusukebe
Copy link
Member

yusukebe commented Oct 1, 2024

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
Copy link
Member

yusukebe commented Oct 1, 2024

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.

@exoego
Copy link
Author

exoego commented Oct 1, 2024

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.

@yusukebe
Copy link
Member

yusukebe commented Oct 1, 2024

@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
Copy link
Member

yusukebe commented Oct 3, 2024

@exoego

Added the issue related to this discussion honojs/hono#3483

Please take a look at it!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants