Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Connection as a Writable stream #124

Merged
merged 1 commit into from
Aug 14, 2013
Merged

Connection as a Writable stream #124

merged 1 commit into from
Aug 14, 2013

Conversation

mcollina
Copy link
Member

I moved the parsing logic directly inside the Connection object, which became a Writable.
It is not needed to be a Duplex, as it will do nothing in writing.

The interface remains the same, but it allows cool stuff like:

// stream is from somewhere, it might be a named pipe, a websocket, or whatever other transport

var mqtt = require("mqtt");
var conn = stream.pipe(new mqtt.Connection());

conn.publish("hello", "world");

Fixes #121

@adamvr and @timoxley please provide feedbacks :).

I'd like this to go into #118, as it simplifies the reconnecting logic.

I'd like to also expose this through the "main" mqtt object, so we can do "crazy" ideas like MQTT over Websocket (and any other kind of Stream).

@mcollina
Copy link
Member Author

I'm also in favor of deprecating createConnection, createServer and createSecureServer.
All of these are just convenience methods around a Connection object, and to me they seem just to make things more complex.

@timoxley
Copy link
Contributor

A big +1 from me for all of the above.

@timoxley
Copy link
Contributor

Would #121 come back perhaps in a later version?

@adamvr
Copy link
Member

adamvr commented Aug 13, 2013

👍 to connections as streams, duplex, writable or otherwise. Much more portable. As to the createX methods, what is your alternative @mcollina? Would we do something like:

var conn = new mqtt.Connection()
    , sock = net.connect(1883, 'localhost');

sock.pipe(conn);
conn.on('connect', ...)

That seems to me to be more complex, unless I'm missing something here.

@mcollina
Copy link
Member Author

Ok, let's leave this reasonings to a next version and merge this.

mcollina added a commit that referenced this pull request Aug 14, 2013
Connection as a Writable stream
@mcollina mcollina merged commit 32e8795 into v0.3.0-dev Aug 14, 2013
@mcollina mcollina deleted the fix-121 branch August 14, 2013 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants