max-mapper / websocket-stream

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

WebSocket.prototype.addEventListener is not a function in latest Chrome #87

Closed tothandras closed 8 years ago

tothandras commented 8 years ago

An error is thrown at line 50 in stream.js.

> WebSocket.prototype.addEventListener
undefined

Chrome version: 50.0.2661.94 According to MDN addEventListener is not a function on WebSocket, although it still works in Chrome Canary, Mozilla and Safari.

mcollina commented 8 years ago

Would you like to send a PR to address this?

tothandras commented 8 years ago

Will do.

mcollina commented 8 years ago

@tothandras thanks!!

tothandras commented 8 years ago

Should it use addEventListener by default, or just replace those lines altogether?

mcollina commented 8 years ago

We need a solution that works across browsers and in node as well. Once this is achieved, I'm fine.

tothandras commented 8 years ago

I've done nothing, but it's there again:

> WebSocket.prototype.addEventListener
function addEventListener() { [native code] }

Same Chrome version... Yesterday it was missing not just for me, and today the function seems to be back.

mcollina commented 8 years ago

Weird, can we close then?

tothandras commented 8 years ago

I am not sure, the addEventListener is not part of the WebSocket standard, so it can disappear anytime.

mcollina commented 8 years ago

I think you should bring this discussion to the ws module, and we'll follow what they suggest. I am against on having feature detection here (unless we have no alternative). Il giorno mar 3 mag 2016 alle 13:15 András Tóth notifications@github.com ha scritto:

I am not sure, the addEventListener is not part of the WebSocket standard, so it can disappear anytime.

— You are receiving this because you commented.

Reply to this email directly or view it on GitHub https://github.com/maxogden/websocket-stream/issues/87#issuecomment-216498559

tothandras commented 8 years ago

I agree. Will you contact them? (Fun fact: one of my coworker still doesn't have WebSocket.prototype.addEventListener.)

tothandras commented 8 years ago

Oh, I just reread your message, I will bring it up then. :)

tothandras commented 8 years ago

It's there, but I haven't noticed: https://github.com/websockets/ws/blob/master/lib/WebSocket.js#L402-L484. I've updated the PR.