Support #[AsEventListener] attribute on classes and methods

I took a look at this issue and gave an attempt to try to work with what we have, by supplementing ContainerAwareEventDispatcher with changes, and keeping our existing tag.

But I realize that we also need to add a compiler pass to reflect on tagged services.

So I agree that It might just be more straightforward to switch from \Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher to \Symfony\Component\EventDispatcher\EventDispatcher and utilize Symfony's\Symfony\Component\EventDispatcher\DependencyInjection\RegisterListenersPass here.

Since the associated MR came to the same conclusion, that swapping to Symfonys EventDispatcher is necessary, I think we should postpone this issue on our main EventDispatcher issue @ 📌 Replace ContainerAwareEventDispatcher with Symfony EventDispatcher Fixed

It'd be valuable to also rely on ✨ Directory based automatic service creation Needs review to do automatic service creation for us.

📌 Replace ContainerAwareEventDispatcher with Symfony EventDispatcher Fixed was merged. Changing status per the most recent comments. Please update to a different status as needed.

Rebased MR 4438. One thing that should be noted: using the AsEventListener attribute without the event property only works if the dispatched event name matches the event object FQCN, or if an alias mapping between the event name and the FQCN is set in the event_dispatcher.event_aliases container parameter. These examples (from Symfony docs) only work if the the dispatched event name is CustomEvent::class, or if the alias is set:

namespace App\EventListener;

use Symfony\Component\EventDispatcher\Attribute\AsEventListener;

#[AsEventListener]
final class MyListener
{
    public function __invoke(CustomEvent $event): void
    {
        // ...
    }
}

namespace App\EventListener;

use Symfony\Component\EventDispatcher\Attribute\AsEventListener;

final class MyMultiListener
{
    #[AsEventListener]
    public function onCustomEvent(CustomEvent $event): void
    {
        // ...
    }
}

For convenience, I've add a commit to the MR to register all the Symfony kernel event aliases. We can probably add follow up tasks to convert core event subscribers to use the AsEventListener attribute, as well as add aliases for other core events.

Added a CR: Event listeners can be defined with the #[AsEventListener] attribute → .

Is there least 1 listener that can be converted to show this change works?

Is there least 1 listener that can be converted to show this change works?

I mentioned converting core subscribers in a follow up, but I had a thought of whether there are benefits to doing it? I think having this feature is nice for DX going forward, but I don't know if there's an advantage to convert existing core code. I'll look into finding one to convert regardless, and the decision can be made from there.

OK, converted MaintenanceModeSubscriber. One thing I ran into is that AutowireTest was exempting MaintenanceModeSubscriber from needing a service alias declared, because it was one of many services implementing EventSubscriberInterface. However, with EventSubscriberInterface removed, needed to add an alias for MaintenanceModeSubscriber. I'm not sure that adding aliases for event subscriber services is worth it, but gonna push this forward for review.

Awesome to see the replacement worked though!

I reviewed the code and the draft CR. LGTM. Nice work.

Actually, I am wondering why we have both:

1. A service is declared in core.services.yml for maintenance_mode_subscriber
2. Various #[EventListener] appear on various methods on the same file. Wouldn't the nicer usage be to omit the yml declared service and rely on Attribute based creation of Listeners? The MR suggests that all we are saving is the getSubscribedEvents() method. I had higher hopes than that :). Maybe we need more plumbing elsewhere in core for what I describe.

Automatic discovery of services without entry in a services.yml needs something like ✨ Directory based automatic service creation Needs review . There's no discovery of services in this MR, and it's probably out of scope for this issue.

Since there are multiple handlers in MaintenanceModeSubscriber, either each method needs to have an attribute as done in the MR, or there needs to be multiple AsEventListener attributes on the class, each including the method property.

Thanks. Once we have ✨ Directory based automatic service creation Needs review , we could add the RegisterListenersPass compiler pass and then get #[AsListener] classes registered as services and listeners. I'm not sure the intermediate step in this MR provides much value. If others think so, feel free to mark this RTBC.

The RegisterListenersPass already occurs within Drupal's Drupal\Core\DependencyInjection\Compiler\RegisterEventSubscribersPass

That pass doesn't handle the #[AsEventListener] attribute, though. That is what this MR introduces, and the code to do so is copied from here: https://github.com/symfony/symfony/blob/9d89e05279631430f9037844fc4cc2ea...

So even if or when ✨ Directory based automatic service creation Needs review gets in, the work here still needs to be done to support #[AsEventListener].

Also see: https://stackoverflow.com/a/78338989

Yes, I think this is still useful as it reduces boilerplate and makes the definition of listeners simpler. Going to set back to RTBC.

   maintenance_mode_subscriber:
     class: Drupal\Core\EventSubscriber\MaintenanceModeSubscriber
     arguments: ['@maintenance_mode', '@config.factory', '@string_translation', '@url_generator', '@current_user', '@bare_html_page_renderer', '@messenger', '@event_dispatcher']
+  Drupal\Core\EventSubscriber\MaintenanceModeSubscriber: '@maintenance_mode_subscriber'

While this is annoying I think in the long run event subscribers should just be named after their class, ie.

   Drupal\Core\EventSubscriber\MaintenanceModeSubscriber:
     arguments: ['@maintenance_mode', '@config.factory', '@string_translation', '@url_generator', '@current_user', '@bare_html_page_renderer', '@messenger', '@event_dispatcher']

Service names come with a BC promise (although I'm not sure how useful that is for event subscribers) so to make this switch we will have to alias the old name and deprecate it, which is out of scope of this issue, but worth considering if we decide to swap all services to use this attribute in a followup.

Also I assume if we do ✨ Directory based automatic service creation Needs review this will have to happen anyway, because the definition will be gone from services.yml.

Alternatively we do this in AutowireTest:

      // Ignore proxy classes for autowiring purposes.
      if (str_contains($class, '\\ProxyClass\\')) {
        continue;
      }

We could also ignore EventSubscriber classes the same way?

Added commit per #23 to ignore classes with '\\EventSubscriber\\' in the path.

Appears to have MR conflicts. Surprised the bot didn't catch it

Rebased.

Sweet thanks for the quick turn around. Since was previously RTBC and previous feedback appears to be addressed restoring that.

It occurs to me that since 📌 OOP hooks using attributes and event dispatcher Needs review introduced a container compiler pass that essentially iterates through module files to find classes or methods with the Drupal\Core\Hook\Attribute\Hook attribute, and turns those into event listener services, if we:

1. Made the Hook attribute a subclass of the Symfony\Component\EventDispatcher\Attribute\AsEventListener attribute class
2. Changed the compiler pass to look for the AsEventListener attribute,

we could have those general event listener services discovered in the same pass. And those event listeners wouldn't need to be declared in services.yml files.

This wouldn't work for core event subscribers outside of modules, but it might be worth looking into?

Created a new draft MR 10119 for a PoC of the approach I mentioned in #28 📌 Support #[AsEventListener] attribute on classes and methods Active . It leverages a lot of what HookCollectorPass is already doing to also find module class with the AsEventListener attribute and registers them as event listener services. No entry in the module .services.yml is required. Note that discovery is limited to the EventSubscriber directory in a module. Also, these services are autowired, so any arguments in class constructors need to be able to be autowired or have the Autowire attribute applied to them.

Putting back to NR for feedback on the PoC MR. I can also move this issue back to RTBC if this new work is better done in a followup.

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.