Skip to content

[5.2][RFC] CMSPlugin: ConfigurableSubscriberInterface as replacement for deprecated registerListeners#43431

Closed
Fedik wants to merge 7 commits intojoomla:5.2-devfrom
Fedik:plugin-configurable-subscriber2
Closed

[5.2][RFC] CMSPlugin: ConfigurableSubscriberInterface as replacement for deprecated registerListeners#43431
Fedik wants to merge 7 commits intojoomla:5.2-devfrom
Fedik:plugin-configurable-subscriber2

Conversation

@Fedik
Copy link
Member

@Fedik Fedik commented May 6, 2024

Summary of Changes

This is alternative to

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 😄
Thanks.

@Fedik
Copy link
Member Author

Fedik commented May 6, 2024

Alternative

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

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

Instead of

public static function getSubscribedEvents(): array
{
return self::$enabled ? [
'onAjaxGuidedtours' => 'startTour',
'onBeforeCompileHead' => 'onBeforeCompileHead',
] : [];
}

The alternative allows to do:

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

@HLeithner
Copy link
Member

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.

@Fedik Fedik marked this pull request as draft May 12, 2024 09:06
@Fedik Fedik changed the title [5.2][RFC] CMSPlugin: deprecation for registerListeners and add new ConfigurableSubscriberInterface [5.2][RFC] CMSPlugin: ConfigurableSubscriberInterface as replacement for deprecated registerListeners May 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature RFC Request for Comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments