plugin load sequence

exodo

@pichalite said in plugin load sequence:

@exodo explain?

If a plugin is slow it wont solve to put it first. It will still finish last. Happened to me with many plugins

yariplus

I was wondering about that, the plugins load asynchronously it seems, so what does the order even do?

pichalite

@yariplus hmm I didn't look at the code I just thought it worked that way based on what I read here when this was implemented.

pichalite

based on this... looks like they are loaded in order one after the other...

https://github.com/NodeBB/NodeBB/blob/master/src/plugins.js#L119

yariplus

@pichalite That makes sense, but the app.load hook is still called asynchronous. (each instead of eachSeries)

https://github.com/NodeBB/NodeBB/blob/master/src/plugins/hooks.js#L169

So there is no way to tell if another plugin has finished loading yet.

yariplus

This topic is deceiving though. The plugin load order has nothing to do with the issue. Like I said in my first post,

Plugins should not be replacing default templates, period.

It is ugly, not guaranteed to work, and breaks every other plugin that attempts to do the same.

I already suggested a more sensible system here.

https://github.com/NodeBB/NodeBB/issues/4728

pichalite

@yariplus so, the plugins are registering for the hooks in order but when the hook is fired, they run asynchronously?

yariplus

@pichalite That seems to me to be the case.

julian

@yariplus said in plugin load sequence:

@pichalite That seems to me to be the case.

However, the code does not agree -- they are run in series.

They do time out for performance reasons, however. e.g. a static hook will continue with the next listener if the current one takes longer than 2 seconds.

frissdiegurke

@julian said in plugin load sequence:

@yariplus said in plugin load sequence:

@pichalite That seems to me to be the case.

However, the code does not agree -- they are run in series.

They do time out for performance reasons, however. e.g. a static hook will continue with the next listener if the current one takes longer than 2 seconds.

That reference is for filter hooks, isn't it? @yariplus and @pichalite are talking about static hooks (e.g. static:app.load). For static hooks @yariplus already referenced the relevant line to prove his point.
With that in mind I don't see any obvious solution/hook for that purpose, nor any reasonable use-case.

julian

Aah yes... static hooks are called asynchronously for performance reasons. Thanks for pointing that out @frissdiegurke, my apologies for missing your link in the earlier post @yariplus

We could change it to eachSeries... not that big a deal, to be honest.

yariplus

I'm fine with it being each. But that's really a separate issue. The app.load hook is not where we should be modifying templates.