joomla-cms icon indicating copy to clipboard operation
joomla-cms copied to clipboard

[5.2][RFC] CMSPlugin: deprecation for registerListeners and add new ConfigurableSubscriberInterface

Open Fedik opened this issue 1 year ago • 2 comments

Summary of Changes

This is alternative to

  • https://github.com/joomla/joomla-cms/pull/43395
  • https://github.com/joomla/joomla-cms/pull/39387

The PR deprecate CMSPlugin::registerListeners() as no longer needed when plugin will implement SubscriberInterface.

And introduce ConfigurableSubscriberInterface as replacement. The interface allows to set up custom event listeners, which is not possible to set up with generic SubscriberInterface (such as LazyServiceEventListener, private listeners etc.). And/or set the listeners only when specific environment requirement are meets (such as the app client id).

I have added couple examples how it could work. Look in to: GuidedTours plugin: https://github.com/Fedik/joomla-cms/blob/e8f9aa5961492ed17f99350791e601658bafb99c/plugins/system/guidedtours/src/Extension/GuidedTours.php#L86

ScheduleRunner plugin: https://github.com/Fedik/joomla-cms/blob/e8f9aa5961492ed17f99350791e601658bafb99c/plugins/system/schedulerunner/src/Extension/ScheduleRunner.php#L65

Why new interface instead of changing existing registerListeners() (as it done in #39387)?

Because of backward compatibility, and modularity. It is more easy to add it right now, without waiting 5 years for "deprecation circle".

I do not like the name!!! Can we rename it?

Yes. Suggest an alternative.

NB

Alternatively, it would be good to create interfaces similar to SubscriberInterface but without static declaration for getSubscribedEvents, and which allows to have a callables in return. Kind of DynamicSubscriberInterface. But for this need to do more update in freamwork Joomla\Event\ side. I would prefer this approach.

Testing Instructions

Code review. Check GuidedTours and ScheduleRunner plugins still works.

Actual result BEFORE applying this Pull Request

Works

Expected result AFTER applying this Pull Request

Works

Link to documentations

Please select:

  • [ ] Documentation link for docs.joomla.org:
  • [ ] No documentation changes for docs.joomla.org needed
  • [ ] Pull Request link for manual.joomla.org: TBD
  • [ ] No documentation changes for manual.joomla.org needed

Please write your opinions, and please without complaining :smile: Thanks.

Fedik avatar May 06 '24 12:05 Fedik

Alternative

I have prepare also an alternative to ConfigurableSubscriberInterface (in framework repo):

  • https://github.com/joomla-framework/event/pull/36

Wich removes limitations of SubscriberInterface, and I think that would be more cleaner solution.

Instead of https://github.com/joomla/joomla-cms/blob/4c9f2b39f4ca25caba88fb0a588f467165958328/plugins/system/guidedtours/src/Extension/GuidedTours.php#L93-L99

The alternative allows to do:

public function getSubscribedEvents(): array
{
        return $this->getApplication()->isClient('administrator') ? [
            'onAjaxGuidedtours'   => 'startTour',
            'onBeforeCompileHead' => 'onBeforeCompileHead',
        ] : [];
}

Fedik avatar May 06 '24 14:05 Fedik

I'm against having any logic in the "get Subscriber Events", this function call should be so simple and fast as possible.

It makes no sense to have compute time for each plugin which might never be called. Many of our events are called only once, compared to that our system plugins (and some more) are loaded on each request.

If you don't want to have your event listener execute on an event do an early return in the triggered function, problem solved.

HLeithner avatar May 07 '24 06:05 HLeithner