make sense for me

I guess it was not marked as deprecated before becaus we changed the plugins way later.

The constructor change should be done using @SharkyKZ 's PR here #39387

The constructor change should be done using @SharkyKZ 's PR here #39387

Yeah, I have copied that code from him, as I wrote in the description.
When he will update his PR (to make it b/c), I will remove it from my PR.

Why are you hellbent on neutering plugins again? It's bad enough that you have already poorly implemented event classes while also introducing a number of undocumented B/C breaks without a deprecation period as well as garbage code like #40037 (comment). What benefit do you actually see in forcing plugins to use only SubscriberInterface? Is there actually some plan to make subscribers useful in the future or are you removing custom listeners just because?

Why are you hellbent on neutering plugins again?

I would like to see the plugins to be as light as possible (there still a long way to go).

poorly implemented event classes while also introducing a number of undocumented B/C breaks

Please more detailed. From quick look on the link, it is probably some bug in implementation, or misuse of the event, which was hiding the bugs in the past.

garbage code like ...

There a loot more garbage ?
But only who make nothing makes no mistakes ?

What benefit do you actually see in forcing plugins to use only SubscriberInterface?

Perfomance, and better code, did you seen code of registerListener and registerLegacyListeners?

But it was set to be this way before, in past, somewhere. From my understanding, it was planed to be similar to Symfony SubscriberInterface.

When we drop legacy listeners and use Dispatcher which will look for SubscriberInterface, then having registerListeners does not make sense to me.

Is there actually some plan to make subscribers useful in the future or are you removing custom listeners just because?

What is NOT usefull for you?

If you have some suggestion to improve it, or alternative ideas, just write it ?

Virtually none of what you said is valid. How exactly does removing support for custom listeners make plugins "as light as possible"? Why did you even mention registerLegacyListeners? It only exists for compatibility with J3 plugins so it's not relevant at all. And what poor performance or bad code do you see in registerListener method? It's literally the correct and most performant way to register listeners. The doc block does say it's the preferred way and should be the only way... So again I have to ask what actual benefit do you see in using subscribers? Because, unless you're cooking up some more Joomla! magic™, subscribers are always going to be slower than custom listeners. Plugins implementing SubscriberInterface are monolithic classes that have to have all their dependencies set up early on, even if their listeners are not used. Meanwhile, normal plugins can use any callable as a listener. Thus they can delegate the actual logic to other classes which can be lazy loaded when a given event is triggered. The event package does already provide Joomla\Event\LazyServiceEventListener class for doing something like that.

Virtually none of what you said is valid

Virtually everything I said is correct, except things that are not correct. Easy.

How exactly does removing support for custom listeners make plugins

Removing $reflectedObject = new \ReflectionObject($this);

And what poor performance or bad code do you see in registerListener

Sorry, I mean registerListeners, but registerListener also useless piece of code.
BTW, you said plugin should not implement DispatcherAwareInterface, but registerListener relies on this.
So, sorry I am cannot follow you here.

Plugins implementing SubscriberInterface are monolithic classes that have to have all their dependencies set up early on, even if their listeners are not used. Meanwhile, normal plugins can use any callable as a listener. Thus they can delegate the actual logic to other classes which can be lazy loaded when a given event is triggered.

Well, because we have all this bootPotato() stuff, developers can implement BootableExtensionInterface on it,
and do what they want with their "lazy" or "not very much lazy" stuff. I do not see problem here.

The event package does already provide Joomla\Event\LazyServiceEventListener class for doing something like that.

Keeping registerListeners does not help with it.
This "something like that", what no one know it exists and how to use it.

To me, removing registerListeners will turn the plugin to a "bare" entity, and the developers can decide if they want to implement SubscriberInterface or BootableExtensionInterface (for lazy stuff) or anything else.
But that is still an open topic (how to make it better), and does not prevent us from registerListeners deprection.

