Input requested: Future of MQTT.js

[BertKleewein]

I'm seeking input on an idea for moving this repo forward.

My team at Microsoft uses MQTT to communicate with the Azure IoTHub service. __MQTT.js does an excellent job here, but, frankly, it's starting to show its age. It was written when the node.js ecosystem was less mature, and it has evolved as the ecosystem changed. Many (but not all) of these "evolutionary steps" were bolted-on pieces of technology that "added a little more to MQTT.js". The result is code that does what it needs to do, but it’s hard to understand and even harder to change.

@YoDaMa and @vishnureddy17 looked at this problem and started to build an updated version of this library from the ground up. This is a noble pursuit. It’s also extremely easy to underestimate how difficult it is. While MQTT, the transport, is relatively simple, libraries with thousands of dependents, like MQTT.js, are not so simple. The thousands of dependents, or customers, or users, or whatever you want to call yourselves are what make MQTT.js powerful. "Rebuilding from the ground up" means "moving forward by moving away from these customers." I personally do not like this idea.

Instead, we should take the time and take a large evolutionary step to bring the existing MQTT.js code "into the modern age." We should call this MQTT.js version 5.0.0, with the major version change from 4 to 5 showing that this is a breaking change. But we should only break things when there is a clear advantage. Changes in API or behavior from version 4.7.3 to version 5 should be justified and documented. Customers who want to "play it safe" can use semantic versioning rules in package.json to make sure they do not get version 5. Other customers who want to "follow along" shouldn’t need to make major changes, but we cannot guarantee this.

We should convert this code to typescript. I have done this as a POC (proof-of-concept). The extra type information has shown me so many places where the code is sloppy and could be tightened up, especially around options and client properties and callback signatures. You can see my POC here

We should also start using prettier and eslint on the code. I hate arguing about "what format is correct" for pretty-printers. I love being able to write code without worrying about formatting because I know it will get automatically formatted. Unfortunately, this means that every line of code gets changed, and merges from "pre-prettier" code into "post-prettier" code gets ugly. I wish I knew a way to avoid this.

We should start publishing commonjs and ECMAScript side-by-side if we can. It's too bad that there is still a distinction here, but I think we can make both sides happy without picking a side.

We need to make sure the browserified packages still work, but I assume they will change. I'm less clear on details here, but I assume that webpack and ECMAScript is the way that people want to go. I could be wrong, and we have options here.

If we can do these things, then the code will be ready for some interesting changes. I have a ton of ideas and opinions here, but I do not want to cloud the discussion just yet. :)

I'm curious to hear what people think. Some of the things I have said here are strong beliefs. Others just seem like the right path.

Tagging an arbitrary list of active/recent contributors to get your attention. Apologies for the spam.
@mcollina, @vishnureddy17, @YoDaMa, @redboltz, @scarry1992, @radekg, @ysfscream, @sublimator, @RangerMauve, @4rzael, @anhldbk, @bkp7, @nosovk, @robertsLando, @ogis-yamazaki, @ryanwinter, @anthonyvercolano

[robertsLando]

Agree that this repo needs a fresh new look, happy to help where I can!

[nosovk]

We moved to https://github.com/shanewholloway/js-u8-mqtt for frontend apps. It works in Vite/Svelte without problems. Also it's tiny. It lacks of some features, like ACK control, but it's very specific usecase.

[radekg]

@BertKleewein have you seen this? https://github.com/mqttjs/MQTT.js#next-major-version-of-mqttjs

[BertKleewein]

@BertKleewein have you seen this? https://github.com/mqttjs/MQTT.js#next-major-version-of-mqttjs

@radekg, Yes, thank you. I actually work directly with the authors of that repo. This is a different path, but the goal is the same. We're putting that project on pause while we see if we can accomplish our goals in a less ambitious way.

[radekg]

@BertKleewein I’d say go for it.

[BertKleewein]

We moved to https://github.com/shanewholloway/js-u8-mqtt for frontend apps.
@nosovk - thanks for the link. That looks interesting.

[nosovk]

We moved to https://github.com/shanewholloway/js-u8-mqtt for frontend apps.
@nosovk - thanks for the link. That looks interesting.

Actually I should tag @shanewholloway, probably he can suggest how to migrate to modern approach (or better rewrite all from scratch).

[mcollina]

Converting this library to TypeScript, publishig it as ESM, etc does not solve any maintenance issues this module has (which has led to me stopping maintaining it). Fundamentally, MQTT.js has to be rewritten to fix all the problems this library has.

