Closed sebpiq closed 7 years ago
For this I would suggest several solutions :
1- using one these EventTarget
implementations : https://www.npmjs.com/search?q=eventtarget
2- just removing all the event target stuff and providing simple callbacks onmessage
, onclose
... the only difference with the current implementation is that you won't be able to bind several event handlers, which in my opinion is totally OK.
3- using a competely different type of event emitter, e.g. https://github.com/asyncly/EventEmitter2
Personally I'd strongly recommend 2 as it would allow to simplify the code and focus on what really matters here : reconnecting websockets :) let users choose which event emitter they want to use if they wanna use one.
I need this quite quickly, so tell me your preference, and I'll make a pull request.
@sebpiq: I need more time to think about this. If you need this urgently, my suggestion is to fork it and do what what makes sense for you - we can resolve the differences later.
It is actually not as urgent as I first thought (for now I have custom reconnect code that works fine) ... but I'll definitely want to use a library at some point. I'd rather wait for your opinion on this before doing some work. I don't like forks so much unless they're really necessary, or unless they're going to be merged :)
I usually try to make libraries that are super focused, do only one thing, and have as few dependencies as possible, which usually makes them much more reliable and less opaque to users (which can understand the workings with a very quick look at the source). The way I see it, is that if you want to keep the API you have at the moment, you'll probably have to sacrifice maintainability and simplicity. So imho, this really is a question of what you think is higher priority.
Let me know when you know more about what you want to do there :)
We are using this package.json dependency:
"dependencies": { "ReconnectingWebSocket": "git+https://github.com/joewalnes/reconnecting-websocket.git",
and got the following error in our command line interface:
~/ReconnectingWebSocket/reconnecting-websocket.js:105
if (!('WebSocket' in window)) {
^
ReferenceError: window is not defined
The work around was to only load reconnecting-websocket if the window object was available. It would be nice if the package could run in nodejs, it will allow our command line interface to re-connect in the same way the web interface can re-connect.
Is there any closer solution for nodejs compatibility ?
This is a browser library, not a Node library. To my knowledge the Node websocket library has a different API so this won't work. On Tue, Oct 20, 2015 at 7:11 AM Ekrem Hajredini notifications@github.com wrote:
Is there any closer solution for nodejs compatibility ?
— Reply to this email directly or view it on GitHub https://github.com/joewalnes/reconnecting-websocket/issues/52#issuecomment-149545874 .
There is no single node WebSocket library, however, WebSocket does have a standard API, and a library like https://github.com/websockets/ws implements this very well for node.js
It will definitely work if we use only the standard WebSocket API and ES5 (vanilla Javascript). Basically what this requires is getting rid of all the browser APIs used here. I think that would also require using less syntactic sugar (no fancy event emitters and stuff).
I will draft a new version in the next 2 weeks and if you are interested @joewalnes we can push it back to master or simply releasing it as a separate library if you feel that this is out of the scope of this one.
I have implement web socket on node js (as a client, not sever, it's a client of another server), but there is no other reconnect-websocket library
Thanks for your thoughts. I think this will be best implemented as a separate Node specific library.
I'm in need to for a (reconnecting) WebSocket that can be used both in the browser and in node.js.
How about allowing to pass the WebSocket
object as an option to ReconnectingWebsocket
? So you could do something like:
var url = 'ws://myserver.com';
var protocols = null;
var options = {};
var isNode = ... // detect whether in a node.js environment
if (isNode) { // node.js
options.WebSocket = require('ws'); // load your favorite node.js WebSocket library
}
var rws = new ReconnectingWebSocket(url, protocols, options);
@ehajredini do you have a repository or patch? We still need this, our c++ guies have to bounce our nodejs server every time they compile and launch their code. This is definitely needed in production soon too.
We also use this lib for our browser and it has been very stable. It does not have to be the same library but some similarities will make it easier to learn and manage.
Is there any progress on this? Dependency on the browser for this kind of thing, is just hmmmmm
Any way to use this library in nodejs?
Adding Node support will add additional complexity and testing. There are no additional plans to support this.
@joewalnes so how about my suggestion here https://github.com/joewalnes/reconnecting-websocket/issues/52#issuecomment-188286780 to allow passing a custom WebSocket
as option? That wouldn't require much effort and gives a lot of flexibility I think.
I would like to use this in node (with node-ws) but it implies getting rid of the "browser-only" bits of code like the use of a div as an event emitter and the use of
document
. This would also fix #51