Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Cms] Hook for after all plugins are loaded #23

Closed
move bot opened this issue Oct 25, 2018 · 17 comments

Comments

@move
Copy link

@move move bot commented Oct 25, 2018

@fabianmichael commented on Jun 28, 2018, 4:14 PM UTC:

I tinkered a bit around with the new plugins API and thought that a hook that fires right after all plugins have been loaded might be kind of useful. As plugins are loaded in a certain order, it is currently not possible to execute something right after plugins have been loaded.

There a few use-cases where this might become handy for developers:

  • Detect which other plugins have been loaded and adapt behavior according to this.
  • Decorate field methods, Kirbytags or Thumb drivers with a wrapper function to provide additional functionality.

This issue was moved by bastianallgeier from k-next/kirby#534.

@bvdputte

This comment has been minimized.

Copy link

@bvdputte bvdputte commented Apr 12, 2019

If we'ld like to build upon plugins, from plugins, it's a must have (to keep our codebase DRY).

It could serve to "instantiate" plugins once all plugins are loaded (AFAIK this is now not possible).

@lukasbestle

This comment has been minimized.

Copy link

@lukasbestle lukasbestle commented Jul 12, 2019

This is definitely something we need to have in the core and I'm planning to add this to one of the next releases.

I'm wondering though: Will a hook work for all use-cases? To test this, I need some examples of what you would do with this feature, if possible in the form of code examples and a little description. Thanks in advance! :)

@bvdputte @sylvainjule @bnomei @medienbaecker

@bnomei

This comment has been minimized.

Copy link

@bnomei bnomei commented Jul 13, 2019

  • use my dotenv plugin in closure called by config of another plugin like sylvanjules matomo plugin. this only works right now in using the classes directly issue
  • create a simple before/after plugins timetracker to determin loadtime of plugins (adding and removing some to determin impact)
@medienbaecker

This comment has been minimized.

Copy link

@medienbaecker medienbaecker commented Jul 13, 2019

I primarily think of custom buttons for the markdown field or wysiwyg editor.

@bvdputte

This comment has been minimized.

@lukasbestle

This comment has been minimized.

Copy link

@lukasbestle lukasbestle commented Jul 14, 2019

Thanks for your replies!

Most of the examples (the dotenv example by @bnomei and the examples by @medienbaecker and @bvdputte) require some support in the plugins you want to extend.

I think the example by @medienbaecker requires a pretty custom solution (like a custom method in the plugins that can be called by your extensions). So that's something that would definitely work with a hook and I think that this is a very good use-case for it.

The other two examples would require overwriting plugin options after the plugins have been loaded. That's currently not supported as that could cause issues when the plugin already checks for an option when initializing. Changing that option later would not do anything. Another issue is a security-related one: The user doesn't expect that a plugin can modify system options that have been set in the config file.

So the question is: Is there maybe another way than overwriting plugin options that would lead to the same result?

@sylvainjule

This comment has been minimized.

Copy link

@sylvainjule sylvainjule commented Jul 14, 2019

I would love for this hook to allow me to build on top of other plugins by extending their config and components, just like we currently can with the core fields.

An option like 'loadAfter' => 'markdown' that would ensure that the extend property can be called on the stated plugin's components.

Ideally, it could work like mentioned here: sylvainjule/kirby-markdown-field#43

@lukasbestle

This comment has been minimized.

Copy link

@lukasbestle lukasbestle commented Jul 14, 2019

Hm, but aren't we mixing backend and frontend now? The hook is only going to fire on the backend, so it won't let you extend Vue components. Or have I misunderstood anything?

@bvdputte

This comment has been minimized.

Copy link

@bvdputte bvdputte commented Jul 14, 2019

I was thinking like this:

  • Plugin A with specific functionality
  • Plugin B with another specific functionality

I want to build a new plugin (C) which needs A + B, but both initialized with "defaults" for plugin C (which are not necessarily the defaults that are set as default in the separate plugins). Then I could release my plugin C in an opinionated way so that it can simply be installed together with the dependant plugins and doesn't need config done for all plugins.