Removing $reflectedObject = new \ReflectionObject($this);

?‍♂️ Why are you bringing up J3 code again? You still don't understand it's irrelevant? It will be removed when J3 plugin support is dropped, regardless whether registerListeners remains or not.

Sorry, I mean registerListeners, but registerListener also useless piece of code.
BTW, you said plugin should not implement DispatcherAwareInterface, but registerListener relies on this.
So, sorry I am cannot follow you here.

You missed the part where the dispatcher would be passed to the method. But nevermind that, registerListener() is indeed a useless shortcut. It has similar limitations as SubscriberInterface in that in can only register public methods of the plugin class and not proper callables. Anyways, it's an implementation detail of CMSPlugin so its removal doesn't have a huge impact; plugin developers can re-implement it if they need it.

Well, because we have all this bootPotato() stuff, developers can implement BootableExtensionInterface on it,
and do what they want with their "lazy" or "not very much lazy" stuff. I do not see problem here.

That doesn't make sense. Not only is it not the right to place to register listeners but it also wouldn't work because the correct dispatcher instance is passed to the plugin after the plugin has been booted. This is correct and expected behavior but it's the way the dispatcher is passed that needs to be fixed.

This "something like that", what no one know it exists and how to use it.

What kind of argument is this supposed be? LazyServiceEventListener is not some sort of secret. Like any other class, it's part of Joomla's public API, it's already used in core and by 3rd party plugins. This just shows that you are unfamiliar with the event codebase and you're trying to introduce changes you don't understand with negative consequences to core and 3rd party developers.

Whatever, you just complaining about anything.

registerListeners need to be removed or replaced, and for this it need to be deprecated.
btw, if we going to keep setDispatcher() in CMSPlugin, it is already a replacement, kind of.

I have split the PR to 2:

• #43430
• #43431

Please review.
I close here for now, if they is not good I will reopen current.

For now, the replacement for deprecated registerListeners() method is in follow up PR:

• #43462

I have tested this item ✅ successfully on 267054c

This pull request has been automatically rebased to 5.3-dev.

I have tested this item ✅ successfully on 267054c

Apllied this patch. After appliying patch, site working properly.

I haven't read the long discussion on this PR, but want to point out that the method is useful for me (and maybe other third party developers)

Basically, some of my plugins are integrated with third party extensions, so I override this method to only register plugin if the integrated extension is enabled. Something like below:

public function registerListeners()
{
	if (!ComponentHelper::isEnabled('com_acym'))
	{
		return;
	}

	parent::registerListeners();
}

If this method is deprecated/removed, what's the right way to handle something like this? Change the logic to getSubscribedEvents and return empty array if the extension is not enabled?

That is valid issue, one of solution I suggested:

• #43657

Please check, if it will work for you.

I haven't read the long discussion on this PR

The PR code changed very much since when I created it first

Please check, if it will work for you

Yes, I saw it on other day. It could be the solution for the issue, but I asked myself why it needs to that complicated? Couldn't we just add shouldRegisterListeners method to to CMSPlugin class and return true by default. Then specific plugin can override that method if needed. But that should be discussed on the PR.

Back to the change this PR, honestly, I don't see the benefit of depcrcating/removing the method:

• You will have to add shouldRegisterListeners method (something like that) to offer the option which is already possible with registerListeners as I am using in my plugins.
• We loose some features which can do with registerListeners (we might not use in our core plugins, but maybe third party developers who is smart enough can use it :D). I will just copy the comment from Sharky here:

Meanwhile, normal plugins can use any callable as a listener. Thus they can delegate the actual logic to other classes which can be lazy loaded when a given event is triggered. The event package does already provide Joomla\Event\LazyServiceEventListener class for doing something like that

...but I asked myself why it needs to that complicated? Couldn't we just add shouldRegisterListeners method to to CMSPlugin class and return true by default

