Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

streamable browser variant for browserify #77

Closed
wants to merge 2 commits into from

5 participants

@substack

This pull request depends on sockjs/sockjs-client#76 in sockjs-client to work.

This patch adds a streamable wrapper around sockjs-client so that people can do require('sockjs') in browserify or a similar node-style require() bundler and use .pipe() and friends on the browser-side like the present node-side api.

@substack

With this API people will be able to use the new pipe-able dnode api like this:

var domready = require('domready');
var sockjs = require('sockjs');
var dnode = require('dnode');

domready(function () {
    var result = document.getElementById('result');
    var stream = sockjs('/dnode');

    var d = dnode();
    d.on('remote', function (remote) {
        remote.transform('beep', function (s) {
            result.textContent = 'beep => ' + s;
        });
    });
    d.pipe(stream).pipe(d);
});
@dominictarr

+1
this is a really good idea because it makes the same interface on the client and the server.
which makes writing sockjs based libs much more convenient!

@igorw

This should be a layer on top of the WebSocket API. That way you can use it with raw WebSockets or with SockJS. If you added to SockJS, it will only work with SockJS, which is exactly not what SockJS is trying to accomplish.

@substack

@ignorw Very true, and that's something that should be taken into consideration. However, I think this patch is justified because:

  • It doesn't change anything about sockjs except that it provides another code path for browserify to latch on to. Presently this module won't build when required by browserify and just adding a browser.js seems sufficiently out of the way.

  • This is the sockjs-node project, and the server-side interface already provides a readable/writable stream for interacting with the browser. The websocket API is so similar to the node stream api that there's a lot of benefit in consolidating that correspondence so programmers only need to keep one api in their head, reducing api surface area.

If this patch is rejected and assuming the sockjs-client patch goes through I will gladly create a sockjs-stream module, but in this case I think being able to use the same require('sockjs') call in both the browser and the server is a big win.

@igorw

Full disclosure, this issue is sort of related to #67. ;-)

@majek
Owner

I'm very confused. Is this patch adding a code to a server-side node.js project, that when run from the browser will use sockjs-client? Why?

@majek
Owner

Hmm... I'm quite sure doing any sockjs-client things should not be a part of sockjs-node server.

@substack Do you think the same functionality can be achieved by launching a separate project (you suggested sockjs-stream)?

I think I'm happy to merge sockjs/sockjs-client#76 BTW.

@majek
Owner

Or is it:

  • we'd like to be able to do require('sockjs') in the browser
  • but as sockjs is already taken in npm, by this project
  • let's extend this project to provide a special call for us
  • so that when we require sockjs, we'd have a pass-through API to a completely different project.

That sounds wrong. Why not make this thing part of sockjs-client or something on top of sockjs-client? Am I right that sockjs-node is completely unrelated to what we want to achieve?

@substack

browserify's primary goal is to let users use modules from npm in the browser so that programmers can use the same apis in the server and the client. Since sockjs-node already offers a stream in node and browserify has compatability layers for making streams work, this patch was intended to increase the correspondence when using browserify. The patches to sockjs-client are separate and merely let you require('sockjs-client') with browserify or other commonjs bundlers. sockjs-client is targeted at server platforms besides node, so I thought the streaming correspondence was most appropriate for sockjs-node to have.

@substack

At any rate, I just made this project instead: http://github.com/substack/shoe
That other patch on sockjs-client would be very handy still however.

@brycekahle brycekahle closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jun 21, 2012
  1. @substack
Commits on Jun 22, 2012
  1. @substack
This page is out of date. Refresh to see the latest.
Showing with 52 additions and 2 deletions.
  1. +48 −0 browser.js
  2. +4 −2 package.json
View
48 browser.js
@@ -0,0 +1,48 @@
+var Stream = require('stream');
+var sockjs = require('sockjs-client');
+
+module.exports = function (uri, cb) {
+ if (/^\/\/[^\/]+\//.test(uri)) {
+ uri = window.location.protocol + uri;
+ }
+ else if (!/^https?:\/\//.test(uri)) {
+ uri = window.location.protocol + '//'
+ + window.location.host
+ + (/^\//.test(uri) ? uri : '/' + uri)
+ ;
+ }
+
+ var stream = new Stream;
+ stream.readable = true;
+ stream.writable = true;
+
+ var ready = false;
+ var buffer = [];
+
+ var sock = sockjs(uri);
+ stream.sock = sock;
+
+ stream.write = function (msg) {
+ if (!ready) buffer.push(msg)
+ else sock.send(msg)
+ };
+
+ sock.onopen = function () {
+ if (typeof cb === 'function') cb();
+ ready = true;
+ buffer.forEach(function (msg) {
+ sock.send(msg);
+ });
+ buffer = [];
+ };
+ sock.onmessage = function (e) {
+ stream.emit('data', e.data);
+ };
+ sock.onclose = function () {
+ stream.emit('end');
+ stream.writable = false;
+ stream.readable = false;
+ };
+
+ return stream;
+};
View
6 package.json
@@ -10,7 +10,8 @@
"url": "https://github.com/sockjs/sockjs-node.git"},
"dependencies": {
"node-uuid" : "1.3.3",
- "faye-websocket" : "0.4.0"
+ "faye-websocket" : "0.4.0",
+ "sockjs-client" : "~0.0.0"
},
"optionalDependencies": {
"rbytes" : "0.0.2"
@@ -18,5 +19,6 @@
"devDependencies": {
"coffee-script" : "1.2.x"
},
- "main": "index"
+ "main": "index",
+ "browserify": "browser.js"
}
Something went wrong with that request. Please try again.