chore!: convert project to typescript #1641
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #1641 +/- ##
==========================================
- Coverage 85.74% 81.43% -4.32%
==========================================
Files 13 21 +8
Lines 1249 1325 +76
Branches 0 316 +316
==========================================
+ Hits 1071 1079 +8
+ Misses 178 173 -5
- Partials 0 73 +73
☔ View full report in Codecov by Sentry. |
|
@vishnureddy17 @BertKleewein All tests are working now 🚀 |
src/lib/connect/index.ts
Outdated
| export default function connect( | ||
| brokerUrl: string | IClientOptions, | ||
| opts?: IClientOptions, | ||
| ): MqttClient { |
There was a problem hiding this comment.
maybe do function overloading here? one signature with just an opts param and another signature with the url string and opts
There was a problem hiding this comment.
Eveytime I try using function overloading in typescript I give up... I always get the error that says the overloading are not compatible
There was a problem hiding this comment.
@vishnureddy17 I have been able to add them, check: 1c56921 (#1641)
|
Is it ready for review or are you still working on it? @robertsLando |
|
@vishnureddy17 the code is ready for review, I was just testing that |
| import EventEmitter from 'events' | ||
| import { applyMixin } from './shared' | ||
|
|
||
| export type EventHandler = |
There was a problem hiding this comment.
Why do we need to have these types? Shouldn't there be typings available for EventEmitter already?
There was a problem hiding this comment.
A friend of mine (@AlCalzone, a TS wizard 🧙🏼♂️ ) suggested me this a while ago. Based on what he told me the TypedEventEmitter is far better because:
you get autocomplete for the events that exist, and you get syntax help for the callback parameters, and typescript will yell at you if you accidentally use an event wrong. No more typos in event names or non-existent events, no more wrong callback params, no more looking up elsewhere what callback params there are
There was a problem hiding this comment.
Examples for a class that is such a TypedEventEmitter:
|
@vishnureddy17 @BertKleewein I'm done here, I will check the code another time to see if I spot something but I would merge this if you agree. If you want to take some time to review this just let me know and I will wait till next week before merging |
|
If you think it's good to merge, go for it |
Fixes #1640