hapijs / glue

Server composer for hapi.js
Other
245 stars 62 forks source link

Possible registration order / dep resolution improvement #35

Closed devinivy closed 8 years ago

devinivy commented 8 years ago

I noticed this note here about dependency registration order. And recently saw issues #34, #19, https://github.com/hapijs/hapi/issues/2803 which made me think of this. I just wrote a library named hodgepodge that reorders a list of hapi plugins to respect their attributes.dependencies. Perhaps it could be an option in glue to use this reordering.

I would otherwise suggest that a user choose to use hodgepodge on their own then pass the ordered plugin registrations to glue, but glue's manifest is not a standard plugin registrations list that could be passed to server.register().

It's just a thought to potentially make glue a little more deterministic– not sure if the community would genuinely find this useful, but figured it would be worth putting the option out there.

devinivy commented 8 years ago

Might be worth looking into any opinions expressed here around plugin dep resolution: https://github.com/hapijs/discuss/issues/173

It may be better dealt with by plugin authors rather than plugin registerers like glue.

csrl commented 8 years ago

I looked at hodgepodge. Certainly it can help with broken plugins that one does not have control over. It could be interesting to merge this functionality into Glue so that one can still use registration options and plugin options. I'd be open to a pull request against the v3 branch.

devinivy commented 8 years ago

Cool. Perhaps I should update hodgepodge to hapi's ES6 style and get it tested on node 4 and 5. Should hodgepodge always be used during registration or should that be a glue option? Additionally, should the looseTally hodgepodge option be exposed through glue options, or just keep it true/false?

Worth adding that the trick explained in the docs for a plugin author to force usage of hodgepodge during registration no longer works because unknown plugin attributes are now allowed. Probably for the best.

Last thing I'd like to add is that I don't wish to encourage plugin authors to lazily/unknowingly misconfigure their plugins– that's my only concern baking this into glue. At the same time, it would be nice for glue to have a more deterministic plugin registration order. Just wanted to throw it out there!

csrl commented 8 years ago

The support for plugin ordering should be off by default and enabled as an option in the compose function options. Also, I'm not expecting to make Glue dependent on Hodgepodge, but rather add the ordering code to Glue itself.

I agree that providing this option is a hack. It is preferable that plugins are instead fixed to handle dependencies properly. I'm not interested in supporting this myself, but if the patch is minimal and functionality optional, I don't mind it existing.

v3 uses an array for registration/plugin loading, so plugins will be loaded in array order now. If that satisfies your need, then we can skip adding this.

lock[bot] commented 4 years ago

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.