At the moment there is no plan for it.
I'm happy to discuss it, because of async/await in node 8.
I suppose the simplest API would be to return promises in cases where a callback was omitted, as is done in most other libraries.
Not all methods can be easily promisified by just leaving the callback param out. Take connect for example, a promisified version would look something like:
var mqtt = require('mqtt')
function connect(host) {
return new Promise((resolve, reject) => {
let client = mqtt.connect('mqtt://test.mosquitto.org')
client.on('connect', resolve)
client.on('error', reject)
}
}
@yan-foto That's more or less exactly what I have in some code that I have in the wild. However, rather than using ".on()", I use ".once()", and I remove the other listener once it's ready.
Though I don't think that a promise is necessarily the right return from "connect". "connect" and "error" can be emitted multiple times, and you might want to cancel the connection. Using the existing event handlers on the connection seems like a better approach for most use cases. Promises would be useful for replacing non-event callbacks, like the result of a "publish".
Though I don't think that a promise is necessarily the right return from "connect". "connect" and "error" can be emitted multiple times, and you might want to cancel the connection. Using the existing event handlers on the connection seems like a better approach for most use cases. Promises would be useful for replacing non-event callbacks, like the result of a "publish".
@RangerMauve I'm more on the "lightweight" promise support as you described when node 8 comes by, as you describe. Having support for it in connect is good, but I would have a mqtt. connectAsPromise() method that returns a Promise.
Ideally we could also have AsyncIterator https://github.com/tc39/proposal-async-iteration support for receiving the messages, if that lands in Ecma.
AsyncIterator support would be amazing. I've been really into it ever since I found it a few months ago. Would the client itself be async iterable and it would yield message events?
Yes. That'd be ideal. Let's wait until it's part of ecma. We'll be probably adding it to streams as well.
If you want, you can prototype it as a separate module in the meanwhile.
Though I don't think that a promise is necessarily the right return from "connect". "connect" and "error" can be emitted multiple times, and you might want to cancel the connection.
@RangerMauve That is true when you are talking about events. However, I would love to have the connect method, which causes the connect event to be a promise.
@yan-foto That's reasonable. I think something like connectAsPromise, as @mcollina proposed, would be a good approach. That way it'll be totally backwards compatible while serving peopel that need the use case. Not sure about the name, though. :P
Fair enough! To sum up:
Alternately, would it make sense to maybe have an AsyncMqttClient which is returned by connectAsync which will have all the methods named the same, but have them be async?
I could get behind that! Let's hope there is enough manpower to have it implemented 馃檹
I think we should have the least breakage, if any. So, new names etc. Alternatively, we can do another package with this org.
@yan-foto The connect method and the connect event work together in a way that doesn't make them compatible with promises.
The connect method tells the client to connect and reconnect as necessary. The connect event gets emitted every time the client connects/reconnects to the broker. That event is needed by "clean" clients to resubscribe to topics they want messages for.
MQTT.js is implemented to queue messages while offline (including before the first connect event is emitted) so it's not normally necessary to wait for a connection to be made before using the client.
I use promises in my apps with MQTT.js, but have never needed to wrap connect with a promise. As @RangerMauve said, I have wrapped publish, though, since that's a method that actually accepts a callback argument (unlike connect) and is, therefore, easily convertible to a method that returns a promise.
@mcollina a new project that has this one as a dependency would make sense.
@jdiamond I tend to agree here. My use case was so that I could abort my connection process if the authentication failed when initializing. I would wager that that isn't the only way to do it, but promises are like a hammer, in that everything looks like a nail.
Perhaps @yan-foto 's use case could be effected by a new method on a client which takes a callback for checking if the current connection succeeds or fails, then the promise version of the mqtt library will yield a promise instead of taking a callback.
On a side note, if we don't supply a callback for a method call now, the error gets sent to the "error" event, right? This would get in the way of reusing existing methof names, since you can't tell if a promise got any error subscribers or not. Personally, I don't like libraries having different method names for promise yielding methods, so I'm kind of leaning towards having a separate type of client, be it another library or added to this one.
@jdiamond I don't see how it's not compatible. I'm not suggesting to kill connect event! The client, as I understand, takes care of reconnection and I don't really need to take care of each connect event thereafter. However, the initial connect (initiated by me) emits the first connect event, which is all I know to get started (e.g. publishing). @RangerMauve described my problem pretty well, it was also well noted that once would be better in my example code.
@RangerMauve I've invited you the the organization :). Feel free to create/coordinate a new repository to provide a promisified client if you want to.
@yan-foto I thought the same thing when I first started using MQTT.js, but then I learnd that you do not need to wait for the connection to be made before using the client to publish messages. Just call connect and start publishing! It's even listed as a feature in the README.
The MQTT protocol itself specifically allows clients to send a connect packet and then immediately start sending publish packets without waiting for a connack packet from the server. MQTT.js goes the extra mile and allows you to publish before even the connection is even opened.
If you really want to wait, you can client.once('connect', onFirstConnection) as @RangerMauve noted. Would a new method really add much value over that? IMO, the first connection is not special enough to justify this because of all the things that could go wrong during publishing that require you to wait for your second, third, etc, connections.
I'm a huge fan of promises and use them in all my apps, but I've painfully learned that it's better to let the MQTT.js client (with a persistent outgoingStore if I can't tolerate losing messages) manage all the complexity around reconnecting and republishing so that I don't have to deal with it in my apps.
Not to discourage @RangerMauve from working on a promisified wrapper. I don't even know what an AsyncIterator is, but it sounds cool so I'm interested to see what it can do. 馃槂
@mcollina Thanks! I've been really busy with work/life balance stuff lately, but I'll see if I can whip something over a few hours this week. I'm thinking I'll copy over the unit tests and docs over from this repo and use it as a dependency.
@jdiamond Yeah, I totally agree with your points. I'm thinking that I'll limit the initial version to just methods that take callbacks directly like publish and leave the events and connect as is.
Can't create repos in the org, but here's an initial sketch of what I was thinking.
To summarize: it's a pass through for all the existing MQTT client methods, with the exception that publish, subscribe, unsubscribe, and end now return promises by supplying the callback argument automatically.
I haven't tested if it works yet, but I doubt the overall design is going to change much unless somebody has suggestions. I'll implement the AsyncIterator stuff after I've got the other stuff to work.
@RangerMauve you should be able to create a repo now.
I like the approach!
馃憤 for Promises :)
Sorry for the lack of progress! Haven't been doing as much open source the past while.:'(
I've created asyn-mqtt and will hopefully copy over the tests later and see if it works.
If so, I'll publish on NPM.
Anybody else want access to the NPM package for publishing?
@RangerMauve would you mind adding unit tests (just some to check everything is in place) and travis-ci?
You can add me, I'll be very happy to help you maintain it.
Also, we should add a link in MQTT.js that if anyone needs a promise-based API, we maintain that as well!
Hey! Sorry for the radio silence. I'm just finishing up the unit tests now and I'm going to publish afterwards. Do you have any suggestions for tests to check that errors are properly being propagated?
Published 1.0.0 and added @mcollina as owner. https://www.npmjs.com/package/async-mqtt
@RangerMauve this is FANTASTIC!!!!
@RangerMauve would you mind to send a PR to the README here?
@mcollina Yup!
Maybe it's a good time to reference this module also from mqtt package in the README 馃
@yan-foto We do! It's right here
@RangerMauve my bad! I must have missed it. sorry! :sweat_smile:
Most helpful comment
Can't create repos in the org, but here's an initial sketch of what I was thinking.
To summarize: it's a pass through for all the existing MQTT client methods, with the exception that
publish,subscribe,unsubscribe, andendnow return promises by supplying the callback argument automatically.I haven't tested if it works yet, but I doubt the overall design is going to change much unless somebody has suggestions. I'll implement the AsyncIterator stuff after I've got the other stuff to work.