Allow module services to specify hooks

@dpi I absolutely support the hux direction:
- Discovery per class/service or per directory with method attributes.
- Keeping the parameter signature of the hook.

It feels very close and natural to our current procedural hooks.
At the same time, hux solves many of the shortcomings:

• It gets rid of procedural code.
• It allows dependency injection.
• It allows more than one implementation per module.
• It allows one method to implement multiple hooks. (e.g. hook_node_insert() + hook_node_update() in one method).
• It allows to control the order of implementations, without having to use hook_module_implements_alter().

As for symfony events, I think for our purpose they are a distraction.
For a new use case we can always consider to introduce an event instead of a hook, or even something else.
But if we already have a hook, and want to keep supporting it, then method + attribute is the best way to go.

I think this would have been a harder sell before attributes were introduced, we would have had to use annotations.

Given previous comment, and a variety of external discussions, I'd like to propose closing this issue in favor of working towards a [yet to be created] issue for utilising classes + method attributes in a same-or-similar manner to Hux → .

At its core, Hux is transparently registering services to the container, much like the proposed resolution of utilising services for this issue. Though there is not necessarily a requirement to define a service definition or dependencies explicitly with a YAML file. Thanks to @larowlan, a class autodiscovery implementation has turned out to be great.

I think it would be absolutely fine to open a new issue and then close this one as a duplicate, there's not really a competing proposal on here or anything, just a very old discussion and for me the hux API (not that I've used it yet, but based on what I've seen so far) covers all the original requirements that made us want to go in this general direction.

For what it is worth I have used both hux → and hook_event_dispatcher → in production projects.

I think both have their merits but hux is a more natural replacement for many of the current hooks because it does away with the overhead of the event object and instead the things one wants to interact with are directly passed as arguments to the method.
But from a DX perspective it would have to be much more clear of when to use which system, since there are subtle differences between events and hooks. Good documentation of which is more appropriate would be a benefit because now one essentially wants to use events just because they are OOP and can be used in services, but with hux all of a sudden the main selling point is gone and the argument for events becomes "be more like symfony". But if we would follow the suggestion by catch from #139 maybe events are for when modules interact directly with symfony and hux is for interacting with drupal components like entities etc.

@bircher

But from a DX perspective it would have to be much more clear of when to use which system, since there are subtle differences between events and hooks

I assume we are only talking about new cases where we would introduce either a new hook or a new event.
I see hux mostly as a tool for implementing existing hooks, without any additional effort on the party that publishes the hook. It is not really a "replacement", it just provides more options on how to implement them.

On the other hand, converting hooks into events requires extra effort on the party that publishes the hook/event. This is obvious from looking at the "Hook Event Dispatcher" module, which supports events only for _some_ hooks, with dedicated code for each of them, see https://git.drupalcode.org/project/hook_event_dispatcher/-/tree/3.x/modules.

The question of hooks vs events for new use cases already exists today.
In fact there are more options, such as tagged services with service collectors.

I think it is a valid question, but I think we would need to gather experience before we can recommend anything.
To implement an existing hook, I would always choose hux (or the equivalent that we would add in core).
To publish a new hook or event, I would try different options and see what works best.

I don't think this question should be a blocker for doing something like hux in core.

Responding to #146 and #147 I think the main question is whether, after core hooks are hux-ified, we would want to convert some core events (config and similar) to hooks or not. But as @donquixote that's not a blocker to the initial huxification of hooks but something that can be decided afterwards.

The main thing to decide is that we definitely want 'OOP hooks' instead of 'convert every hook to an event' and I am +1000 to OOP hooks and -1000 to 'convert every hook to an event' just in case it wasn't already clear ;)

Converting to hux-like implementations seems nice and easy, and it's not too different from other things done in core. You create a service class, implement some functions, and you're good to go. I'm on board.

@donquixote My understanding is that Hook Event Dispatcher now supports or is in the process of implementing a dynamic system that doesn't require manually implementing every procedural hook, as of Drupal core 9.4 and Hook Event Dispatcher 3.2.0 → .

