Skip to content
New issue

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

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

Already on GitHub? Sign in 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!

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

No branches or pull requests

2 participants