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

Add support for Ring websocket API (WIP) #30

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

weavejester
Copy link

This PR implements the Ring WebSocket API for this adapter. This is not intended to be merged yet, as the Ring WebSocket API is not yet stable, however I wanted to get some early feedback before the next version of Ring is released.

This PR will eventually close #29.

@yogthos
Copy link
Member

yogthos commented Nov 5, 2023

Overall looks good, and pretty straight forward I think. Only thing I'd add is that it might be nice to add a helper like the following that would just let the users pass a map of functions as the public API:

(defn ws-listener [{:keys [on-open on-message on-pong on-close]}]
  {:ring.websocket/listener
   (reify wsp/Listener
     (on-open [_ sock] (on-open sock))
     (on-message [_ sock mesg] (on-message sock mesg))
     (on-pong [_ sock mesg] (on-pong sock mesg))
     (on-close [_ sock code reason] (on-close sock code reason)))})

@weavejester
Copy link
Author

That's a good idea, and I've already implemented it 😉 . The ring.websocket namespace extends the Listener protocol to cover a map of functions.

@yogthos
Copy link
Member

yogthos commented Nov 5, 2023

awesome, otherwise just ping me whenever new Ring version is out with ws support and we can push this change out along with it then. :)

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.

Ring websocket API support
2 participants