Expose socket events #1
Conversation
- Implements the peek-buried operation. - The connect() promise rejects if connection can't be made. - Adds support for listening to key socket events to enable reconnect logic. - Adds isConnected() operation so that consumers don't need to wrap the client just to determine when the client has lost connection.
| * @param listener | ||
| */ | ||
| JackdClient.prototype.on = function(event, listener) { | ||
| if (SOCKET_EVENTS.includes(event)) { |
There was a problem hiding this comment.
The way this is written, JackdClient could never be an eventemitter on it's own. Not sure that is a problem or not.
There was a problem hiding this comment.
It uses an emitter internally to trigger the callbacks passed in (see the createCommandHandler pattern used throughout). But it wasn't emitting those to consumers as there was no .on() function available to register.
I originally planned to allow consumers to the internal emitter (which is why I accidentally left in the SOCKET_EVENTS array and check) but I decided that was needless scope creep. And the check for 'if (SOCKET_EVENTS.includes(event)) {' doesn't hurt anything.
There was a problem hiding this comment.
After going back through the original code, I think that makes sense. Seems simpler to use callbacks in this case than exposing the eventemitter.
A few updates:
- Implements the peek-buried operation.
- The connect() promise rejects if connection can't be made.
- Adds support for listening to key socket events to enable reconnect logic.
- Adds isConnected() operation so that consumers don't need to wrap the client just
to determine when the client has lost connection.