@Ambient.Impact
I looked at that patch, and I still see the distinct event classes for different hooks.
I don't even know how you would ever get rid of these event classes.

Perhaps you could auto-generate them based on the hook signature, but this would be quite adventurous.
And in the end you need these classes as parameter types in the event subscribers. And with code generation, at the time you write the subscriber, the to-be-generated event class might not be generated yet, making it hard for the IDE to help you out.

@donquixote That issue just laid the groundwork from what I can tell, but that was a year ago and it's had a bunch of work since. Worth poking around the 4.x tree to see where they're at because it looks like they've got a bunch of stuff to generate events and also a plug-in factory, which I suppose could work better in some cases. This isn't necessarily a full endorsement of the event approach, since I agree it tends to have a lot more boilerplate needed and Hux looks more pleasant to get started with, but I've written so many event subscribers at this point that I can do in my sleep.

Just to be clear, I am a huge fan of hux. I think it is much better than the proposal in the issue summary (which of course predates possibilities with current php versions) and addresses all the concerns listed in comments.

Hux may still need to be profiled, but given that it replaces what is described in #36 and others I think it will probably be about the same as normal hooks when they call a method on a service.

Of course it is not a blocker to document the subtle differences between events and this hook2.0 system. And we may decide to move some hooks to events still or some events to hooks2.0 but how this is decided should be in its own documentation page.

I think the fact that hooks stayed around and have not all been replaced by events in a decade is probably a sign and it is not just due to the lack of funding. Hux on the other hand has a 1to1 upgrade path so it seems like a much easier transition.

Here we go, a new issue!
🌱 [META] Hooks via attributes on service methods (hux style) Active

The Events system requires use of an Event object rather than named parameters, this creates additional boilerplate when defining events and additional levels of indirection when subscribing to events.

It's possible to use payload as an event object.

$node = Node::load(1);
$event_dispatcher->dispatch($node, 'entity_delete');

Multiple hook parameters can be wrapped into some generic event object. It could be even stdClass.

$data = [
  'build' => $build,
  'entity' => $entity,
  'display' => $display,
  'view_mode' => $view_mode,
];
$event_dispatcher->dispatch((object) $data, 'entity_view');

'Payload as event object' has been possible since Symfony 4.3, which is after the issue summary was written. We should definitely compare Symfony 2023 with hux though instead of hux with Symfony as it was in 2014.. The old method signature was previously type hinted with Event.

The big remaining issue, which isn't in the issue summary but is discussed elsewhere, is that $event_dispatcher->dispatch($node, 'entity_delete'); wouldn't give us the dynamic hooks that we currently have (hook_entity_delete(), hook_node_delete()),
and similar ones like hook_form_FORM_ID_alter() etc.) so we'd still be looking at one-to-one conversions both for dispatching and implementation. This becomes even more so with the single $event argument - being able to send stdClass as essentially an array of arguments would mean less conversions for boilerplate, but this would still need to be changed for every hook.

We'd also still have things like hook ordering vs. priorities to sort out too although hoping whatever we do can include some improvements over hook_module_implements_alter().

The big remaining issue, which isn't in the issue summary but is discussed elsewhere, is that $event_dispatcher->dispatch($node, 'entity_delete'); wouldn't give us the dynamic hooks that we currently have (hook_entity_delete(), hook_node_delete()),

One possible approach is dispatching two events. A module interesting in node events could subscribe only to endity_delete.node event.

$event_dispatcher->dispatch($entity, 'entity_delete');
$event_dispatcher->dispatch($entity, 'entity_delete.' . $entity->getEntityTypeId());

I think the arguments provided by @Chi in favor of events are very compelling.

For the priorities problem we have 📌 Add before/after ordering to events Needs work , which seems doable at first sight.

