Implement support for notification namespaces #11
Conversation
|
Not sure how much to describe for listen/unlisten in the docs. There didn't seem to be much to mention in the end. |
|
|
||
| > *`void`* [**`session.listen`**](#session.listen) `(...namespaces)` | ||
|
|
||
| Listens to notifications on the given `namespaces` variable argument list. |
There was a problem hiding this comment.
We probably want to include info about why you need to call listen in the first place, and talk about how it is safe to call listen with a namespace you're already listening to etc. etc.
The "Managed module usage" section of the docs also makes use of notifications, and we probably need to update that to show that you have to listen to the appropriate namespace as well.
|
|
||
| > *`void`* [**`session.unlisten`**](#session.unlisten) `(...namespaces)` | ||
|
|
||
| Unlistens from notifications on the given `namespaces` variable argument list. |
There was a problem hiding this comment.
Once again, we probably need to make note of the fact that you can call unlisten safely with namespaces that you're not already listening to.
| type | ||
| ], | ||
| [[{type: type, payload: payload}]] | ||
| [[{namespace: namespace, type: type, payload: payload}]] |
There was a problem hiding this comment.
I guess for consistency, we should probably log listens and unlistens more like this. That is, we don't JSON.stringify(), but rather log the values directly, as a "secondary".
| var namespaces = header[0] | ||
|
|
||
| if (!Array.isArray(namespaces)) throw new Error('Invalid NOTIFICATION_LISTEN message header (namespaces).') | ||
| namespaces.map(function (namespace) { |
There was a problem hiding this comment.
We should probably use forEach, instead of map, as we're not actually intending to build a new array.
| var namespaces = header[0] | ||
|
|
||
| if (!Array.isArray(namespaces)) throw new Error('Invalid NOTIFICATION_UNLISTEN message header (namespaces).') | ||
| namespaces.map(function (namespace) { |
There was a problem hiding this comment.
Same here, replace map with forEach.
|
Changes made based on review comments. |
Fixes #9