feat: drop readable-stream and buffer in favour of `node_stream/web` and `UInt8Array`

[robertsLando]

This would be the first part of the work needed to make MQTTjs full browser compatible without polifilly and also improve compatibility of libraries with bun/deno. I would like to know your opinion about this @mcollina

Useful resources:

• https://exploringjs.com/nodejs-shell-scripting/ch_web-streams.html
• https://developer.mozilla.org/en-US/docs/Web/API/Streams_API
• https://streams.spec.whatwg.org/

[mcollina]

It’s not a good idea to remove node stream support. For Node.js client and server side usage, we need them to have decent performance.

Moreover we need the bl module to avoid massive reallocations when adding new things to the packet. The current algorithm does not allocate the full packet size, but keep the individual parts separate and then write them directly to the stream.

In other words, to have decent performance on Node.js, we have to keep those.

I have no problems in adding a more whatwg friendly implementation here so that browser MQTT.js can use the proper standards.

[robertsLando]

I have no problems in adding a more whatwg friendly implementation here so that browser MQTT.js can use the proper standards.

Could you explain me more in details what you mean with this? Does using WebStreams instead of readable-stream in browsers have any benefits?

[mcollina]

Does using WebStreams instead of readable-stream in browsers have any benefits?

Bundle size would be significantly lower.

[robertsLando]

Bundle size would be significantly lower.

Ok so why not doing this on readable-stream package instead?

[mcollina]

Ok so why not doing this on readable-stream package instead?

I don't understand the question. WebStreams and Node.js streams have incompatible APIs and semantics.

[robertsLando]

WebStreams and Node.js streams have incompatible APIs and semantics

My idea was to create a browser export of readable-stream that relies on WebStreams, I never tried WebStreams so I dunno how compatible they are with Node.js streams, sorry for the noob question.

That said, I think that adding a more whatwg friendly implementation here could be a good starting point, thanks :)

About the switch from Buffer to UInt8Array are you +1 or not? Ref: https://sindresorhus.com/blog/goodbye-nodejs-buffer

[mcollina]

About the switch from Buffer to UInt8Array are you +1 or not

We can't, due to how bl works.

[robertsLando]

@mcollina Yeah I know but I'm open to create a PR to bl as well if you think there could be some benefits.

[mcollina]

I don't really understand what the benefit of that would be, the goal of bl is to implement the API of Buffer over a list of buffers.

[robertsLando]

The benefits of switching to UInt8Array are described here: https://sindresorhus.com/blog/goodbye-nodejs-buffer.

But if you think that's not worth it I understand

[mcollina]

What I mean is that it's 100% out of scope for bl, because that exposes the Buffer API and aims to be compatible with Buffer. Therefore you'd need an equivalent module for UInt8Array.

There are also

mqtt-packet/parser.js

Line 626 in d54a270

const result = this._list.slice(this._pos, end)

, which will impact performance if converted to a UInt8Array.

[robertsLando]

Ok, I understand. Sorry for the misunderstanding, I had these things in mind for long and wanted your opinion on this given your deep understanding of streams on Node.js side.

So if it's ok for you I would like more details about:

adding a more whatwg friendly implementation here so that browser MQTT.js can use the proper standards.

[mcollina]

I think adding a way to generate mqtt-packets on top of WHATWG standards things would be good.

We can't get rid of the ones for Node.js because WHATWG have such a huge performance penalty there that would make a lot of the usecases of this module useless. However having a goal to run a MQTT client on to of a WebSocketStream in the browser is laudable.

So, we might need to create another module/add functionality to this module one so that by requiring mqtt-packet/brower.js, they get some code that:

• It does not use bl, but rather resizable ArrayBuffer or another equivalent solution.
• it does not use Node.js streams, but provides some interface to easily write to a WHATWG WritableStream.

[robertsLando]

@mcollina I agree, I think it would be better having it implemented here instead of having a complete new module