v2 rework to promises #126
Conversation
| */ | ||
| init(pluginConfig, callback) { | ||
| init(pluginConfig) { |
There was a problem hiding this comment.
make async? ... also for the other methods ... then simply throw
There was a problem hiding this comment.
Why? They are returning Promise<void> On type level how you achieve this on implementation is up to code of specific plugin, there it will likely be async. See https://github.com/ioBroker/plugin-base/pull/126/files#diff-91cd6348c12238c4c2afe46f71253579dbc43ddb1db8872b76bff80ff25025e2R34 for type level
There was a problem hiding this comment.
yes... just personal preference and "the same as in other classes" ... I hate "new Promise.reject" constructions in the meantime :-)
There was a problem hiding this comment.
Normally we have eslint rules which complain if you have async function without doing anything async thus I prefer new Promise for such scenarios 😀
There was a problem hiding this comment.
lol ... ok ... strange ... the throw normally suppreses that ... but all good
| try { | ||
| await this.init(pluginConfig); | ||
| await this.setActive(true); |
There was a problem hiding this comment.
code is not the same ... old code returned a boolean and only if that boolean is true (setActive is called with true ... so here better const success = await .... setActive(success)
... ok OORRR methiod is now throwing also in non error but unsuccesseffull cases ... also good, just needs to make clear in docs
There was a problem hiding this comment.
| typeof callback === 'function' && callback(); | ||
| }) | ||
| try { | ||
| await instance.initPlugin(this.plugins[name].config, parentConfig) |
There was a problem hiding this comment.
same her e... init not successfull ... needs to always throw
| async initPlugins(parentConfig) { | ||
| for (const [pluginName, plugin] of Object.entries(this.plugins)) { | ||
| if (!plugin.instance) return; | ||
| await this.initPlugin(pluginName, parentConfig); |
There was a problem hiding this comment.
I think doing them sequencially is ok ... could also be done in oparallel as before
There was a problem hiding this comment.
We can also go for promise.all but I guess it won't have an significant impact. Okay rn we only have sentry but also in general. If you want I can change but I would personally keep it.
Breaking Changes:
asyncpostfix are now working renamed to methods without the postfixwhile the callback methods have been removed
instanciatePlugintoinstantiatePluginisPluginInstanciatedtoisPluginInstantiated