MQTT.js vNext: Initial Spike

[YoDaMa]

Hi MQTT.js community,

Since joining this project and working on-and-off on maintenance tasks and github issues, I've slowly built up an understanding of the key strengths and weaknesses I've felt in the architecture of MQTT.js. I've released a major update along with various minor and patch version updates to fix security and usability bugs in the existing codebase. I've spent a significant amount of time exploring and redesigning the highly-flakey gate, migrating away from Travis to Github Actions, improving test isolation, and increasing overall reliability of the test code.

After contributing in this way I feel we've reached a point as a developer community where we should embark on a significant re-architecture of the package. This is the purpose of this PR.

What you'll see right now is an initial spike of a new code architecture for MQTT.js that I believe will improve readability and code maintainability. One of my goals here is to modernize the codebase toward ES6, and leverage rollup for cross platform (particularly with Node.js) usability. As these platforms modernize their engines to match ES6, rollup will be able to fill in the gaps.

Another goal in this spike is a more "handler" oriented design, similar to moscajs's aedes package.

There are many blindspots in my design and understanding, which is why I'd like community input, particularly from other longtime maintainers, on this.

I'd also like to understand the main benefits people might see in Paho MQTT, and how those benefits could be incorporated into this package.

So in conclusion, a re-design is long overdue, and necessary to keep this package up-to-date and functioning as a cornerstone MQTT Client package in JavaScript. I can lead this effort of course, but the more community input and knowledge-share, the better.

[mcollina]

I really like this design. I also like you are aiming for a full rewrite and that you are targeting esm: +1.

(I won't be opposed for this to be in TS either - you'd be doing the vast majority of this anyway).

I recommend to throw away all the tests.

[YoDaMa]

Here's my rough visualization of the data flow of a "connect" call in the current MQTT.js architecture.

My thought is that work and workNextTick seem like unnecessary complications considering the event driven nature of the streams. Simplifying the data flow would look approximately like this:

[YoDaMa]

Also here is a rough diagram of the method flow when someone would call a "connect" in the current SDK.

It's very convoluted, particularly because setupStream is such a misnaming of the functionality of that piece of code. Additionally there's a really confusing back and forth from the index.js file in connect folder, where the connect method will create a wrapper function around a transport layer implementation that will then be passed as a function to the client for instantiation where the client calls the wrapper function in setupStream while also instantiating mqtt-packet's parser, actually building the transport stream (aka calling wrapper which is passed in as streamBuilder when instantiating the client), performing the necessary wiring to connect the different streams together so that the writable stream inside the client can process data by storing it and then calling a work function (which is all part of the setup but not really relevant in terms of data flow right now), and then it also finally sends a connect packet, while also creating a timeout for a connack.

In my view.... there's got to be a much better way :)

[mcollina]

Sounds good!