{"action":"created","issue":{"url":"https://api.github.com/repos/jbmusso/gremlin-javascript/issues/50","repository_url":"https://api.github.com/repos/jbmusso/gremlin-javascript","labels_url":"https://api.github.com/repos/jbmusso/gremlin-javascript/issues/50/labels{/name}","comments_url":"https://api.github.com/repos/jbmusso/gremlin-javascript/issues/50/comments","events_url":"https://api.github.com/repos/jbmusso/gremlin-javascript/issues/50/events","html_url":"https://github.com/jbmusso/gremlin-javascript/pull/50","id":148835535,"number":50,"title":"[WIP] Handle all things asynchronous internally with RxJS","user":{"login":"jbmusso","id":203476,"avatar_url":"https://avatars.githubusercontent.com/u/203476?v=3","gravatar_id":"","url":"https://api.github.com/users/jbmusso","html_url":"https://github.com/jbmusso","followers_url":"https://api.github.com/users/jbmusso/followers","following_url":"https://api.github.com/users/jbmusso/following{/other_user}","gists_url":"https://api.github.com/users/jbmusso/gists{/gist_id}","starred_url":"https://api.github.com/users/jbmusso/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jbmusso/subscriptions","organizations_url":"https://api.github.com/users/jbmusso/orgs","repos_url":"https://api.github.com/users/jbmusso/repos","events_url":"https://api.github.com/users/jbmusso/events{/privacy}","received_events_url":"https://api.github.com/users/jbmusso/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":0,"created_at":"2016-04-16T09:07:41Z","updated_at":"2016-04-16T09:12:36Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/jbmusso/gremlin-javascript/pulls/50","html_url":"https://github.com/jbmusso/gremlin-javascript/pull/50","diff_url":"https://github.com/jbmusso/gremlin-javascript/pull/50.diff","patch_url":"https://github.com/jbmusso/gremlin-javascript/pull/50.patch"},"body":"I'm trying to lower the size of the dependency as well as make it easier to add new features (throttling, auto reconnect, etc.). My goal is:\r\n- to remove `highland` and `readable-stream` dependencies\r\n- to add `rx` lightweight dependency since I'm only using very few Rx operators internally\r\n\r\nAll incoming and outgoing messages are now handled with RxJS streams. I'm unsure about the performance but I don't think this should be a concern when compared to network I/O.\r\n\r\n`Observable` are essentially higher level streams that can be converted to Node.js `Stream`. They are currently a [stage-1 candidate](https://github.com/zenparsing/es-observable) for addition to the EcmaScript standard. Upcoming RxJS v5 Observable tries to be compliant with the EcmaScript Observable Spec. I added `rx` v4 dependency since it's stable and offers more higher level methods absent in `rxjs` v5, especially `.pausableBuffer()` (absent in v5) which is quite handy for buffering outgoing messages until the WebSocket connection is actually open. I should add that regarding package names, `rx` is stable v4 on npm while `rxjs` is experimental/beta v5 (or so I figured ?!).\r\n\r\nThis cleans up the code quite a bit and also removes the `executeHandler` thing which I've never been a fan of.\r\n\r\nMost tests pass, except those related to the previous `Stream` API obviously (`.stream()` and `.messageStream()`). I currently added a public (though undocumented yet) `.observable()` method which currently returns an RxJS stream. `.execute()` now calls `.observable()` internally.\r\n\r\nQuestion : do people even use `.stream()` and `.messageStream()`? Even though these are public and documented methods, they've also been used internally by `.execute()` (which I believe most people use). By using RxJS, these two stream methods will no longer be necessary internally and I could just drop them from the public API altogether. It's also worth noting that `Observable` can be easily converted to Node.js `Streams` with a third party library https://github.com/Reactive-Extensions/rx-node (I don't think it's worth adding a dependency for this considering how trivial it is).\r\n\r\nThis PR is heavily experimental and I'll most likely send another clean PR when confident. This will most likely be a major version bump (3.x) depending on whether we want to keep the old Stream API or not.\r\n\r\nThoughts welcomed. Yeah, I'm looking at you @PommeVerte ;)."},"comment":{"url":"https://api.github.com/repos/jbmusso/gremlin-javascript/issues/comments/210778722","html_url":"https://github.com/jbmusso/gremlin-javascript/pull/50#issuecomment-210778722","issue_url":"https://api.github.com/repos/jbmusso/gremlin-javascript/issues/50","id":210778722,"user":{"login":"coveralls","id":2354108,"avatar_url":"https://avatars.githubusercontent.com/u/2354108?v=3","gravatar_id":"","url":"https://api.github.com/users/coveralls","html_url":"https://github.com/coveralls","followers_url":"https://api.github.com/users/coveralls/followers","following_url":"https://api.github.com/users/coveralls/following{/other_user}","gists_url":"https://api.github.com/users/coveralls/gists{/gist_id}","starred_url":"https://api.github.com/users/coveralls/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/coveralls/subscriptions","organizations_url":"https://api.github.com/users/coveralls/orgs","repos_url":"https://api.github.com/users/coveralls/repos","events_url":"https://api.github.com/users/coveralls/events{/privacy}","received_events_url":"https://api.github.com/users/coveralls/received_events","type":"User","site_admin":false},"created_at":"2016-04-16T09:12:36Z","updated_at":"2016-04-16T09:12:36Z","body":"\n[![Coverage Status](https://coveralls.io/builds/5812764/badge)](https://coveralls.io/builds/5812764)\n\nCoverage decreased (-12.3%) to 79.479% when pulling **1e1c2431fc4552bf1f430f67acd39c92c3cd4647 on rxjs** into **5cc8c1c7ff83dc81b0abb1b35043531cfaa70bb6 on master**.\n"}}