Make client Async-Iterable

[RangerMauve]

Since Async Iteration is on the cusp of being available everywhere, the following dream code could become a reality:

async function (client) {
  await client.subscribe("some/+/topic");

  for await (let [topic, payload] of client) {
    await saveToDB(topic, payload);
  }
}

I propose we detect whether the environment has a Symbol.asyncIterator defined, and it it does, define a method for making the client async-iterable.

[mcollina]

Maybe we should do mqtt@3 and integrate promises there too.
I'm 👍 in having them here anyway.

[RangerMauve]

Maybe we should do mqtt@3 and integrate promises there too.

Would it mean a total overhaul of mqtt to use promises internally, or just make the methods that take callbacks optionally return promises?

[Tabrizian]

I think integrating promises with the main mqtt library would be very great. Mongoose does exactly what @RangerMauve said. Making methods that take callbacks and optionally return promises.

[mcollina]

Yes, that'd be my idea.

[RangerMauve]

Should I hold off on adding the Async-Iteration until that's done, or would it be good to add it here first so that we can play around with the implementation for when we add it to the main mqtt lib?

[TimSusa]

Hey,

I would like to mention a few things. At first, why would you say a dream would come true, to have asyncIterators, while still being able to do stuff without them? The proposal is still not finished, by the way. I can only say, that I was at the same position, but then all of sudden, we had a huge memory-leak problem in production. on a real big project. Investing days with digging in memory-heap-dumps... seeing parts of async iterator in the dumps. Issues on Github occured. A lot of hazzle. Things can quickly get complex and hopefully anyone is there, who understands that iterator code then.
Another concern is the current implementation of async-mqtt, which is really not shining so bright IMO. It is looking more like being finished in a hurry. Sry. Please consider, people should use this lib in Production.

1. Please avoid creating Promises with new.
2. Consider using util.promisify as you have there nodejs callback style already, why not wrap every method? https://hackernoon.com/node8s-util-promisify-is-so-freakin-awesome-1d90c184bf44
3. Please , get rid of just tunneling ...args parameter through all class methods. This makes the code unreadable.
4. Consider using typescript with all consequences, not just only providing types.
5. Consider using documentation in the code
6. Most of the class methods are not async at all, just returning a function still with callback
7. Consider using simple async, await, Promise.all(), Promise.resolve()

Sry, for the strong words. I only wanted to give constructive feedback.

Cheers!

[mindrunner]

for await (const [topic, message, _] of EventEmitter.on(mqttClient, 'message')) {
  console.log(`${topic} - ${message}`)
}

Seems to work perfectly fine for me.