We can, but we should not. Use of the interface allows us to clearly distinguish plugins that explicitly requesting this feature. And do not call the method for ALL plugins, which save us a few nanoseconds.
Also this allow more easily change implementation when we decide to. And will keep CMSPlugin nice an clean.

We loose some features which can do with registerListeners...

Yea, but after changing to SubscriberInterface a current implementations registerListeners kind of useless.
It would require extra argument to make it work, but it will be hard b/c break, that will not going to happen.
Sharky suggested it in his PR:

• #39387

However if it will be a "popular feature request" we could do something similar, but in b/c way, I tried with:

• #43431

It can be some kind of combination of all these PR-s: with different method/interface name, different method arguments.
But it should be isolated form CMSPlugin and optional.

We need to reach an agreement what we want in the end
I am fine with trashing all of it, and just use SubscriberInterface 😄

We can, but we should not. Use of the interface allows us to clearly distinguish plugins that explicitly requesting this feature. And do not call the method for ALL plugins, which save us a few nanoseconds.

I expect not too much difference between checking instanceof and calling a method a method just contains a return true statement :).

Yea, but after changing to SubscriberInterface a current implementations registerListeners kind of useless.
It would require extra argument to make it work, but it will be hard b/c break, that will not going to happen

Just change method registerListeners signature, right? I would say if we have to make change, do it right. I don't think there are many developers override that method and compare to removing support for legacy listeners, that change has minimum affect.

We need to reach an agreement what we want in the end

I will leave it to some of you who has more knowledge with these stuffs to decide. Should be a topic for maintenance meeting.

Remove the registerListeners function it's not useful at all, I haven't seen a valid usecase for not simple register to all events you might need.

keep it simple, one function which returns the events you want to register to.

I personally would need to register to one event twice but can life without it.

if you can give me a usecase when you not want to register to an event it might be easier to talk about it.

Just change method registerListeners signature, right?

Correct. I think it is more safe to add new method than changing existing, then it will be a "smooth upgrade" operation.

But in any case it should be detached from PluginInterface, as separated feature.
It will keep code more modular and clean, in my opinion.

Remove the registerListeners function it's not useful at all, I haven't seen a valid usecase for not simple register to all events you might need.

I actually mostly only care about the case I mentioned in earlier comment #43395 (comment)

if you can give me a usecase when you not want to register to an event it might be easier to talk about it

I do no have a valid use-case myself. But Sharky gave some valid points in his comment, especially if someone wants to use Lazy event listeners

Plugins implementing SubscriberInterface are monolithic classes that have to have all their dependencies set up early on, even if their listeners are not used. Meanwhile, normal plugins can use any callable as a listener. Thus they can delegate the actual logic to other classes which can be lazy loaded when a given event is triggered. The event package does already provide Joomla\Event\LazyServiceEventListener class for doing something like that

I actually mostly only care about the case I mentioned in earlier comment #43395 (comment)

Not really valid, because first it's a global thing you (shouldn't do anyway) can do the same in the getSubscribed function if you really think that brings any gain. For the "is component active" thing it doesn't matter if you listen to events from this component because if it's disabled they didn't get triggered.

if you can give me a usecase when you not want to register to an event it might be easier to talk about it

I do no have a valid use-case myself. But Sharky gave some valid points in his comment, especially if someone wants to use Lazy event listeners

That's a different scenario and if I understand it correctly would be now true for all events that uses lazyproxy registration.

The much bigger gain now is that you can do things in the constructor without overhead because it only gets triggered if an event is triggered.

For the "is component active" thing it doesn't matter if you listen to events from this component because if it's disabled they didn't get triggered.

Not fully correct. If it Content/Model events like onPrepareForm, BeforeSave, PrepareContent etc. then it will be triggered.
Just saying

Not really valid, because first it's a global thing you (shouldn't do anyway) can do the same in the getSubscribed function if you really think that brings any gain

