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

[@hono/vite-dev-server] simple bun usage fails #107

Closed
ceopaludetto opened this issue Mar 8, 2024 · 7 comments · Fixed by #119
Closed

[@hono/vite-dev-server] simple bun usage fails #107

ceopaludetto opened this issue Mar 8, 2024 · 7 comments · Fixed by #119

Comments

@ceopaludetto
Copy link

ceopaludetto commented Mar 8, 2024

When I do a simple vite configuration to use with hono, like this:

import { defineConfig } from "vite";
import hono from "@hono/vite-dev-server";

export default defineConfig({
  plugins: [hono({ entry: "./server.ts" })],
});

My server.ts:

import { Hono } from "hono";

const application = new Hono();

application.get("*", async (c) => {
  console.log("HERE");
  return c.text("Hello world!");
});

export default application;

Running with bun --bun vite

I got bun default feedback:

image

But my console.log("HERE") is executed. Reproduction: https://codesandbox.io/p/devbox/bun-vite-hono-repro-dwzcnv

@yusukebe
Copy link
Member

yusukebe commented Mar 9, 2024

Hi @ceopaludetto

That's a known issue, not a bug. @hono/vite-dev-server does not support bun --bun vite because Bun does not completely support Vite. So, please use bun vite or bun run vite.

@yusukebe yusukebe closed this as completed Mar 9, 2024
@ersinakinci
Copy link

@yusukebe @ceopaludetto, I was running into the same problem and I figured out why it's happening.

Here is where Bun defines Response. I don't know Zig, but as you can see, it's aliasing the JS class to a Zig class:

https://github.com/oven-sh/bun/blob/a09c421f2a112e018c4f04b63101af676b36fcd2/src/bun.js/bindings/generated_classes_list.zig#L40

Here is where Bun.serve checks to see whether a response is actually a response. Instead of using JS' instanceof, which would go up the prototype chain, it's checking to see whether the response is the Zig class JSC.WebCore.Response, which is the class to which Bun aliases Response (see above):

https://github.com/oven-sh/bun/blob/a09c421f2a112e018c4f04b63101af676b36fcd2/src/bun.js/api/server.zig#L1329-L1333

@hono/vite-dev-server relies on @hono/node-server, which replaces the Response and Request classes with its own lightweight versions that contain a cache. Here's where the swap occurs for Response:

https://github.com/honojs/node-server/blob/e76c687178f403c524b13651c16580f66b7edeb4/src/response.ts#L83-L85

I think what's going on is that when @hono/node-server replaces the native Response with its own version, it breaks the Bun alias to the Zig class, and Bun.serve's type check fails.

If you comment out the above lines in @hono/node-server, the plugin works, but of course this means that you lose out on the Response caching and whatever other performance benefits you gain from this lightweight class. Then again, it doesn't really matter because you wouldn't typically use @hono/node-server in production w/ Bun; this package is only used for the dev server plugin.

@yusukebe, maybe the best solution is to figure out how to remove the dependency on @hono/node-server. Specifically getRequestListener. Otherwise, we could wrap the offending lines upstream in if (!process.versions.bun) { ... }, but that seems kinda weird since the whole point of that package is to provide compatibility for Node.

@yusukebe
Copy link
Member

yusukebe commented Apr 5, 2024

Hi @ersinakinci

Thank you for investigating and creating the PR. I have some thoughts. I'll try to find the best way after coming back from my trip. Please wait a little.

@ersinakinci
Copy link

No worries! Thanks for your quick reply and for making Hono.

I don't think that my approach in my PR is very good, too much code duplication. But I don't know the internals of Hono or Vite well enough to make a better solution. I hope that it's inspiration.

@ersinakinci
Copy link

ersinakinci commented Apr 8, 2024

One more important finding. I get the same bug during dev when I import anything from honox into my Hono project because honox also imports from @hono/node-server. So this issue seems to affect multiple parts of the Hono ecosystem.

My current workaround is to import what I need from honox directly from source code. In my case, I added the honox project as a submodule in Git to my project's repository, and I do a relative path import from honox/src/.... That way avoids triggering an import of @hono/node-server as a side effect.

@yusukebe
Copy link
Member

yusukebe commented Apr 9, 2024

@ersinakinci

One more important finding. I get the same bug during dev when I import anything from honox into my Hono project because honox also imports from @hono/node-server. So this issue seems to affect multiple parts of the Hono ecosystem.

Does this issue occur when you are running the HonoX app on Bun?

Anyway, as mentioned honojs/node-server#154, if it has overrideGlobalObjects option, the problem will be solved!

@ersinakinci
Copy link

@yusukebe yep it happens when you run bun run --bun vite. Doesn't happen with bun run vite, as expected.

Anyway, as mentioned honojs/node-server#154, if it has overrideGlobalObjects option, the problem will be solved!

Yay! I saw that the PR moves the class override into a function call, so it should no longer execute as a side effect when importing files within @hono/node-server. Thank you, @usualoma!

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