gornostay25 / svelte-adapter-bun

A SvelteKit adapter for Bun
MIT License
534 stars 33 forks source link

Can't use Bun websocket publish() or subscribe() #66

Open dukejones opened 2 months ago

dukejones commented 2 months ago

The current method works well enough for a single client of a websocket. But the main use case of websockets (especially with HTTP2 solving the head-of-line blocking for many requests) is to be able to publish to all of the subscribers.

Bun includes a simple pub/sub facility in its websockets. For this to work, you call .publish() on the server instance. https://bun.sh/docs/api/websockets#pub-sub

The way svelte-adapter-bun works, there's no way to get a server instance from the websocket handlers. It should be possible from something like .upgrade(), save that ref locally in the websocketHandler file, but the websocket handler / upgrade method has been patched to receive only the upgrade method from the server rather than the entire server.

Bun.serve({
  fetch(req, server) {
    const sessionId = await generateSessionId();
    server.upgrade(req, {
      headers: {
        "Set-Cookie": `SessionId=${sessionId}`,
      },
    });
  },
  websocket: {}, // handlers
});

^^^ instead of server, .upgrade() receives just the server.upgrade() function.

I'd suggest getting rid of the custom .upgrade() function in line with the other websocket handlers -- it's a special hack that makes it hard to debug, coming from the Bun docs into the svelte-adapter-bun codebase.

Not exactly sure how you'd go about wiring up the upgrade etc., and it'd be a big-ish refactor. So I'm afraid I'm dropping back to Node and using a slower websocket library for now.

I'll be checking back in~ wishing this project much success.

KyleFontenot commented 1 month ago

Help me understand if this doesnt answer your question. I think you might just be mixing up the uses for subscribe, upgrade, and publish.

.upgrade() belongs to Server which is the second parameter in .fetch() so yes you can access the server instance within the fetch method. Why would you need the upgrade() method from within the websocket handler, since the handler is only for upgraded connections? You could pass in the server instance into the upgrade's data object, but that might dirty it up a little.

.upgrade() is in Bun.fetch, while .subscribe() and publish() are in the websocket handler methods.

Bun.serve({
  fetch: (req: Request, server: Server) => {
    server.upgrade(req, {
      data: {
        server
      }
    });
  },
  websocket: {
    open(ws: ServerWebSocket) {
      ws.subscribe('chatroom')
    },
    message(ws: ServerWebSocket, msg: string | Buffer) {
      console.log(msg.toString());
    },
  },
});

Bun types clarifies it a little

dukejones commented 1 month ago

Thanks for taking the time to write this up.. i'll try to find the time soon to dig up the old code and test out what you are saying.