Actually, as of right now, that is the correct way. It prevent register listeners in my plugin if the component which plugin depends on is not enabled/installed on the site. The component here is not the component which trigger events but the component receives data sends from my plugin.

For the "is component active" thing it doesn't matter if you listen to events from this component because if it's disabled they didn't get triggered

I guess you misunderstood the case. My plugin depends on events triggered by my component, not other component, so the events are always triggered. But we can only push data (for example, subscribe user to ACYMailing Lists) if ACYMailing extension is installed/enabled. And for that, we only register listeners in the plugin if ACYMailing component is enabled and the way I am using right now is the most correct way.

That's a different scenario and if I understand it correctly would be now true for all events that uses lazyproxy registration

I haven't read that PR carefully, but a quick look, it requires PHP 8.4 which is not always available. In theory, lazy events listeners which is already supported in our event package, do the same thing, and does not requires PHP 8.4.

We can, but we should not. Use of the interface allows us to clearly distinguish plugins that explicitly requesting this feature

@Fedik In real life, as third party developer, we usually have to support at least two Joomla versions. So assume you introduce the interface in Joomla 6, I guess there is no way for us to have a single plugin class, one for Joomla 5 (no interface available) and one for Joomla 6 (with the newly introduced interface). Just want to mention something to concern/think about. No worry, I can always find a workaround with the any decisions :D.

I guess there is no way for us to have a single plugin class, one for Joomla 5 and one for Joomla 6

Not fully correct. The method is not removed.
In current implementation it will stay until 6.x, and removed/changed in 7.0 (could be extended, depend from situation).
Will be a loot of time for extensions. Also the interface can be add in 5.x, but enabled on 6.x.

But that is details.

Not fully correct. The method is not removed.
Ah, Yes. I can still use the old way for sometime.

Not fully correct. The method is not removed.

Ah, Yes. I can still use the old way for sometime.

But yea, the sooner we decide what we want the better. for 3rd developers.
But from other side, we always can extend it to 8.0 or longer 😄

My suggestion to limited the subscribed events would be use provide all needed variables to the getSubscribedEvents such as $app, $container, $dispatcher. Not sure if more is needed to get to an conclustion.

... provide all needed variables to the getSubscribedEvents such as $app, $container, $dispatcher

Anything but not changing existing interfaces.
It will lead for b/c chaos, but will benefits only for a few plugins.

as already said not changing existing functions of course instead introducing a new one, but tbh I'm not sure if this is really needed.

I was playing around with lazy objects introduce in PHP 8.4. The issue is that plugins do register their events within the class, in worst case with some conditions. Like that we will never be able to lazy instantiate them. As long as we don't have a way to register events outside of the plugin class (service provider, cache, manifest file, etc.) there will be no benefit to deprecate this function or do some other complicated stuff.

I was playing around with lazy objects introduce in PHP 8.4. The issue is that plugins do register their events within the class, in worst case with some conditions. Like that we will never be able to lazy instantiate them. As long as we don't have a way to register events outside of the plugin class (service provider, cache, manifest file, etc.) there will be no benefit to deprecate this function or do some other complicated stuff.

If there is no condition in the getSubscriptedEvents() it's lazy loaded, so this pr works it should or do I miss anything?

As soon as a function is called of the plugin class, the plugin object is instantiated. With this pr it is getSubscribedEvents.

Even when the function here would be called in a static way, the lazy factory is does instantiate an object here. Did a test this morning.

I also tested this a month ago and it doesn't initialize the class as long as it doesn't touch the $this object.

https://3v4l.org/OERUAl#v8.4.6

@laoneo you played it wrong 😉

Do it with that PR #45062

You are right, there was some strange caching issue in combination with a messed up composer install. After a server restart, it worked as expected. Sorry for the noise.