Anyway, I stopped maintaining this library long ago, so I'd leave any decisions to @YoDaMa.
This makes an issue completely within Microsoft and Azure, I guess there is budget to maintain this library... which makes things significantly easier.

[robertsLando]

Converting this library to TypeScript, publishig it as ESM, etc does not solve any maintenance issues this module has

@mcollina Could you be more specific about this? Would be interesting to know which, in your opinion, are the main issues with this library

[mcollina]

It's a bit hard to list here, I fully briefed @YoDaMa at the time. There are 300+ open issues, too :/.

[shanewholloway]

We moved to https://github.com/shanewholloway/js-u8-mqtt for frontend apps.
@nosovk - thanks for the link. That looks interesting.

Actually I should tag @shanewholloway, probably he can suggest how to migrate to modern approach (or better rewrite all from scratch).

Thanks for the compliment @nosovk -- glad to hear you're using it. Feature/pull requests requests are welcome regarding the "ACK control feature" mentioned above.

I created u8-mqtt-packet to be a ESM-based browser-first library that only parsed MQTT packets. The idea being that one could make one-off single purpose MQTT clients simply; e.g. a web-hook to MQTT update relay, or a service-worker MQTT intermediary. Developed directly from the MQTT 3.1 and MQTT 5 specs, I developed and ran my test suites against the Mosquito and eJabberD MQTT servers in both 3.1 and 5 protocol modes. The packet library was designed to be lightweight for the browser, as well as work in Deno and NodeJS ecosystems. My u8-mqtt client library puts the necessary pieces together for the use cases I needed -- and to get that iterative development outside of the more stable packet library.

All that to say, I'd be happy if an MQTT.js client compatible fork chose to leverage these ESM packet parsing libraries. I'm not a TypeScript fan, but I'd be happy to publish static types alongside the package if someone creates a pull request for it.

[BertKleewein]

@mcollina, @robertsLando

Converting this library to TypeScript, publishig it as ESM, etc does not solve any maintenance issues this module has

Agree that Typescript and ESM by themselves don’t solve any problems. Typescript helps to highlight sloppy programming and helps to enforce less sloppy programming in the future. ESM is a byproduct that makes the library more attractive to some customers.

I’ve started to make a list of the things that are “wrong” with this code. In short, this code is a mess. It’s hard to work on. There’s no way to know that you didn’t break something. It’s easy to break things.

In the end, I think (or hope) that we’ll end up with a piece of code that looks like a total rewrite. I’m viewing this like a “tear it down to the studs” renovation.

Here’s a few notes on things I’ve noticed. This is not complete and some of them probably don’t make sense to anyone but me.

(I mean no disrespect to anyone who has worked on this code. I try very hard to practice egoless coding -- "you are not your code")

testability

1. The tests we have rely on a fake MQTT server. Understanding existing tests means keeping both the client and server in your head and being able to debug both at the same time.
2. There is no way to test against live servers (aedis, mosquito, etc).
3. In-browser testing is extremely limited and clearly hasn’t been run recently.
4. Some code is just not tested at all.

client.js

1. Client class is just too big and poorly organized. It’s a big mass of spaghetti that needs to be detangled.
2. Some code clearly tacked on, like topic alias support which was added as static helper functions outside the client class.
3. Options are not well defined. Some are stored in options struct, others in client, with no definition or consistency.
4. Many individual state variables (such as MqttClient.disconnecting and MqttClient._pingResp) not documented and not consistently implemented.
5. MqttClient.end() is a mess.
6. EventEmitter handlers used for internal flow, such as this.on(‘connect’ and this.on(‘close’, make code flow extremely difficult to reason about. This is part of why MqttClient.end is so messy.
7. Outgoing queue and store queue, which are two different things, have poorly defined structure. It’s hard to know what is stored in each queue outside of a debugger.
8. In short, you cannot read client.js and have any idea or how it will behave at runtime. The only way to know how it’s going to work is to run it. There is no way to confidently make changes to the code.

[mcollina]

I agree on all accounts.

[robertsLando]

@BertKleewein What is the implementation status of https://github.com/mqttjs/mqttjs-v5?

We're putting that project on pause while we see if we can accomplish our goals in a less ambitious way.

What you mean with less ambitious way?

[BertKleewein]

@robertsLando -

The mqttjs-v5 repo is on hold for the moment.

What you mean with less ambitious way?

The previous (more ambitious) goal was to write mqttjs-v5 from the ground up, as a fully modernized promise-first library based on the MQTT spec. Features, interfaces, behaviors, and tests would all be written from scratch.

The current goal is to modernize the current implementation, but keep as many of the current features, interfaces, behaviors, and tests as possible.