As for the need to have an Event class (which is not a strict requirement anymore per #155), tbh I think that would be an improvement over our current way of documenting hooks and their signature in a module.api.php file.

$event_dispatcher->dispatch($entity, 'entity_delete');
$event_dispatcher->dispatch($entity, 'entity_delete.' . $entity->getEntityTypeId());

This is very similar to what we do in EntityStorageBase already:

  $this->moduleHandler()->invokeAll($this->entityTypeId . '_' . $hook, [$entity]);
    // Invoke the respective entity-level hook.
    $this->moduleHandler()->invokeAll('entity_' . $hook, [$entity]);

So combined with being able to use a payload rather than an Event object it'd be a lot closer to what we have now for at least single-argument event listeners - i.e. the listeners would be able to type hint on EntityInterface/NodeInterface etc. as appropriate.

However if we do that, then there's no Event class to document the event (dynamic name or not. What we currently do with the Events class constants has had issues in the past too ( 📌 [policy, no patch] Decide on policy regarding use of Events class with string constants for event names or class names instead Active isn't the issue I was thinking of but it's the issue I found).

@Chi @amateescu
I am sure a lot of the current hooks could be converted to events, but, assuming it is done as I imagine:

• It has to be done separately for each hook, including for contrib.
• It will cause BC break, if existing hook implementations no longer work.

Is this how you imagine it to be done?
Or do you propose a generic way to dispatch events directly from ModuleHandler->invokeAll(), with old implementations still working?

One main benefit of Hux is that it naturally integrates into the existing system, will immediately work for all hooks, and minimizes the BC impact.
To me, it is a huge benefit to be able to use contrib modules with multiple major versions of Drupal core, or to upgrade to a new core version with minimal effort.

And yes, the problems mentioned by @catch also still apply.

#155 is the first comment since about 2012 that made me seriously consider that events could be a viable alternative to hooks (compared to the original idea in this issue or hux), so trying to think through a bit more.

A big advantage of hux still is bc - because we'd still be dispatching hooks, the dispatching API doesn't change (or not unless we want it to) and we have central control over handling all the other additions and deprecations.

If we switch to events we lose at least some of that because eventually every hook invocation needs to eventually dispatch an event instead.

However maybe there's a bit of automation that's possible, although we'd need to deprecate different things at different times.

public function dispatchEventAndInvokeAllDeprecated(object $event, $event_name = NULL, $hook, $args = []) {}

This would:

1. Build a deprecation message with the event and hook names.
2. Call invokeAllDeprecated().
3. Dispatch the event
4. Merge the results

We could then (eventually, once all of core is converted) deprecate ::invokeAll() and point people to ::dispatchEventAndinvokeAllDeprecated().

Then at some point in the next major release, we'd also deprecate ::dispatchEventAndInvokeAllDeprecated() and tell people to use event dispatcher directly. Although we'd probably also need an event-return-merging helper somewhere, not sure how many hooks still rely on that, but at least hook_views_data() probably does.

Another remaining concern here is that are hooks that we don't want to migrate to events but to something else entirely, like hook_mail() to mail templates in 🌱 [META] Adopt the symfony mailer component Needs review . It would be annoying to go to all of the trouble of switching hook_mail to a MailEvent class and then having to deprecate the MailEvent class. But this is more an issue of ordering/timing - it might mean we have to do all the additions and deprecations on quite a long timeline compared to usual.

I don't think you have to worry about old hooks as you could provide one event subscriber in core that subscribes to the new events but calls the old hooks. Then the whole event propagation still at some point gets influenced by the old hooks' results. When Drupal 11 is about to be released, we simply remove the event subscriber that polyfills hook implementations.

@catch This would mean that the calling code needs to be changed 2x.
I would prefer if you can just call ->dispatch() and it does all the rest under the hook, without the calling code having to be updated again in the future.

We are still talking in slack atm, so I don't want to go into too much detail.

But one thing:
Even though I still support the Hux direction, we should explore the event direction in more depth, and see how far we get, possibly with a technical POC.
Some questions to be addressed:

• How does the entire system look like in Drupal 12, after any kind of transition period?
• How can we do a smooth transition, and avoid too much BC breakage?
• How would we convert multiple-argument hooks?

In the end, we can compare and choose between different options. E.g.:

1. Discard events, go for "Hux in core" all the way.
2. Discard hooks and hux, go for events all the way.
3. Use Hux for hooks that don't easily convert to events, and as a transition solution. Then convert specific hooks to events one by one. at some point re-evaluate if we convert everything to events or keep the Hux system.

Additional notes from slack discusson in #contribute with @donquixote and @amateescu

1. Add EventDispatcher::dispatchVariadic($event_name, ...$args) - this would be a small addition to the event dispatcher, but it would allow for example a hook_form_alter() conversion with no signature changes for the listener method (and for any contrib hooks where they don't want to use an event class). People could still introduce an event class if they want to for specific hooks.

2. In EventDispatch::dispatch() look for a special event class/interface with ::getHookName() so we can dispatch the old hook. For payload hooks use an event_hook_mapping.yaml (or similar) and then find the hook to dispatch from that.

3.Possible approach for docs to replace *.api.php - this would equally work for hux or events, you'd be able to actually use the trait if wanted, but @group etc. makes it discoverable regardless.

Trait EntityLoadLoadTrait {
  /**
   * Useful docs with @group and etc.
   /*
  [#Hook node.load]
  public function entityLoad(EntityInterface $entity) {
    $this->doNodeLoad();
  }

  abstract protected function doEntityLoad(EntityInterface $entity) {
  }
}

use EntityLoadTrait

protected function doEntityload(EntityInterface $entity) {
  // stuff.
}

Discard events, go for "Hux in core" all the way.

This isn't going to be 100% possible due to kernel events and similar dispatched by Symfony. Unless we wrote some kind of event -> hux adapter, but then we can't stop people registering event listeners in services.yml and we'd not be able to drop event dispatcher as a dependency.

Discard events, go for "Hux in core" all the way.

This isn't going to be 100% possible due to kernel events and similar dispatched by Symfony.

Sorry, I did not express myself well.
What I meant is drop the idea that we want to convert all hooks to events, and make peace with most hooks remaining hooks.
I updated my comment.

I created a comparison table as a spreadsheet.
It is open for editing.
(To reduce the risk for it to be vandalized by bots, I scramble the url a bit.)
ht tps://doc s.go ogle.c om/spr eadsheets/d/1581ZT4ezs JLFAZIH7EW2n4NDyfQFENVEcApNxlYYOC4/edit#gid=0

I scramble the url a bit

I could not decrypt the URL.

@Chi, all you have to do is remove the spaces/newline.

Yep, missed the new line.

There are a few hooks that available for themes to implement. HOOK_form_alter, HOOK_theme_suggestions_alter, etc. If those are transformed to events, how will themes implement them? Does it mean that themes should be able to register services?

There are a few hooks that available for themes to implement. HOOK_form_alter, HOOK_theme_suggestions_alter, etc. If those are transformed to events, how will themes implement them? Does it mean that themes should be able to register services?

Good question!
This applies both to hux-ified hooks, and to event-ified hooks.

First thing to note is that technically a call to $theme_manager->alter() is completely separate from $module_handler->alter().
Calling code has to call both separately.
We could convert module hooks without converting theme hooks. Or we do theme hooks as a follow-up.

But let's say we do want themes to register services.

(Just a reminder for people who follow this and forgot how themes work:
Of all the themes available on a website, any number of them can be installed (in D7 we would say "enabled") at a time. The list of installed themes applies site-wide and only changes when configuration (core.extension.theme) changes.
Only one of the installed themes can be active at a time, and this can be determined dynamically, typically based on the page you are visiting.)

If we allow themes to register services, it has to be for all themes that are installed, no matter which is the active theme on a specific page.
Otherwise we would need different containers on different pages, which would not really work.

On the other hand, theme hooks should only be invoked for the active theme.
So the theme manager has to cache hook implementation methods per theme, not globally.
Or for events, there would have to be a special layer to the dispatcher that selects the active theme.

(just for the record, I still favor the hux direction)

Postponing this on 📌 OOP hooks using event dispatcher Needs review which is where the current implementation activity is happening.

📌 OOP hooks using event dispatcher Needs review is going to land hopefully imminently. Closing this as duplicate even though it's the (much) older issue, and moving credit over. What we ended up with over there is pretty similar to what we came up with here.