enisdenjo / graphql-ws

Coherent, zero-dependency, lazy, simple, GraphQL over WebSocket Protocol compliant server and client.
https://the-guild.dev/graphql/ws
MIT License
1.75k stars 162 forks source link

rename onMessage in server to postMessage, withMessage (or something more appropriate) #515

Closed fridaystreet closed 1 year ago

fridaystreet commented 1 year ago

Story

onMessage implies this is a hook who's callback will be passed a message. When in fact it provides a callback to use to post a message into the server for processing. This has been quite confusing when trying to build integration with this library. In particular because it is so integrated with websocket libraries who all have similar 'onMessage' hooks, the intent of which are effectively the complete opposite of this.

even having 'on' in the name is a bit of a misdirection, as you are expenting it to be fired onMessage, not 'withMessage' as this is.

Just a suggestion that it might be better if this was named postMessage, or processMessage, or withMessage maybe?

Acceptance criteria

N/A

enisdenjo commented 1 year ago

It is intentionally named onMessage because it is used to set up the "on message" event handling. I personally think the name's fine. However, even if not, I don't think changing it is worth a breaking change.