search

sitegui / nodejs-websocket

A node.js module for websocket server and client
MIT License
736 stars 155 forks source link

Suggestion: alias 'send' to 'sendText' #27

Closed itpmngt closed 8 years ago

itpmngt commented 8 years ago

Love the simplicity - Zen style - of this solution. I would like to suggest to create an alias to 'sendText' so to be in line with other libraries - they use 'send' as the default for text messages.

It is a minor request but well, I thought to make this suggestion

itpmngt commented 8 years ago

And in line with this suggestion, to emit('message') instead of ('text') on Connection line 396 - maybe in a new version because it breaks, again is a bit confusing for who is used to work with websockets.

I can appreciate that the development of this module started a while ago (when probably there were no common-uses) but I don't understand why this module is not on the top-lists around the Internet - maybe people like complicated...

sitegui commented 8 years ago

Hello @itpmngt

Thank you for taking your time to open this issue.

We may take the idea of aliasing one step further and make it an abstraction over both sendText and sendBinary. That is: send(aString) calls sendText and send(aBuffer) calls sendBinary. What do you think?

For the event name change, I don't think it's worth breaking other people code without a clear benefit. The current terminology was grabbed from the WebSocket spec itself. This module was designed to be a low-level implementation for WS, that's why I think it's good to keep the names similar to the spec (and not much with other implementations).

The simplicity of this module comes from the fact it implements a subset of the WS spec that I find most useful. But one size does not fit all ;)

Regards

itpmngt commented 8 years ago

Hello sitegui

Yes, a simple send() fits spot on. Feels great. And I do understand and agree with the 'breaking code' issue regarding 'text' to 'message'

Going into production next week with this WSS - exciting times

scripting commented 8 years ago

As someone who has deployed this package, I would not install a new version if it broke my existing implementation.

That's the value of a package like this. The standard is stable. It's great to know that the design of this package was guided by the terminology of the standard. Keep it that way! :-)

And thanks. It works great for my blog, readers get instant updates as I make changes to stories. It's like magic to them (and me actually too).

Dave

On Sun, Jan 10, 2016 at 12:34 PM, Joe Rosa notifications@github.com wrote:

Hello sitegui

Yes, a simple send() fits spot on. Feels great. And I do understand and agree with the 'breaking code' issue regarding 'text' to 'message'

Going into production next week with this WSS - exciting times

— Reply to this email directly or view it on GitHub https://github.com/sitegui/nodejs-websocket/issues/27#issuecomment-170373269 .

sitegui commented 8 years ago

Thank you for the feedback guys, released as minor 1.5.0