User tests: Successful: Unsuccessful:
Pull Request for Issue # .
The PR deprecate CMSPlugin::registerListeners()
as no longer needed when plugin will implement SubscriberInterface
(I wonder why it was not marked as deprecated before).
Apply PR, and navigate around the site, all should work as before.
The PR requires review from maintainers.
Please select:
Ping @HLeithner @wilsonge
Status | New | ⇒ | Pending |
Category | ⇒ | Libraries |
Labels |
Added:
Feature
PR-5.2-dev
|
I guess it was not marked as deprecated before becaus we changed the plugins way later.
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.
Category | Libraries | ⇒ | Libraries Front End Plugins |
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2024-05-06 12:11:52 |
Closed_By | ⇒ | Fedik |
Status | Closed | ⇒ | New |
Closed_Date | 2024-05-06 12:11:52 | ⇒ | |
Closed_By | Fedik | ⇒ |
Status | New | ⇒ | Pending |
Category | Libraries Front End Plugins | ⇒ | Libraries |
Title |
|
I have tested this item ✅ successfully on 267054c
This pull request has been automatically rebased to 5.3-dev.
Title |
|
make sense for me