mqtt-packet
mqtt-packet copied to clipboard
feat: convert project to typescript
@mcollina would you accept such PR?
Why? What's the end goal? This project is stable and essentially "done".
From my experience re-writing projects using TS helps discover issues in the code that JS hides, also it's easier to apply any future update to the library giving that everything is type-safe then.
Also having to deal with https://github.com/mqttjs/mqtt-packet/issues/151#issuecomment-2677854833 would make things easier to handle
Unless some significant feature is coming (like a new version of MQTT), it's not worth doing it.
Usually, it's harder to get typescript code to be as fast as JS, and a rewrite would require extensive benchmarks.
I'm not necessarily opposed, but given the amount of work needed on on MQTT.js, I don't think it's a good use of effort.
It might make sense doing it if we end up adding some support for the WHATWG specs.
Unless some significant feature is coming (like a new version of MQTT), it's not worth doing it.
Usually, it's harder to get typescript code to be as fast as JS, and a rewrite would require extensive benchmarks.
I'm not necessarily opposed, but given the amount of work needed on on MQTT.js, I don't think it's a good use of effort.
It might make sense doing it if we end up adding some support for the WHATWG specs.
@mcollina I stumbled upon this issue because I am involved in eventually making some changes to the MQTT.js library which led me to this repository following a discussion with @robertsLando .
Could you elaborate on this sentence, please?
Usually, it's harder to get typescript code to be as fast as JS, and a rewrite would require extensive benchmarks.
This considerably challenges the information I have regarding the V8 engine and it's internal optimisations, especially regarding TurboFan and Ignition. This has been explained in a concise way by Franziska Hinkelmann quite a few years ago at JSConf. Since Javascript does not use any explicit typing, ensuring type consistency and stability as function parameters is significantly harder than with a properly configured TypeScript repository. I would agree with you only in the context of an overly lenient TypeScript configuration compiling with built-in polyfills.
Generally there are two things that make Javascript perform poorly (with half-decent code):
- Threadpool saturation through excessive I/O and other tasks requiring getting out of the LibUV loop.
- Ever changing typing which creates a situation where TurboFan is never called and everything goes through Ignition.
I am very interested in getting more details about this since performance is always nice and welcome.
Usually, it's harder to get typescript code to be as fast as JS, and a rewrite would require extensive benchmarks.
Depending on the TS configuration, down leveling can be involved, and the resulting JS will be significantly slower because a lot more code is being executed. Just one line change in tsconfig.json could cause this.
In addition, manually written JS usually has fewer checks (e.g., ? or if (.. !== 'something')), which results in fewer operations. Most of those checks are essentially redundant for conditions that would not happen during operations.
Usually, it's harder to get typescript code to be as fast as JS, and a rewrite would require extensive benchmarks.
Depending on the TS configuration, down leveling can be involved, and the resulting JS will be significantly slower because a lot more code is being executed. Just one line change in tsconfig.json could cause this.
In addition, manually written JS usually has fewer checks (e.g.,
?orif (.. !== 'something')), which results in fewer operations. Most of those checks are essentially redundant for conditions that would not happen during operations.
I agree when using a too lenient TypeScript configuration, which I would even question the need for TypeScript is it is lenient. Regarding checks, type observations are almost always optimised at assembly level by TurboFan (on Chromium and default Node.js, see Spidermonkey for Firefox), it makes next to no difference. Typeguards from TypeScript also have the benefit of typechecking at runtime, guaranteeing strict typing at runtime for everything after their call.
It's a balance to be had. Generally speaking, very strict typing yields far greater performance than lenient typing. It lowers pressure on the compiler and makes code more predictable. It also makes the code far easier to maintain when every type can be ensured. You will eventually get some losses here and there, especially during input types inspection, but the benefits outweighs the drawbacks on almost every front.
but the benefits outweighs the drawbacks on almost every front.
I have to agree with this based on my personal experience, also last but not least I see more users engagement on repos that have types as most users feel more safe making changes when there are types checks.
Anyway, I'm not totally opposed. I would recomemnd migrating to ESM at the same time and adopting node:test and Node 22+, and keep a purely type-stripping setup. You can see the approach working in https://github.com/platformatic/kafka.
Anyway, I'm not totally opposed. I would recomemnd migrating to ESM at the same time and adopting node:test and Node 22+, and keep a purely type-stripping setup. You can see the approach working in https://github.com/platformatic/kafka.
I've had a quick look at the configuration of the repository you pointed and it's pretty much what I had in mind. I don't want to use TypeScript automatically polyfilling as they output pretty bad code. TypeScript can be configured with a very strict typing approach and preserve native ES2022 syntax without alteration (as long as decorators are not being used of course as those compile to rather... exotic code).
Regarding ESM I'm glad to read that, I believe migrating to ESM would make things easier to maintain and understand, with the several benefits of ESM against CJS. Finally, just let me know which minimum version of Node you want to support. My personal approach for my libraries is to support the oldest still maintained LTS version of Node but no further, which means Node.js 20+ at this time.
Migrating to node:test: If we are speaking only about the test runner, then sure it makes total sense. The native assertion library is still a bit crude and unwieldy to use. We use it on our main OSS library so I am familiar with it, I just expect many people to complain about the lack of functionality.
Let's do 22+ to leverage native TypeScript in Node tests for development. This future-proof setup avoids most of the pain when dealing with TypeScript, building, and running tests. I'm mostly concerned in reducing dependencies and maintainance burden.
This will be released as a new major, and 20 goes out of LTS in April 2026, which seems far in the future but is not really considered, #as this new major would be used in a matching new major of MQTT.js and other packages.
Let's do 22+ to leverage native TypeScript in Node tests for development. This future-proof setup avoids most of the pain when dealing with TypeScript, building, and running tests. I'm mostly concerned in reducing dependencies and maintainance burden.
This will be released as a new major, and 20 goes out of LTS in April 2026, which seems far in the future but is not really considered, #as this new major would be used in a matching new major of MQTT.js and other packages.
Fair enough, I'll let @robertsLando drive the road map for this evolution. Let me know how and when you want to proceed.
@SmashingQuasar What kind of roadmap would you like to have? I think we can focus on converting everything to ts first and then do the benchmarks, if you want we can start by integrating benchmarks with a workflow action that we can trigger manually or automatically when a PR gets a commit
Would you prefer if I first converted the project to a super lenient TypeScript and then added each rule through a separate PR or do you prefer an all-in-one PR?
Yeah that is what I have in mind but I would like to merge everything only when ready so my idea is to create a ts branch and have the pr merging there then once we have a ready state merge that branch on master