moscajs
chore: migrate to ES6 classes
This PR migrates Aedes to ES6 class syntax.
This should help in further refactoring.
I tried to migrate everything that looked like a class but I might have missed one as this is also used in functions that are not constructors but whose this will be set by parallel etc.
Kind regards, Hans
Pull Request Test Coverage Report for Build 15754464211
Details
- 337 of 337 (100.0%) changed or added relevant lines in 9 files are covered.
- No unchanged relevant lines lost coverage.
- Overall coverage decreased (-0.007%) to 99.568%
| Totals | |
|---|---|
| Change from base Build 15752165255: | -0.007% |
| Covered Lines: | 810 |
| Relevant Lines: | 811 |
💛 - Coveralls
@robertsLando the constructor of Aedes immediatly starts running,e.g. it contains:
this._heartbeatInterval = setInterval(heartbeat, opts.heartbeatInterval)
and:
that.publish({
topic: birthTopic,
payload: bufId
}, noop)
My suggestion would be to migrate these actions to a separate async method as discussed before where we also can initialize async persistence as those action should imo come after persistence has been initialized.
We already have Aedes.createBroker() which we could repurpose for that.
As long as we are using the callback persistence we can automagically call this in the createBroker factory. In the next major version the way to start Aedes could then be:
const { Aedes } = require("aedes")
const aedes = await Aedes.createBroker(opts)
What do you think?
@seriousme I was going to comment the same on the other issue, I think, like we do in MQTTjs, that having an helper function createBroker that can be async could be the best solution here. Of course this will be a major release :)
btw: I'm still getting to know the codebase (and I got some days off, hence the extra activity, sorry for that 😉 ) so you might see some more optimization PR's before I get to changing the persistence interface.