AFAIK we can't set options for plugins in another plugin. The only way to do so would be to have the developer copy-paste some config boilerplate to the config.php-file.

I thought, if there was a hook which could be called when all plugins were loaded, we could set options there then. Then we could off-load the instantation of options to a custom init-function in the plugin which would be called in this hook?

When options have been set in the config.php, they should be the ones to be used. I don't think I'ld want plugins to be able to override those. It's only when they aren't defined in config.php, we should be able to do so?

Does this make any sense?

@lukasbestle

This comment has been minimized.

Copy link

@lukasbestle lukasbestle commented Jul 14, 2019

Makes sense, but I'm not 100 % sure if this would work. The issue with options that are already used when the plugin is loaded is still unsolved and the possible security issue is also still there – a user could rely on the default value for an option because it's what the user wanted and then not expect that that value got changed by another plugin.

Maybe we could still allow this as a compromise, but I think we should first find out if there are ways completely outside of options. Would it for example be acceptable if plugins had to have explicit entry-points for third-party extension? Or would that hinder you because you would want to extend third-party plugins in ways the original author didn't intend?

@bvdputte

This comment has been minimized.

Copy link

@bvdputte bvdputte commented Jul 15, 2019

I guess that would do too.

I can't speak for other plugin devs, but I tend to like plugins which do 1 thing (in the best way). Then if I'ld need that, I need a simple way to plug that in to another plugin, which sometimes needs initialisation (what we can't do now).

If we're talking about "extending" a plugin, then it usually means that you change the "thing" it does: I'ld rather do in a separate plugin which can extend the original plugin's class in my "new way" (I know this would mean that plugins are coded with classes, which isn't always the case now).

Does this still make sense?

@lukasbestle

This comment has been minimized.

Copy link

@lukasbestle lukasbestle commented Jul 15, 2019

As I wrote, the plugin you are extending would need some kind of entry-point for extension. Let's make an example:

Let's assume there is a queue plugin that runs scheduled tasks in the background using a cronjob. You want to extend that plugin to run a task of your own plugin. So in the "plugins loaded hook" you would check if the queue plugin is installed and if so, you would call a method on that plugin's class to register a task with it. That method would need to be provided by the queue plugin. What the new Kirby core feature would provide is the hook so you can run custom code after the plugins have been loaded. Plugin extension would still be something that would need to be provided by the specific plugins.

I think that this would be the most solid way to implement this. Of course, it doesn't provide as much flexibility as if you could override any part of a third-party plugin, but it's more secure and also easier to debug. Would that work for you all or do you need an additional core feature for this to be useful?

@bvdputte

This comment has been minimized.

Copy link

@bvdputte bvdputte commented Jul 15, 2019

Personally I believe this would be the necessary first step.

Agreed that we depend on the goodwill of other plugin devs, but in my experience the Kirby community is very helpful and open for collaboration. I also added composer "things" to my plugins as they were requested by other devs.

And this way, we can indeed keep Kirby secure and behaving as users expect.

@lukasbestle

This comment has been minimized.

Copy link

@lukasbestle lukasbestle commented Aug 17, 2019

There is now a PR for this here: getkirby/kirby#2014

Unfortunately it doesn't fully work yet, but we are working on it. :)

Roadmap automation moved this from Low-hanging fruits to Done Aug 17, 2019
@bastianallgeier

This comment has been minimized.

Copy link
Contributor

@bastianallgeier bastianallgeier commented Oct 14, 2019

@bvdputte

This comment has been minimized.

Copy link

@bvdputte bvdputte commented Oct 15, 2019

Where can I find more info (usage etc) about this please?

@lukasbestle

This comment has been minimized.

Copy link

@lukasbestle lukasbestle commented Oct 15, 2019

You can find the changed code here: https://github.com/getkirby/kirby/pull/2014/files
Docs on this will follow once 3.3.0 is out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Roadmap
  
Done
7 participants
You can’t perform that action at this time.