honojs / vite-plugins

Vite Plugins for Hono
https://hono.dev
130 stars 34 forks source link

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

Closed ceopaludetto closed 8 months ago

ceopaludetto commented 8 months ago

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 commented 8 months ago

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.

ersinakinci commented 7 months ago

@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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

@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 https://github.com/honojs/node-server/issues/154, if it has overrideGlobalObjects option, the problem will be solved!

ersinakinci commented 7 months ago

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

Anyway, as mentioned https://github.com/honojs/node-server/issues/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!