After playing a bit more around with this. I'm brave now and would say that in 6.0 we just remove the setDispatcher call in the PluginHelper and properly document that the plugin developer needs to set the dispatcher in the service/provider.php file by itself. Adding a marker interface is overhead for me. The important point is that the plugin developer can be compatible down to 4 while setting the dispatcher in the provide file. This outweights for me more instead of adding a new deprecated interface where they have to add it now and then remove it again.

I'm brave now and would say that in 6.0 we just remove the setDispatcher call in the PluginHelper and properly document that the plugin developer needs to set the dispatcher in the service/provider.php file by itself.

Unfortunately not that easy. Try play more. Remove the setDispatcher call in the PluginHelper and then try login on the site, Good luck with that

The thing that we can boot plugins into own dedicated Dispatcher instance, there is no way service/provider.php will know it. And if your plugins should be able to dispatch some events in this dispatcher then you are in trouble.

I added an example implementation which allows to provide an own container to the component boot methods based on fedirs PR at Fedik#12

@Fedik I did that as part of #45392. Worked as expected, but did only change the fields plugin to lazy.

I think we need to accept that plugins with DispatcherAwareInterface cannot be lazy.
However this is deprecated already in #43430

I suggest following:

• in 5.4 we remove these interfaces from CMSPlugin, but will keep traits,
• setDispatcher() will be called only for plugins which explicitly have this interface on their own class,

Thoughts?

Other idea would be doing something like here #43431 (which is alternative to #39387). But I do not very like it much.

I think 5.4 is not optimal to remove the interface, removing it in 6.0 should be ok and remove the trait in 7 (I think that's already planned right?)

Yeah, but if we keep traits nothing will explode, at least :)

Ah, wait, I wanted to write different, not in 5.4 but in 6.0.
This:

• in 5.4 for plugins with PluginWithSubscriberInterface (new in 5.4) we ignore registerListeners() and DispatcherAwareInterface (as it done in this PR)
• in 6.0 we remove DispatcherAwareInterface interface from CMSPlugin, but will keep traits. And also add check for DispatcherAwareInterface for plugins which explicitly use it on their class (similar to as it done in #45062 but for all plugins)
• in 7.0 we remove traits

Introducing a new Interface makes no sense to me, just remove the DispatcherAwareInterface in 6.0 and the trait in 7.0.

Only call the registerListeners() if SubscriberInterface is not set, maybe a bit more b/c friendly could be to check if the the method is overwritten and call it if it's overwritten but to be honest I would do that.

Only call the registerListeners() if SubscriberInterface is not set

That is not possible, we already discussed that. It will be b/k break.

yes I know, but not if you check if the function is overridden or?

Maybe, but checking whether function is overridden would require use of reflection for every plugin. Use of interface is much cleaner.
Or I missing something?

Yeah the interface might be cleaner, but it also means a 3rd party dev needs to touch the plugin for this reason 2 times...

An alternativ could be to override the methods in with dummy/empty methods in core this would not initialise the object and wouldn't need to be removed (even if it should) later to be b/c.

especially if you like that your plugin works between 4.0 and 7.4 it's hard (not really possible with out defining your own poly-fill) to use a new interface,

Agree here with @HLeithner, as removing the interface will be problematic. I keep my extensions at least the two major version compatible, when not even three. So having a required interface in 6 which get removed in 8 and is still used in 7 would be problematic.

I think we need to accept that plugins with DispatcherAwareInterface cannot be lazy.

I'm fine with this. In the provider.php the plugin knows if the setDispatcher function exists or not to inject the dispatcher.

I removed the extra interface from the PR.
Will find a solution in another PR, here just deprecation.

I have tested this item ✅ successfully on bff97f4

Deprecation messages do look ok

The PR for the manual joomla/Manual#422 is made for the migrations/52-53/new-deprecations.md document. Should that not be migrations/53-54/new-deprecations.md?

I've restored @laoneo 's test result in the issue tracker as the commit which invalidated the test count was just a grammar change in a comment.

I have tested this item ✅ successfully on bb9a233

RTC

thanks all for participation