Add Websocket#subscribe() and #unsubscribe() #213

rmm5t · 2018-01-09T06:38:52Z

What does it do?

This PR is stacked on top of @fuzzyTew's PR Add subscribe/unsubscribe functions to websocket. #179.
Instead of a stateful Websocket#[un]subscribeProduct() and #[un]subscribeChannel(), the changes here simplify the method signatures down to just a stateless #subscribe() and #unsubscribe() that is a direct pass through to the GDAX API.
These new method signatures match the GDAX API subscribe/unsubscribe messages directly.

Related Issues

Closes Add subscribe/unsubscribe functions to websocket. #179

Example Usage:

websocket.unsubscribe({ channels: ['full'] });

websocket.subscribe({ product_ids: ['LTC-USD'], channels: ['ticker', 'user'] });

websocket.subscribe({
  channels: [{
    name: 'user',
    product_ids: ['ETH-USD']
  }]
});

websocket.unsubscribe({
  channels: [{
    name: 'user',
    product_ids: ['LTC-USD']
  }, {
    name: 'user',
    product_ids: ['ETH-USD']
  }]
});

OrderbookSync now tracks new product subscriptions.

rmm5t · 2018-01-20T03:52:28Z

Just rebased this (and rewrote parts of the earlier commits) against all the new changes in master.

fb55 · 2018-01-22T16:25:33Z

lib/clients/websocket.js

+    const message = { type };
+
+    if (channels) {
+      message.channels = [...channels];


Can we do this in onOpen? No real need here to create copies of them.

I think the copies were because Sets were being used. I'll kill 'em.

fb55 · 2018-01-22T16:26:59Z

tests/websocket.spec.js

@@ -99,6 +99,117 @@ suite('WebsocketClient', () => {
    });
  });

+  test('subscribes to additional products', done => {
+    var client;
+    const server = testserver(++port, () => {


Didn't we stop incrementing the port?

True. This must have been left over from multiple rebases. I'll fix.

fb55 · 2018-01-22T16:27:25Z

tests/websocket.spec.js

@@ -99,6 +99,117 @@ suite('WebsocketClient', () => {
    });
  });

+  test('subscribes to additional products', done => {
+    var client;


Good catch. I'll fix.

fb55 · 2018-01-22T16:27:57Z

lib/clients/websocket.js

@@ -17,13 +17,11 @@ class WebsocketClient extends EventEmitter {
    { channels = null } = {}
  ) {
    super();
-    this.productIDs = Utils.determineProductIDs(productIDs);
+    this.productIDs = new Set(Utils.determineProductIDs(productIDs));


Not sure what these sets are used for — why aren't we using plain arrays?

Sets are a better construct for unique lists (obviously), but if you don't like them, I'll clean them out.

rmm5t · 2018-01-22T16:51:03Z

@fb55 Requested changes applied.

fb55

One more nit, otherwise lgtm

fb55 · 2018-01-22T23:04:45Z

lib/orderbook_sync.js

+    const channel = data.channels.find(c => c.name === 'full');
+    channel && channel.product_ids
+      .filter(productID => !(productID in this.books))
+      .forEach(productID => this._newProduct(productID));


Use same pattern as above (this.productIDs.forEach(this._newProduct, this);)?

I dig it. Good eye. Done.

fb55 · 2018-01-22T23:32:04Z

Awesome, thanks :)

rmm5t mentioned this pull request Jan 9, 2018

Add subscribe/unsubscribe functions to websocket. #179

Closed

fuzzyTew and others added 3 commits January 19, 2018 20:48

Add subscribe/unsubscribe functions to websocket.

85f714a

OrderbookSync now tracks new product subscriptions.

Updated websocket client usage in README.

187d67c

Add WebsocketClient#subscribe() and #unsubscribe()

2fd0dda

rmm5t force-pushed the websocket-subscribe-unsubscribe branch from c78031e to 2fd0dda Compare January 20, 2018 03:50

fb55 reviewed Jan 22, 2018

View reviewed changes

rmm5t added 3 commits January 22, 2018 09:32

Avoid incrementing the port in test suite

e8ea4fe

Remove var keyword in tests

d99da97

Remove use of Sets in WebsocketClient

c0fd6ca

fb55 approved these changes Jan 22, 2018

View reviewed changes

Prefer thisArg where possible while using Array.prototype.forEach

df6d38c

fb55 merged commit c14bdab into coinbase:master Jan 22, 2018

rmm5t deleted the websocket-subscribe-unsubscribe branch January 22, 2018 23:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Websocket#subscribe() and #unsubscribe() #213

Add Websocket#subscribe() and #unsubscribe() #213

rmm5t commented Jan 9, 2018

rmm5t commented Jan 20, 2018 •

edited

Loading

fb55 Jan 22, 2018

rmm5t Jan 22, 2018

fb55 Jan 22, 2018

rmm5t Jan 22, 2018

fb55 Jan 22, 2018

rmm5t Jan 22, 2018

fb55 Jan 22, 2018 •

edited

Loading

rmm5t Jan 22, 2018

rmm5t commented Jan 22, 2018

fb55 left a comment

fb55 Jan 22, 2018

rmm5t Jan 22, 2018

fb55 commented Jan 22, 2018

Add Websocket#subscribe() and #unsubscribe() #213

Add Websocket#subscribe() and #unsubscribe() #213

Conversation

rmm5t commented Jan 9, 2018

What does it do?

Related Issues

Example Usage:

rmm5t commented Jan 20, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fb55 Jan 22, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rmm5t commented Jan 22, 2018

fb55 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fb55 commented Jan 22, 2018

rmm5t commented Jan 20, 2018 •

edited

Loading

fb55 Jan 22, 2018 •

edited

Loading