search

max-mapper / websocket-stream

websockets with the node stream API
BSD 2-Clause "Simplified" License
668 stars 114 forks source link

Support newer browsers not implementing addEventListener #88

Closed tothandras closed 8 years ago

tothandras commented 8 years ago

Fixes #87 Replace addEventListener calls with onopen, onclose, onerror, onmessage.

mcollina commented 8 years ago

why can't we just use this.onopen() and the like?

I'm 👎 on monkey patching the socket object.

tothandras commented 8 years ago

Alright, that's why I asked. :) And what about the changes?

mcollina commented 8 years ago

Changed are ok. However I will prefer to have a single way to set the events. Is that possible? Why can't we always use onopen  and the like?

tothandras commented 8 years ago

I don't see onopen and the like here: https://github.com/websockets/ws/blob/master/lib/WebSocket.js. :/ Not true: https://github.com/maxogden/websocket-stream/issues/87#issuecomment-216544499

tothandras commented 8 years ago

Anything else? @mcollina

mcollina commented 8 years ago

Let's wait @mafintosh and @maxogden :)