Refactor plugin #225
Refactor plugin #225
Conversation
lib/load-plugin.js
Outdated
| @@ -31,12 +32,32 @@ function loadPlugin (instance, plugin, callback) { | |||
| // place the plugin at the top of _current | |||
| instance._current.unshift(plugin) | |||
|
|
|||
| plugin.exec((last && last.server) || instance._server, (err) => { | |||
| if (instance._error && !plugin.isAfter) { | |||
There was a problem hiding this comment.
I moved this from plugin.exec, as it seemed odd to me, that we could do this already in the loadPlugin function, and decouple so Plugin from Avvio.
lib/plugin.js
Outdated
| const err = new AVV_ERR_READY_TIMEOUT(name) | ||
| err.fn = func | ||
| done(err) | ||
| const readyTimeoutErr = new AVV_ERR_READY_TIMEOUT(name) |
There was a problem hiding this comment.
Wouldnt it make more sense to create and use a AVV_ERR_PLUGIN_EXEC_TIMEOUT?
There was a problem hiding this comment.
Imho there are two timeouts. One is that the plugin loaded too long. For this I throw AVV_ERR_PLUGIN_EXEC_TIMEOUT. The other timeout is when the overall loading takes simply too long, e.g. all plugins take together more time than the avvio is allowed to.
I think it should be differenciated. But I am willing to revert that change.
There was a problem hiding this comment.
We should discuss here if the new Error makes more sense.
|
I have some question and ask for feedback ;) |
|
Do you have maybe some input regarding my questions? |
There was a problem hiding this comment.
Seeing the changes, I'm starting to think if it would be worth it to explore a further change to the data structure that manages the plugins. Currently is a queue (that slightly mimics a linked list in the sense that they are more or less aware of each other).
Instead, if we can successfully shift it to a tree, we would be able to enable several functionalities (like parallel loading) by identifying plugin dependants to locate nodes that can be loaded in parallel (does not depend on other plugins, a leaf node basically).
Just thinking out loud, but is it something we can explore further in another PR?
I do not have the mental capacity right now to study this codebase and provide useful feedback. |
|
@jsumners @metcoder95 |
|
Thank you. I will hopefully provide more PRs today. ;) |
Refactoring Plugin.
Trying to use more meaningful parameter names. And also kind of decoupling avvio/boot from plugin.
I decoupled plugin from boot/avvio and added a new Error AVV_ERR_PLUGIN_EXEC_TIMEOUT
loadPlugin is now a method of boot/avvio.
Checklist
npm run testandnpm run benchmarkand the Code of conduct