Skip to content

[5.4] CMSPlugin: deprecation for registerListeners#43395

Merged
HLeithner merged 23 commits intojoomla:5.4-devfrom
Fedik:plugin-deprecation-registerListeners
May 17, 2025
Merged

[5.4] CMSPlugin: deprecation for registerListeners#43395
HLeithner merged 23 commits intojoomla:5.4-devfrom
Fedik:plugin-deprecation-registerListeners

Conversation

@Fedik
Copy link
Member

@Fedik Fedik commented Apr 28, 2024

Pull Request for Issue # .

Summary of Changes

The PR deprecate CMSPlugin::registerListeners() as no longer needed when plugin will implement SubscriberInterface
(I wonder why it was not marked as deprecated before).

Testing Instructions

Apply PR, and navigate around the site, all should work as before.

The PR requires review from maintainers.

Link to documentations

Please select:

Ping @HLeithner @wilsonge

@HLeithner
Copy link
Member

make sense for me

@laoneo
Copy link
Member

laoneo commented Apr 28, 2024

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

@wilsonge

This comment was marked as outdated.

@Fedik

This comment was marked as outdated.

@SharkyKZ

This comment was marked as outdated.

@Fedik

This comment was marked as outdated.

@SharkyKZ

This comment was marked as outdated.

@Fedik

This comment was marked as outdated.

@SharkyKZ

This comment was marked as outdated.

@Fedik

This comment was marked as outdated.

@Fedik

This comment was marked as outdated.

@Fedik Fedik closed this May 6, 2024
@Fedik Fedik reopened this May 12, 2024
@Fedik Fedik changed the title [5.2] CMSPlugin: deprecation for registerListeners and dispatcher dependency [5.2] CMSPlugin: deprecation for registerListeners May 12, 2024
@Fedik

This comment was marked as outdated.

@Fedik
Copy link
Member Author

Fedik commented May 4, 2025

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 [6.1] CMSPlugin: use Lazy Object feature #45062 but for all plugins)
  • in 7.0 we remove traits

@HLeithner
Copy link
Member

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.

@Fedik
Copy link
Member Author

Fedik commented May 4, 2025

Only call the registerListeners() if SubscriberInterface is not set

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

@HLeithner
Copy link
Member

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

@Fedik
Copy link
Member Author

Fedik commented May 4, 2025

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?

@HLeithner
Copy link
Member

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,

@laoneo
Copy link
Member

laoneo commented May 5, 2025

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.

@laoneo
Copy link
Member

laoneo commented May 5, 2025

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.

@Fedik Fedik changed the base branch from 5.3-dev to 5.4-dev May 15, 2025 09:41
@Fedik Fedik changed the title [5.3] CMSPlugin: deprecation for registerListeners [5.4] CMSPlugin: deprecation for registerListeners May 15, 2025
@Fedik
Copy link
Member Author

Fedik commented May 15, 2025

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

@laoneo laoneo removed the PR-5.3-dev label May 15, 2025
@laoneo
Copy link
Member

laoneo commented May 15, 2025

I have tested this item ✅ successfully on bff97f4

Deprecation messages do look ok


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43395.

@richard67
Copy link
Member

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?

HLeithner and others added 2 commits May 17, 2025 11:58
Co-authored-by: Richard Fath <richard67@users.noreply.github.com>
@richard67
Copy link
Member

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.

@richard67
Copy link
Member

I have tested this item ✅ successfully on bb9a233


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43395.

@richard67
Copy link
Member

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43395.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label May 17, 2025
@richard67 richard67 added this to the Joomla! 5.4.0 milestone May 17, 2025
@HLeithner HLeithner enabled auto-merge (squash) May 17, 2025 11:57
@HLeithner HLeithner merged commit 8f6a981 into joomla:5.4-dev May 17, 2025
4 checks passed
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label May 17, 2025
@HLeithner
Copy link
Member

thanks all for participation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.