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

feat(WebSocket): Add Pub/Sub feature #3466

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

hayatosc
Copy link
Contributor

@hayatosc hayatosc commented Sep 29, 2024

relate to #3230

Desctiption

This PR will allow us to use Publish/Subscribe messaging model on WebSocket like Bun can do natively.

Demo

Bun

Bun has own Pub/Sub API.

import { Hono } from 'hono'
import { createBunWebSocket } from 'hono/bun'
import type { ServerWebSocket } from 'bun'

const app = new Hono()
const { upgradeWebSocket, websocket } =
  createBunWebSocket<ServerWebSocket>()

app.get(
  '/ws',
  upgradeWebSocket((c) => {
    return {
      onMessage(event, ws) {
        ws.subscribe('chatroom')
        server.publish('Hello!')
      }
    }
  })
)

const server = Bun.serve({
  fetch: app.fetch,
  websocket,
})

Other runtimes

Other runtimes such as Deno will get minimal PubSub class.

import { Hono } from 'hono'
import { upgradeWebSocket } from 'hono/deno'
import { WSPubSub } from 'hono/websocket'

const app = new Hono()
const pubsub = new WSPubSub()

app.get(
  '/ws',
  upgradeWebSocket(pubsub, (c) => {
    return {
      onMessage(event, ws) {
        ws.subscribe('chatroom')
        pubsub.publish('Hello!')
      }
    }
  })
)

I don't know this method of implementation will accept all people. So if you have the opinion, please comment.

The author should do the following, if applicable

  • Add tests
  • Run tests
  • bun run format:fix && bun run lint:fix to format the code
  • Add TSDoc/JSDoc to document the code

@hayatosc
Copy link
Contributor Author

How do you think, @yusukebe @nakasyou ?

I think the implementation of Other runtime may have a more good solution.

@nakasyou
Copy link
Contributor

nakasyou commented Sep 29, 2024

It looks very good to me. I like it.

I have an idea.
How about like that?:

const chatroom1 = new PubSub()
app.get(
  '/ws',
  upgradeWebSocket((c) => {
    return {
      onMessage(event, ws) {
        chatroom1.publish('Hello World')
      },
      pubSub: chatroom1
    }
  })
)

I think it's more extendable.

And in my opinion, I think we have to avoid extending WSContext and WSEvents for each runtime. I have no idea to implement it, but I think it should be made clear that the API is runtime-specific.

@hayatosc
Copy link
Contributor Author

hayatosc commented Sep 29, 2024

@nakasyou

It looks very good to me. I like it.

I have an idea. How about like that?:

const chatroom1 = new PubSub()
app.get(
  '/ws',
  upgradeWebSocket((c) => {
    return {
      onMessage(event, ws) {
        chatroom1.publish('Hello World')
      },
      pubSub: chatroom1
    }
  })
)

I think it's more extendable.

And in my opinion, I thinkwe have to avoid extending WSContext and WSEvents for each runtime. I have no idea to implement it, but I think it should be made clear that the API is runtime-specific.

Hmm. Your opinion was not bad for me.

However, most of actual use cases need multiple topics, so there is a risk that the completed code will become complicated.

app.get(
  '/ws/:id',
  upgradeWebSocket((c) => {
    return {
      onMessage(event, ws) {
        const { id } = c.req.param()
        pubsub.subscribe(`chatroom-${id}`) // we want to do this
      }
    }
  })
)

Individually, also in the context of code compatibility, I feel we should follow Bun's Pub/Sub API as possible.

@nakasyou
Copy link
Contributor

nakasyou commented Sep 30, 2024

@hayatosc

However, most of actual use cases need multiple topics, so there is a risk that the completed code will become complicated.

I agree with you.
So, what do you think about that?

// Bun
const {
  upgradeWebSocket, 
  websocket,
  pubSub // Provides pubSub instance for Bun
} = createBunWebSocket<ServerWebSocket>()
app.get(
  '/ws/:id',
  upgradeWebSocket((c) => {
    const { id } = c.req.param()
    return {
      onOpen(_evt, ws) {
        ws.subscribe(`chatroom-${id}`) // Use received `pubSub`
      },
      onMessage(event, ws) {
        pubsub.publish(`chatroom-${id}`, event.data)
      },
      pubSub
    }
  })
)
// Other runtimes
const pubSub = new PubSub()
app.get(
  '/ws/:id',
  upgradeWebSocket((c) => {
    const { id } = c.req.param()
    return {
      onOpen(_evt, ws) {
        ws.subscribe(`chatroom-${id}`) // Use received `pubSub`
      },
      onMessage(event, ws) {
        pubsub.publish(`chatroom-${id}`, event.data)
      },
      pubSub
    }
  })
)

In this idea, users can switch runtime easier. And it doesn't extends WSContext and WSEvents for only Bun.

@moshOntong-IT
Copy link

I like this when we will have this?

@moshOntong-IT
Copy link

@hayatosc

However, most of actual use cases need multiple topics, so there is a risk that the completed code will become complicated.

I agree with you.

So, what do you think about that?

// Bun

const {

  upgradeWebSocket, 

  websocket,

  pubSub // Provides pubSub instance for Bun

} = createBunWebSocket<ServerWebSocket>()

app.get(

  '/ws/:id',

  upgradeWebSocket((c) => {

    const { id } = c.req.param()

    return {

      onOpen(_evt, ws) {

        ws.subscribe(`chatroom-${id}`) // Use received `pubSub`

      },

      onMessage(event, ws) {

        pubsub.publish(`chatroom-${id}`, event.data)

      },

      pubSub

    }

  })

)
// Other runtimes

const pubSub = new PubSub()

app.get(

  '/ws/:id',

  upgradeWebSocket((c) => {

    const { id } = c.req.param()

    return {

      onOpen(_evt, ws) {

        ws.subscribe(`chatroom-${id}`) // Use received `pubSub`

      },

      onMessage(event, ws) {

        pubsub.publish(`chatroom-${id}`, event.data)

      },

      pubSub

    }

  })

)

In this idea, users can switch runtime easier. And it doesn't extends WSContext and WSEvents for only Bun.

when we can have this ?

@nakasyou
Copy link
Contributor

nakasyou commented Nov 5, 2024

@moshOntong-IT To bypass it, you can write code like

const wses = new Set()

app.get('/ws', upgradeWebSocket(() => ({
  onOpen(_, ws) {
    wses.add(ws)
  },
  onClose(_, ws) {
    wses.delete(ws)
  }
})))

// Publish
for (const ws of wses) {
  ws.send('Hello')
}

@moshOntong-IT
Copy link

@moshOntong-IT To bypass it, you can write code like

const wses = new Set()



app.get('/ws', upgradeWebSocket(() => ({

  onOpen(_, ws) {

    wses.add(ws)

  },

  onClose(_, ws) {

    wses.delete(ws)

  }

})))



// Publish

for (const ws of wses) {

  ws.send('Hello')

}

I am using bun is this still compatible?

@nakasyou
Copy link
Contributor

nakasyou commented Nov 5, 2024

@moshOntong-IT

I am using bun is this still compatible?

Yes!

@hayatosc
Copy link
Contributor Author

hayatosc commented Nov 5, 2024

@moshOntong-IT To bypass it, you can write code like

const wses = new Set()



app.get('/ws', upgradeWebSocket(() => ({

  onOpen(_, ws) {

    wses.add(ws)

  },

  onClose(_, ws) {

    wses.delete(ws)

  }

})))



// Publish

for (const ws of wses) {

  ws.send('Hello')

}

I am using bun is this still compatible?

I think Bun Pub/Sub API will work fine for you than Set object.

import { Hono } from 'hono'
import { createBunWebSocket } from 'hono/bun'
import type { ServerWebSocket } from 'bun'

const app = new Hono()
const { upgradeWebSocket, websocket } =
  createBunWebSocket<ServerWebSocket>()

app.get(
  '/ws',
  upgradeWebSocket((c) => {
    return {
      onMessage(event, ws) {
        ws.raw.subscribe('chatroom')
        server.publish('Hello!')
      }
    }
  })
)

const server = Bun.serve({
  fetch: app.fetch,
  websocket,
})

@hayatosc
Copy link
Contributor Author

hayatosc commented Nov 5, 2024

@nakasyou 's #3531 PR will be good for creating this feature. I'm too busy to work this now but near future I'll back it.

@moshOntong-IT
Copy link

@moshOntong-IT To bypass it, you can write code like

const wses = new Set()



app.get('/ws', upgradeWebSocket(() => ({

  onOpen(_, ws) {

    wses.add(ws)

  },

  onClose(_, ws) {

    wses.delete(ws)

  }

})))



// Publish

for (const ws of wses) {

  ws.send('Hello')

}

I am using bun is this still compatible?

I think Bun Pub/Sub API will work fine for you than Set object.

import { Hono } from 'hono'
import { createBunWebSocket } from 'hono/bun'
import type { ServerWebSocket } from 'bun'

const app = new Hono()
const { upgradeWebSocket, websocket } =
  createBunWebSocket<ServerWebSocket>()

app.get(
  '/ws',
  upgradeWebSocket((c) => {
    return {
      onMessage(event, ws) {
        ws.raw.subscribe('chatroom')
        server.publish('Hello!')
      }
    }
  })
)

const server = Bun.serve({
  fetch: app.fetch,
  websocket,
})

This is actually works but how can I access the server in different file?

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

Successfully merging this pull request may close these issues.

3 participants