- Issue created by @berdir
- π©πͺGermany geek-merlin Freiburg, Germany
Yes, this would help a lot. Also, the restriction to the
Hooks
namespace is problematic even in modules:I implemented hundreds of hooks in the hux POC, where there was the choice to use either the Hooks namespace with auto-service registration, or ANY service. In >90% of the cases, from a code organization perspective, the hook was best to live where it belonged semantically, not in the Hooks namespace.
Or stated it differently: For sane code organization, restrikting hooks to Drupal\my_module\Hooks is the same PITA as to have to put them in the module file.
- πͺπΈSpain rodrigoaguilera Barcelona
While migrating a module to OOP hooks I noticed that the restriction of "one implementation per module" is still there. Now is rather silent as there is no error about it. For example having two classes in the \Drupal\my_module\Hook namespace that have the same hook annotations.
I would be really convenient to have multiple implementations per module to organize related features into the same class. E.g. a module that has two field widget third party settings and each one has its hook_field_widget_third_party_settings_form with a corresponding hook_field_widget_complete_form_alter. One implementation for each of the third party setting separated in different classes.
Is this something that can be tackled in this issue or should I open a new one?
- πΊπΈUnited States nicxvan
Yeah we need to update that cr. The one per module restriction was added back when we had to fix hook module implements alter.
- πΊπΈUnited States nicxvan
However you can still mark one method with more than one hook.
- πͺπΈSpain rodrigoaguilera Barcelona
Can you link to the issue? Just want to investigate if there is a possibility to be added back.
I updated the CR to clarify the point about the multiple implementations
https://www.drupal.org/node/3442349 β - πΊπΈUnited States nicxvan
https://www.drupal.org/project/drupal/issues/3484754 π Ordering of alter "extra hooks" is a gigantic mess Active
- πͺπΈSpain rodrigoaguilera Barcelona
Thanks for the link.
The project I migrated is https://git.drupalcode.org/project/change_labels/-/tree/1.x?ref_type=heads
but is already frankensteined with traits to workaround this issue that I'm talking about.To showcase the problem I created a patch for automated_cron with similar hook_field_widget_third_party_settings_form implementations.
Only the "second" implementation end up displaying its form in the widget settings.Maybe this is a special case since the calling module is important to later determine where to save the third party setting.
Moving one of the classes to another module (and changing the namespace) makes both form items to appear.
@rodrigoaguilera My understanding is that having multiple implementations of the same hook in a module makes re-ordering hooks via
hook_module_implements_alter()
convoluted, so there will be only 1 hook per module allowed until the whole procedural hook BC layer is removed in the future.As for this issue: I think the need can be addressed by something similar to WIP MRs in π Support #[AsEventListener] attribute on classes and methods Active . If we do the following, then registered services that have the #[Hook] and #[AsEventListener] attributes should get registered correctly as event listeners.
- Make the
Hook
attribute a subclass ofAsEventListener
- Add something like this:
$container->registerAttributeForAutoconfiguration(AsEventListener::class, static function ( ChildDefinition $definition, AsEventListener $attribute, \ReflectionClass|\ReflectionMethod $reflector, ) { $tagAttributes = get_object_vars($attribute); if ($reflector instanceof \ReflectionMethod) { if (isset($tagAttributes['method'])) { throw new LogicException(sprintf('AsEventListener attribute cannot declare a method on "%s::%s()".', $reflector->class, $reflector->name)); } $tagAttributes['method'] = $reflector->getName(); } $definition->addTag('kernel.event_listener', $tagAttributes); });
There is the risk that someone registers a Hook service in their .services.yml, and the class for the service is in the Hook namespace, that the listener would get added twice and fire twice per event, but it can be documented that services in the Hook namespace should not be added to services.yml, and services outside that namespace should.
- Make the
- π¨π¦Canada Charlie ChX Negyesi πCanada
The hmia patch did not remove the multiple implementations feature, at least not intentionally, if it's missing that's a bug, it is collecting into
$this->implementations[$hook->hook][$module][$class][] = $hook->method;
and themoduleImplementsAlters
property is only used to determine module order. By all means it should work. I would recommend running\Drupal::getContainer()->getParameter('hook_implementations_map')['field_widget_third_party_settings_form']
fromdrush php
and checking what's in there. If you see both of your classes then the problem is in eitherModuleHandler
or the closure used byEditFormDisplayEditForm::thirdPartySettingsForm
(it's using closure $hook, that's one possible problem down). Truth to be told, I don't think either nicxvan or myself have testedinvokeAllWith
by itself which is what's used here but at the same timeinvokeAll
relies on it so the chances of it being broken is quite low. But, it's software, and even worse, it's software written by me, so it being broken is not as impossible as one might desire.As for supporting the hook attribute in any service, that's not a particularly hard problem, it's just a performance/memory nightmare as you'd need to reflect every class and method defining a service to see whether they have a #Hook attribute. Of course, it's not my call but if it were then I'd split Rodrigo's problem into a separate issue and postpone this one on π Add a core FileCache implementation that is database-backed Active . Just a suggestion.
it's just a performance/memory nightmare as you'd need to reflect every class and method defining a service to see whether they have a #Hook attribute
Yeah, forgot to mention that the MRs for π Support #[AsEventListener] attribute on classes and methods Active need to be rebased, but I was holding off on that until π Add a filecache-backed attribute parser Active goes in.
- πͺπΈSpain rodrigoaguilera Barcelona
Apologies for the noise. Indeed multiple implementations are supported but there is special cases like hook_field_widget_third_party_settings_form that won't work the multiple implementations. Will file an issue for that.
I changed the CR to reflect that
https://www.drupal.org/node/3442349 β - π¨π¦Canada Charlie ChX Negyesi πCanada
Indeed let's move to a separate issue, the fix is as simple as
$settings_form[$module] = ($settings_form[$module] ?? []) + $hook(
that a trivial bug / missing feature it's not worth changing main CR over a small bug. Like, of all hooks only field_widget_third_party_settings_form/field_formatter_third_party_settings_form is affected and it won't be affected for long. - πͺπΈSpain rodrigoaguilera Barcelona
- π¨πSwitzerland berdir Switzerland
For example \Drupal\Core\Extension\ModuleHandler::invoke() does in fact only support a single hook for a given module, I assumed that's true in general, and based on that adjusted ultimate_cron in π Fix 11.1 test fails Active to still check for hooks and invoke them through the module handler. Might need to change that then, but the nice thing about this approach is that it doesn't care about where that hook lives exactly, same config works in D10 and D11.1+.
Back to the topic.
> As for supporting the hook attribute in any service, that's not a particularly hard problem, it's just a performance/memory nightmare as you'd need to reflect every class and method defining a service to see whether they have a #Hook attribute
Two ideas to make that less of a nightmare.
a) What if we'd make the discovery/reflection opt in? We do it for everything in the Hooks namespace, but if you explicitly add a tag to your service, we scan that too? Means autoconfig of services won't work.
b) We keep the Hook namespace restriction, but expand it to components. That only helps core (for now?), but it's similar to plugin discovery, where we support that too (various plugins are for example in (Drupal\Core\Entity\Plugin). So if we want to move the cache component part of system_cron() to the Cache component, we'd add a Drupal\Core\Cache\Hooks\CacheHooks class? - π¨π¦Canada Charlie ChX Negyesi πCanada
Hrmmmm, I will update the CR to talk about
::invoke
the original IS did it just didn't make it into the CR -- in short, those are an anomaly too, those are not really hooks :) and luckily there are very few of them.To be on topic. Sure, adding a service tag to indicate it's a hook and then adding those classes into
HookCollectorPass
is certainly a possibility. Let's think aloud what that would take, first it needs an optional$container
passed tocollectAllHookImplementations
(optional becauseModuleHandler::add
calls it). Once that's there code should read something like:foreach ($container->findTaggedServiceIds('drupal_hook') as $service_id => $tag) { $container->getDefinition($service_id)->getClass() foreach (static::getHookAttributesInClass($class) as $attribute) { $this->addFromAttribute($attribute, $class, $module); } }
That's nice -- but there's no
$module
. That information just doesn't exist and while for service YAMLs it could be persisted with a little effort --DrupalKernel
does index$this->serviceYamls
by$module
even if it doesn't pass it on -- services created by providers do not have this information.I am guessing you could make
$module
optional inaddFromAttribute
and grab the module from the class name:if ($hook->module) { $module = $hook->module; } elseif (!$module) { $module = explode('\\', $class)[1]; } if (!$module) { throw new WhateverException; }
this will work for Drupal modules and when it doesn't, well, just specify
$module
in#[Hook]
. Most of the time it is irrelevant anyways --invokeAllWith
needs it because it calls$callback($listener, $module);
where$module
is cheerfully ignored ininvokeAll
and indeed in almost all code usinginvokeAllWith
but still, that signature can't be changed without serious upheaval and so something needs to be supplied there. - π©πͺGermany geek-merlin Freiburg, Germany
@ghost of drupal past #11, @berdir #16, et al
it's just a performance/memory nightmare as you'd need to reflect every class and method defining a service to see whether they have a #Hook attribute
Maybe you are not aware of
$container->registerAttributeForAutoconfiguration
?
See #10 on how to implement. The
$container->registerAttributeForAutoconfiguration
is directly copied from Symfony Framework bundle: https://github.com/symfony/symfony/blob/e128d76b3dfd5147fe1263a807e8ce83..., but it effectively just does the reflection looping that is the performance issue in question.- π©πͺGermany geek-merlin Freiburg, Germany
> but it effectively just does the reflection looping that is the performance issue in question.
Which means:
- That compiler pass is already in place, so no nightmare to fear.
- It's the central place to do the reflection, with no performance penelty for any additional consumer.So IF that is used, no reason for performance fear left.
Which means:
- That compiler pass is already in place, iterating over all services, and no nightmare happened.It only iterates with reflection if attributes are registered for autoconfiguration via
$container->registerAttributeForAutoconfiguration
. Right now there aren't any in Drupal core, so this reflection iteration does not occur.- π¨π¦Canada Charlie ChX Negyesi πCanada
Supporting magic naming instead of or in addition to tagged services is a good idea, thanks. But I don't think it merits a separate issue, indeed it should be discussed here:
- support tagged services
- support magic named classes
- support both
Implementations wise, this is once again a minor tweak to HookCollectorPass. In this case, the filter iterator needs a minor adjustment: the iterator should iterate all of src instead of src/Hook but only return php files if the subPath starts with Hook or the file name ends in Hooks. This is not a problem, much as it was trivial to support tagged services the same is true for magic named classes.
No Symfony magic can avoid the problem outlined -- catch however is solving it with caching. Indeed, the less Symfony used, all the better as we have just seen they are incapable of keeping even the most basic semver. Of course, that's just my two cents, of course someone will eventually refactor the entire HookCollectorPass to use registerAttributeForAutoconfiguration and then no one ever will be able to debug the ensuing problems -- I know, I read the entire code flow for the latter to evalute for the hook oop patch, the complexity is horrifying. But, at least, it'll use Symfony, yay! But I digress.
- π©πͺGermany geek-merlin Freiburg, Germany
> registerAttributeForAutoconfiguration is only used by the full Symfony framework, not the components as shipped and I am fairly sure it requires the Symfony Config component.
Hmm, i am using it i a quite big customer project with >300 contrib and custom modules to autoregister hux classes. No package added, except ~10 lines of custom code, and no extraordinary container build times.
- π¬π§United Kingdom alexpott πͺπΊπ
I created a duplicate of this (which I closed) - β¨ Allow any service to subscribe to hooks Active which lists some more reasons to do this:
We have a lovely new hooks system that integrates the module handler and the event dispatcher to invoke hooks. In doing this we had to move a hook. core_field_views_data to \Drupal\views\Hook\ViewsViewsHooks::fieldViewsData() even though what it does - provide entity reference field integration with views could be considered to be core's responsibility.
Another example could be kernel tests to enable hook testing whether the test and the test hook implementation live side-by-side - see π Make hook testing with kernel tests very simple Active
Also this could be used to make things like
/** * Implements hook_cron(). */ #[Hook('cron')] public function cron(): void { $this->workspaceManager->purgeDeletedWorkspacesBatch(); }
from \Drupal\workspaces\Hook\WorkspacesHooks simpler. \Drupal\workspaces\WorkspaceManager::purgeDeletedWorkspacesBatch could subscribe to the hook - although we'd have to ensure anything that decorates it is also subscribed...
So IF
$container->registerAttributeForAutoconfiguration
is used, i see no rational reason for performance fear left.So while core does not use
$container->registerAttributeForAutoconfiguration
, with
autoconfigure: true
and several interfaces registered for autoconfiguraiotn in
CoreServiceProvider::register()
with$container->registerForAutoconfiguration()
(noAttribute
), it does turn out that at compile time all core services are iterated over and reflected.Given this, @ghost of drupal past mentioned that all service classes are being loaded into memory at compile time anyway, so iterating with reflection on these classes to discover class-level `Hook` attributes has a negligible affect performance. @nicxvan also ran performance tests iterating reflection through each of a class's methods, and the hit on performance was not significant even with the number of methods in the class in 100,000s. Without an additional performance concern, it seems like a good idea to go forward.
- πΊπΈUnited States nicxvan
Just to add to it, here is the code we used for benchmarking:
$r = new \ReflectionClass($object); $time = microtime(TRUE); $n = count($r->getMethods()); print (microtime(TRUE) - $time) . \PHP_EOL; $time = microtime(TRUE); for ($i = 0; $i < $n; $i++) { new stdClass; } print (microtime(TRUE) - $time) . \PHP_EOL; print ($n) . \PHP_EOL . \PHP_EOL;
We ran it with different classes with different numbers of methods.
The numbers returned are the time to check the class, the time to create the new stdClass and the number of methods:0.24680495262146 0.058710098266602 1000000 0.015760898590088 0.0052649974822998 100000 0.0014050006866455 0.00056791305541992 10000 0.00011301040649414 7.2002410888672E-5 1000 1.5974044799805E-5 1.0013580322266E-5 100 2.8610229492188E-6 2.1457672119141E-6 10
Note that this is probably soft-blocked by π Hook ordering across OOP, procedural and with extra types Active , or at least there will likely be a giant merge conflict resolution needed between the two.
A couple thoughts on issues that might need accounting for, without confirming in code:
- If someone has registered a class in their module's Hooks folder as a service in their *.services.yml, whether by mistake or holdover from D10 compatibility, probably need to make sure that that class method is not called twice for the hook
- If someone alter's an existing service using a ServiceModifier or service decorator, then presumably the hook methods in the original class, and for decorated services, hook methods in every class except the outermost decorator, should not run. So only the outermost class's hook methods should be invoked
If there are complications with either scenario that aren't easy to resolve, then maybe perhaps to reduce scope it can be documented that hooks aren't supported for altered services in whatever use cases and punt those to be done later.
MR 12222 is up.
Notes:
- Any module service that is not abstract or synthetic can use Hook attributes to implement hooks
- This does not apply to services that (somehow?) do not have a class defined, services in the
Drupal\Core
andDrupal\Component
namespaces and non-Drupal classes - Kernel test classes can implement hooks. Use
register()
to register the kernel test class itself as service and any Hook attributes in the class will be picked up - For services provided and/or altered by a service provider/modifier, or decorated services:
- Hook implementations that exist in the new service class will be picked up
- For altered or decorated services, if the new service class extends the original service class, and the hook implementation method is not overridden, the hook method will run once
- For altered or decorated services, if the new service class extends the original service class, and the hook implementation method is overridden with no Hook attribute, the hook method will not be invoked
- For altered or decorated services, if the new service class does not extend the original service class, none of the original service class's hook implementations will be invoked.
- It is not possible to use a service modifier to alter
Hook\
class services that are not registered services. This is because, in order to support altered or decorated services, HookCollectorPass has been moved to run after the ModifyServiceDefinitionsPass, so the the Hook class definitions do not exist whenalter()
in a service modifier runs (see more below*). IfHook\
class behavior needs to be changed, using a service decorator is recommended
For reference, here's output of a representative local run of
drush si standard -y
-vvvv onHEAD
:[info] Starting bootstrap to root [0.15 sec, 2.91 MB] [info] Drush bootstrap phase 1 [0.15 sec, 2.91 MB] [info] Try to validate bootstrap phase 1 [0.15 sec, 2.91 MB] [info] Try to validate bootstrap phase 1 [0.15 sec, 2.92 MB] [info] Try to bootstrap at phase 1 [0.15 sec, 2.92 MB] [info] Drush bootstrap phase: bootstrapDrupalRoot() [0.15 sec, 2.92 MB] [info] Change working directory to /var/www/html [0.15 sec, 2.91 MB] [info] Initialized Drupal 11.3-dev root directory at /var/www/html [0.15 sec, 2.92 MB] [info] Drush bootstrap phase: bootstrapDrupalSite() [0.16 sec, 3.19 MB] [debug] Could not find a Drush config file at sites/default/drush.yml. [0.16 sec, 3.19 MB] [info] Initialized Drupal site drupal.ddev.site at sites/default [0.16 sec, 3.19 MB] [info] Drush bootstrap phase: bootstrapDrupalConfiguration() [0.16 sec, 3.19 MB] [info] Executing: command -v sqlite3 [0.16 sec, 3.41 MB] You are about to: * DROP all tables in your 'sites/default/files/.sqlite' database. // Do you want to continue?: yes. [info] Sites directory sites/default already exists - proceeding. [0.16 sec, 3.48 MB] [info] sql:query: .tables [0.16 sec, 3.48 MB] [info] Executing: sqlite3 sites/default/files/.sqlite < /tmp/drush_uaOpuf [0.16 sec, 3.49 MB] [info] sql:query: DROP TABLE block_content; DROP TABLE block_content__body; DROP TABLE block_content__field_body; DROP TABLE block_content__field_content_link; DROP TABLE block_content__field_copyright; DROP TABLE block_content__field_disclaimer; DROP TABLE block_content__field_media_image; DROP TABLE block_content__field_summary; DROP TABLE block_content__field_title; DROP TABLE block_content_field_data; DROP TABLE block_content_field_revision; DROP TABLE block_content_revision; DROP TABLE block_content_revision__body; DROP TABLE block_content_revision__field_body; DROP TABLE block_content_revision__field_content_link; DROP TABLE block_content_revision__field_copyright; DROP TABLE block_content_revision__field_disclaimer; DROP TABLE block_content_revision__field_media_image; DROP TABLE block_content_revision__field_summary; DROP TABLE block_content_revision__field_title; DROP TABLE cache_container; DROP TABLE config; DROP TABLE content_moderation_state; DROP TABLE content_moderation_state_field_data; DROP TABLE content_moderation_state_field_revision; DROP TABLE content_moderation_state_revision; DROP TABLE file_managed; DROP TABLE file_usage; DROP TABLE help_search_items; DROP TABLE history; DROP TABLE inline_block_usage; DROP TABLE key_value; DROP TABLE locale_file; DROP TABLE locales_location; DROP TABLE locales_source; DROP TABLE locales_target; DROP TABLE media; DROP TABLE media__field_media_audio_file; DROP TABLE media__field_media_document; DROP TABLE media__field_media_image; DROP TABLE media__field_media_oembed_video; DROP TABLE media__field_media_video_file; DROP TABLE media_field_data; DROP TABLE media_field_revision; DROP TABLE media_revision; DROP TABLE media_revision__field_media_audio_file; DROP TABLE media_revision__field_media_document; DROP TABLE media_revision__field_media_image; DROP TABLE media_revision__field_media_oembed_video; DROP TABLE media_revision__field_media_video_file; DROP TABLE menu_link_content; DROP TABLE menu_link_content_data; DROP TABLE menu_link_content_field_revision; DROP TABLE menu_link_content_revision; DROP TABLE menu_tree; DROP TABLE node; DROP TABLE node__body; DROP TABLE node__field_body; DROP TABLE node__field_cooking_time; DROP TABLE node__field_difficulty; DROP TABLE node__field_ingredients; DROP TABLE node__field_media_image; DROP TABLE node__field_number_of_servings; DROP TABLE node__field_preparation_time; DROP TABLE node__field_recipe_category; DROP TABLE node__field_recipe_instruction; DROP TABLE node__field_summary; DROP TABLE node__field_tags; DROP TABLE node__layout_builder__layout; DROP TABLE node_access; DROP TABLE node_field_data; DROP TABLE node_field_revision; DROP TABLE node_revision; DROP TABLE node_revision__body; DROP TABLE node_revision__field_body; DROP TABLE node_revision__field_cooking_time; DROP TABLE node_revision__field_difficulty; DROP TABLE node_revision__field_ingredients; DROP TABLE node_revision__field_media_image; DROP TABLE node_revision__field_number_of_servings; DROP TABLE node_revision__field_preparation_time; DROP TABLE node_revision__field_recipe_category; DROP TABLE node_revision__field_recipe_instruction; DROP TABLE node_revision__field_summary; DROP TABLE node_revision__field_tags; DROP TABLE node_revision__layout_builder__layout; DROP TABLE path_alias; DROP TABLE path_alias_revision; DROP TABLE router; DROP TABLE search_dataset; DROP TABLE search_index; DROP TABLE search_total; DROP TABLE sequences; DROP TABLE shortcut; DROP TABLE shortcut_field_data; DROP TABLE shortcut_set_users; DROP TABLE taxonomy_index; DROP TABLE taxonomy_term__parent; DROP TABLE taxonomy_term_data; DROP TABLE taxonomy_term_field_data; DROP TABLE taxonomy_term_field_revision; DROP TABLE taxonomy_term_revision; DROP TABLE taxonomy_term_revision__parent; DROP TABLE user__roles; DROP TABLE user__user_picture; DROP TABLE users; DROP TABLE users_data; DROP TABLE users_field_data; DROP TABLE watchdog; [0.17 sec, 3.51 MB] [info] Executing: sqlite3 sites/default/files/.sqlite < /tmp/drush_SYPKPZ [0.17 sec, 3.51 MB] [notice] Starting Drupal installation. This takes a while. [0.24 sec, 3.49 MB] [debug] Calling install_drupal(Composer\Autoload\ClassLoader, array, array) [0.24 sec, 3.49 MB] [notice] Performed install task: install_select_language [0.4 sec, 12.08 MB] [notice] Performed install task: install_select_profile [0.4 sec, 12.08 MB] [notice] Performed install task: install_load_profile [0.4 sec, 12.08 MB] [notice] Performed install task: install_verify_requirements [0.4 sec, 12.43 MB] [notice] Performed install task: install_verify_database_ready [0.4 sec, 12.43 MB] [info] sqlite module installed. [0.5 sec, 17.62 MB] [info] system module installed. [0.61 sec, 20.98 MB] [notice] Performed install task: install_base_system [0.61 sec, 21.01 MB] [notice] Performed install task: install_bootstrap_full [0.61 sec, 21.01 MB] [info] user module installed. [0.98 sec, 27.73 MB] [info] path_alias module installed. [0.98 sec, 27.73 MB] [info] big_pipe module installed. [0.98 sec, 27.73 MB] [info] config module installed. [0.98 sec, 27.73 MB] [info] help module installed. [0.98 sec, 27.53 MB] [info] page_cache module installed. [0.98 sec, 27.53 MB] [info] dynamic_page_cache module installed. [0.98 sec, 27.53 MB] [info] automated_cron module installed. [0.98 sec, 27.53 MB] [info] announcements_feed module installed. [0.98 sec, 27.53 MB] [info] block module installed. [0.98 sec, 27.53 MB] [info] filter module installed. [0.99 sec, 27.85 MB] [info] views module installed. [0.99 sec, 27.87 MB] [info] field module installed. [0.99 sec, 27.68 MB] [info] text module installed. [0.99 sec, 27.68 MB] [info] block_content module installed. [1 sec, 27.79 MB] [info] breakpoint module installed. [1 sec, 27.79 MB] [info] file module installed. [1 sec, 27.59 MB] [info] editor module installed. [1 sec, 27.59 MB] [info] ckeditor5 module installed. [1 sec, 27.59 MB] [info] image module installed. [1.01 sec, 27.82 MB] [info] media module installed. [1.31 sec, 34.62 MB] [info] language module installed. [1.32 sec, 34.67 MB] [info] locale module installed. [1.32 sec, 34.71 MB] [info] config_translation module installed. [1.32 sec, 34.71 MB] [info] contact module installed. [1.32 sec, 34.45 MB] [info] workflows module installed. [1.32 sec, 34.19 MB] [info] content_moderation module installed. [1.32 sec, 34.19 MB] [info] node module installed. [1.35 sec, 34.99 MB] [info] content_translation module installed. [1.36 sec, 35.01 MB] [info] contextual module installed. [1.36 sec, 34.75 MB] [info] datetime module installed. [1.36 sec, 34.75 MB] [info] taxonomy module installed. [1.37 sec, 34.92 MB] [info] dblog module installed. [1.37 sec, 34.65 MB] [info] layout_discovery module installed. [1.37 sec, 34.65 MB] [info] field_ui module installed. [1.37 sec, 34.65 MB] [info] history module installed. [1.37 sec, 34.65 MB] [info] layout_builder module installed. [1.37 sec, 34.65 MB] [info] link module installed. [1.37 sec, 34.65 MB] [info] media_library module installed. [1.52 sec, 42.65 MB] [info] menu_link_content module installed. [1.52 sec, 42.65 MB] [info] menu_ui module installed. [1.69 sec, 50.33 MB] [info] path module installed. [1.69 sec, 50.33 MB] [info] options module installed. [1.69 sec, 50.33 MB] [info] responsive_image module installed. [1.69 sec, 50.33 MB] [info] search module installed. [1.69 sec, 50.33 MB] [info] toolbar module installed. [1.69 sec, 50.33 MB] [info] shortcut module installed. [1.69 sec, 50.29 MB] [info] views_ui module installed. [1.69 sec, 50.29 MB] [notice] Performed install task: install_profile_modules [1.71 sec, 45.49 MB] [info] claro theme installed. [1.72 sec, 44.5 MB] [info] umami theme installed. [1.72 sec, 44.54 MB] [notice] Performed install task: install_profile_themes [1.74 sec, 45 MB] [warning] The "field_block:node:page:field_body" block plugin was not found [3.11 sec, 78.42 MB] [warning] The "extra_field_block:node:page:links" block plugin was not found [3.11 sec, 78.42 MB] [warning] The "extra_field_block:node:page:content_moderation_control" block plugin was not found [3.11 sec, 78.42 MB] [warning] The "field_block:node:page:field_body" block plugin was not found [3.11 sec, 78.42 MB] [warning] The "extra_field_block:node:page:links" block plugin was not found [3.11 sec, 78.42 MB] [warning] The "extra_field_block:node:page:content_moderation_control" block plugin was not found [3.11 sec, 78.42 MB] [warning] The "field_block:node:article:field_tags" block plugin was not found [3.13 sec, 77.49 MB] [warning] The "field_block:node:article:field_media_image" block plugin was not found [3.13 sec, 77.49 MB] [warning] The "field_block:node:article:field_body" block plugin was not found [3.13 sec, 77.49 MB] [warning] The "extra_field_block:node:article:links" block plugin was not found [3.13 sec, 77.49 MB] [warning] The "extra_field_block:node:article:content_moderation_control" block plugin was not found [3.13 sec, 77.49 MB] [warning] The "field_block:node:article:field_tags" block plugin was not found [3.14 sec, 77.49 MB] [warning] The "field_block:node:article:field_media_image" block plugin was not found [3.14 sec, 77.49 MB] [warning] The "field_block:node:article:field_body" block plugin was not found [3.14 sec, 77.49 MB] [warning] The "extra_field_block:node:article:links" block plugin was not found [3.14 sec, 77.49 MB] [warning] The "extra_field_block:node:article:content_moderation_control" block plugin was not found [3.14 sec, 77.49 MB] [warning] The "field_block:node:recipe:field_tags" block plugin was not found [3.31 sec, 82.28 MB] [warning] The "field_block:node:recipe:field_recipe_category" block plugin was not found [3.31 sec, 82.28 MB] [warning] The "field_block:node:recipe:field_summary" block plugin was not found [3.32 sec, 82.28 MB] [warning] The "field_block:node:recipe:field_media_image" block plugin was not found [3.32 sec, 82.28 MB] [warning] The "field_block:node:recipe:field_difficulty" block plugin was not found [3.32 sec, 82.28 MB] [warning] The "field_block:node:recipe:field_number_of_servings" block plugin was not found [3.32 sec, 82.28 MB] [warning] The "field_block:node:recipe:field_cooking_time" block plugin was not found [3.32 sec, 82.28 MB] [warning] The "field_block:node:recipe:field_preparation_time" block plugin was not found [3.32 sec, 82.28 MB] [warning] The "field_block:node:recipe:field_media_image" block plugin was not found [3.32 sec, 82.28 MB] [warning] The "field_block:node:recipe:field_ingredients" block plugin was not found [3.32 sec, 82.28 MB] [warning] The "field_block:node:recipe:field_recipe_instruction" block plugin was not found [3.32 sec, 82.28 MB] [warning] The "extra_field_block:node:recipe:content_moderation_control" block plugin was not found [3.32 sec, 82.28 MB] [warning] The "field_block:node:recipe:field_tags" block plugin was not found [3.32 sec, 82.29 MB] [warning] The "field_block:node:recipe:field_recipe_category" block plugin was not found [3.32 sec, 82.29 MB] [warning] The "field_block:node:recipe:field_summary" block plugin was not found [3.32 sec, 82.29 MB] [warning] The "field_block:node:recipe:field_media_image" block plugin was not found [3.32 sec, 82.29 MB] [warning] The "field_block:node:recipe:field_difficulty" block plugin was not found [3.32 sec, 82.29 MB] [warning] The "field_block:node:recipe:field_number_of_servings" block plugin was not found [3.32 sec, 82.29 MB] [warning] The "field_block:node:recipe:field_cooking_time" block plugin was not found [3.32 sec, 82.29 MB] [warning] The "field_block:node:recipe:field_preparation_time" block plugin was not found [3.32 sec, 82.29 MB] [warning] The "field_block:node:recipe:field_media_image" block plugin was not found [3.32 sec, 82.29 MB] [warning] The "field_block:node:recipe:field_ingredients" block plugin was not found [3.32 sec, 82.29 MB] [warning] The "field_block:node:recipe:field_recipe_instruction" block plugin was not found [3.32 sec, 82.29 MB] [warning] The "extra_field_block:node:recipe:content_moderation_control" block plugin was not found [3.32 sec, 82.29 MB] [info] demo_umami_content module installed. [4.72 sec, 104.79 MB] [info] demo_umami module installed. [4.77 sec, 99.85 MB] [notice] Performed install task: install_install_profile [4.77 sec, 98.92 MB] [error] Import of string "<span class="visually-hidden">Mostrar </span>@title<span class="visually-hidden"> medios </span> <span class="active-tab visually-hidden"> (seleccionados) </ lapso>" was skipped because of disallowed or malformed HTML. [7.16 sec, 99.48 MB] [notice] Translations imported: 8854 added, 0 updated, 0 removed. [7.69 sec, 99.55 MB] [warning] 1 disallowed HTML string(s) in files: translations://drupal-11.3-dev.es.po. [7.69 sec, 99.55 MB] [notice] Performed install task: install_import_translations [7.69 sec, 99.01 MB] [info] update module installed. [7.89 sec, 110.12 MB] [notice] Performed install task: install_configure_form [8.46 sec, 111.8 MB] [notice] The configuration was successfully updated. 203 configuration objects updated. [9.01 sec, 97.03 MB] [notice] Performed install task: install_finish_translations [9.01 sec, 96.99 MB] [notice] Performed install task: install_finished [9.09 sec, 97.83 MB] [success] Installation complete. User name: admin User password: J79hpJRQ87 [9.09 sec, 97.82 MB]
And here's the same output run against the MR branch:
[info] Starting bootstrap to root [0.78 sec, 2.91 MB] [info] Drush bootstrap phase 1 [0.78 sec, 2.91 MB] [info] Try to validate bootstrap phase 1 [0.78 sec, 2.91 MB] [info] Try to validate bootstrap phase 1 [0.78 sec, 2.92 MB] [info] Try to bootstrap at phase 1 [0.78 sec, 2.92 MB] [info] Drush bootstrap phase: bootstrapDrupalRoot() [0.78 sec, 2.92 MB] [info] Change working directory to /var/www/html [0.78 sec, 2.91 MB] [info] Initialized Drupal 11.3-dev root directory at /var/www/html [0.78 sec, 2.92 MB] [info] Drush bootstrap phase: bootstrapDrupalSite() [0.78 sec, 3.19 MB] [debug] Could not find a Drush config file at sites/default/drush.yml. [0.78 sec, 3.19 MB] [info] Initialized Drupal site drupal.ddev.site at sites/default [0.78 sec, 3.19 MB] [info] Drush bootstrap phase: bootstrapDrupalConfiguration() [0.78 sec, 3.19 MB] [info] Executing: command -v sqlite3 [0.79 sec, 3.41 MB] You are about to: * DROP all tables in your 'sites/default/files/.sqlite' database. // Do you want to continue?: yes. [info] Sites directory sites/default already exists - proceeding. [0.79 sec, 3.48 MB] [info] sql:query: .tables [0.79 sec, 3.48 MB] [info] Executing: sqlite3 sites/default/files/.sqlite < /tmp/drush_OHkNT4 [0.79 sec, 3.49 MB] [info] sql:query: DROP TABLE block_content; DROP TABLE node_revision; DROP TABLE block_content__body; DROP TABLE node_revision__body; DROP TABLE block_content_field_data; DROP TABLE node_revision__comment; DROP TABLE block_content_field_revision; DROP TABLE node_revision__field_image; DROP TABLE block_content_revision; DROP TABLE node_revision__field_tags; DROP TABLE block_content_revision__body; DROP TABLE path_alias; DROP TABLE comment; DROP TABLE path_alias_revision; DROP TABLE comment__comment_body; DROP TABLE router; DROP TABLE comment_entity_statistics; DROP TABLE search_dataset; DROP TABLE comment_field_data; DROP TABLE search_index; DROP TABLE config; DROP TABLE search_total; DROP TABLE file_managed; DROP TABLE sequences; DROP TABLE file_usage; DROP TABLE shortcut; DROP TABLE help_search_items; DROP TABLE shortcut_field_data; DROP TABLE history; DROP TABLE shortcut_set_users; DROP TABLE key_value; DROP TABLE taxonomy_index; DROP TABLE menu_link_content; DROP TABLE taxonomy_term__parent; DROP TABLE menu_link_content_data; DROP TABLE taxonomy_term_data; DROP TABLE menu_link_content_field_revision; DROP TABLE taxonomy_term_field_data; DROP TABLE menu_link_content_revision; DROP TABLE taxonomy_term_field_revision; DROP TABLE menu_tree; DROP TABLE taxonomy_term_revision; DROP TABLE node; DROP TABLE taxonomy_term_revision__parent; DROP TABLE node__body; DROP TABLE user__roles; DROP TABLE node__comment; DROP TABLE user__user_picture; DROP TABLE node__field_image; DROP TABLE users; DROP TABLE node__field_tags; DROP TABLE users_data; DROP TABLE node_access; DROP TABLE users_field_data; DROP TABLE node_field_data; DROP TABLE watchdog; DROP TABLE node_field_revision; [0.79 sec, 3.5 MB] [info] Executing: sqlite3 sites/default/files/.sqlite < /tmp/drush_8ep5L0 [0.79 sec, 3.5 MB] [notice] Starting Drupal installation. This takes a while. [0.85 sec, 3.49 MB] [debug] Calling install_drupal(Composer\Autoload\ClassLoader, array, array) [0.85 sec, 3.49 MB] [notice] Performed install task: install_select_language [1.03 sec, 12.1 MB] [notice] Performed install task: install_select_profile [1.03 sec, 12.1 MB] [notice] Performed install task: install_load_profile [1.03 sec, 12.1 MB] [notice] Performed install task: install_verify_requirements [1.04 sec, 12.45 MB] [notice] Performed install task: install_verify_database_ready [1.04 sec, 12.45 MB] [info] sqlite module installed. [1.14 sec, 17.64 MB] [info] system module installed. [1.2 sec, 21 MB] [notice] Performed install task: install_base_system [1.21 sec, 21.03 MB] [notice] Performed install task: install_bootstrap_full [1.21 sec, 21.03 MB] [info] user module installed. [1.58 sec, 27.84 MB] [info] path_alias module installed. [1.58 sec, 27.83 MB] [info] big_pipe module installed. [1.58 sec, 27.83 MB] [info] config module installed. [1.58 sec, 27.83 MB] [info] help module installed. [1.58 sec, 27.63 MB] [info] page_cache module installed. [1.58 sec, 27.63 MB] [info] dynamic_page_cache module installed. [1.58 sec, 27.63 MB] [info] automated_cron module installed. [1.58 sec, 27.63 MB] [info] announcements_feed module installed. [1.58 sec, 27.63 MB] [info] block module installed. [1.58 sec, 27.63 MB] [info] filter module installed. [1.59 sec, 28.02 MB] [info] views module installed. [1.59 sec, 28.04 MB] [info] field module installed. [1.59 sec, 27.85 MB] [info] text module installed. [1.59 sec, 27.85 MB] [info] block_content module installed. [1.6 sec, 27.9 MB] [info] breakpoint module installed. [1.6 sec, 27.9 MB] [info] file module installed. [1.6 sec, 27.7 MB] [info] editor module installed. [1.6 sec, 27.7 MB] [info] ckeditor5 module installed. [1.6 sec, 27.7 MB] [info] image module installed. [1.61 sec, 27.92 MB] [info] media module installed. [1.94 sec, 34.8 MB] [info] language module installed. [1.95 sec, 34.85 MB] [info] locale module installed. [1.95 sec, 34.9 MB] [info] config_translation module installed. [1.95 sec, 34.89 MB] [info] contact module installed. [1.96 sec, 34.63 MB] [info] workflows module installed. [1.96 sec, 34.37 MB] [info] content_moderation module installed. [1.96 sec, 34.37 MB] [info] node module installed. [1.99 sec, 35.17 MB] [info] content_translation module installed. [1.99 sec, 35.2 MB] [info] contextual module installed. [2 sec, 34.94 MB] [info] datetime module installed. [2 sec, 34.94 MB] [info] taxonomy module installed. [2 sec, 35.1 MB] [info] dblog module installed. [2 sec, 34.84 MB] [info] layout_discovery module installed. [2 sec, 34.84 MB] [info] field_ui module installed. [2 sec, 34.84 MB] [info] history module installed. [2 sec, 34.84 MB] [info] layout_builder module installed. [2 sec, 34.84 MB] [info] link module installed. [2 sec, 34.84 MB] [info] media_library module installed. [2.1 sec, 42.83 MB] [info] menu_link_content module installed. [2.1 sec, 42.83 MB] [info] menu_ui module installed. [2.29 sec, 50.61 MB] [info] path module installed. [2.29 sec, 50.61 MB] [info] options module installed. [2.29 sec, 50.61 MB] [info] responsive_image module installed. [2.29 sec, 50.61 MB] [info] search module installed. [2.29 sec, 50.61 MB] [info] toolbar module installed. [2.29 sec, 50.61 MB] [info] shortcut module installed. [2.3 sec, 50.56 MB] [info] views_ui module installed. [2.3 sec, 50.56 MB] [notice] Performed install task: install_profile_modules [2.31 sec, 45.76 MB] [info] claro theme installed. [2.33 sec, 44.77 MB] [info] umami theme installed. [2.33 sec, 44.82 MB] [notice] Performed install task: install_profile_themes [2.34 sec, 45.27 MB] [warning] The "field_block:node:page:field_body" block plugin was not found [3.82 sec, 78.62 MB] [warning] The "extra_field_block:node:page:links" block plugin was not found [3.82 sec, 78.62 MB] [warning] The "extra_field_block:node:page:content_moderation_control" block plugin was not found [3.82 sec, 78.62 MB] [warning] The "field_block:node:page:field_body" block plugin was not found [3.82 sec, 78.61 MB] [warning] The "extra_field_block:node:page:links" block plugin was not found [3.82 sec, 78.61 MB] [warning] The "extra_field_block:node:page:content_moderation_control" block plugin was not found [3.82 sec, 78.61 MB] [warning] The "field_block:node:article:field_tags" block plugin was not found [3.85 sec, 77.69 MB] [warning] The "field_block:node:article:field_media_image" block plugin was not found [3.85 sec, 77.69 MB] [warning] The "field_block:node:article:field_body" block plugin was not found [3.85 sec, 77.69 MB] [warning] The "extra_field_block:node:article:links" block plugin was not found [3.85 sec, 77.69 MB] [warning] The "extra_field_block:node:article:content_moderation_control" block plugin was not found [3.85 sec, 77.69 MB] [warning] The "field_block:node:article:field_tags" block plugin was not found [3.85 sec, 77.69 MB] [warning] The "field_block:node:article:field_media_image" block plugin was not found [3.85 sec, 77.69 MB] [warning] The "field_block:node:article:field_body" block plugin was not found [3.85 sec, 77.69 MB] [warning] The "extra_field_block:node:article:links" block plugin was not found [3.85 sec, 77.69 MB] [warning] The "extra_field_block:node:article:content_moderation_control" block plugin was not found [3.85 sec, 77.69 MB] [warning] The "field_block:node:recipe:field_tags" block plugin was not found [4.11 sec, 82.48 MB] [warning] The "field_block:node:recipe:field_recipe_category" block plugin was not found [4.11 sec, 82.48 MB] [warning] The "field_block:node:recipe:field_summary" block plugin was not found [4.11 sec, 82.48 MB] [warning] The "field_block:node:recipe:field_media_image" block plugin was not found [4.11 sec, 82.48 MB] [warning] The "field_block:node:recipe:field_difficulty" block plugin was not found [4.11 sec, 82.48 MB] [warning] The "field_block:node:recipe:field_number_of_servings" block plugin was not found [4.11 sec, 82.48 MB] [warning] The "field_block:node:recipe:field_cooking_time" block plugin was not found [4.11 sec, 82.48 MB] [warning] The "field_block:node:recipe:field_preparation_time" block plugin was not found [4.11 sec, 82.48 MB] [warning] The "field_block:node:recipe:field_media_image" block plugin was not found [4.11 sec, 82.48 MB] [warning] The "field_block:node:recipe:field_ingredients" block plugin was not found [4.11 sec, 82.48 MB] [warning] The "field_block:node:recipe:field_recipe_instruction" block plugin was not found [4.11 sec, 82.48 MB] [warning] The "extra_field_block:node:recipe:content_moderation_control" block plugin was not found [4.11 sec, 82.48 MB] [warning] The "field_block:node:recipe:field_tags" block plugin was not found [4.11 sec, 82.48 MB] [warning] The "field_block:node:recipe:field_recipe_category" block plugin was not found [4.11 sec, 82.48 MB] [warning] The "field_block:node:recipe:field_summary" block plugin was not found [4.11 sec, 82.48 MB] [warning] The "field_block:node:recipe:field_media_image" block plugin was not found [4.11 sec, 82.48 MB] [warning] The "field_block:node:recipe:field_difficulty" block plugin was not found [4.11 sec, 82.48 MB] [warning] The "field_block:node:recipe:field_number_of_servings" block plugin was not found [4.11 sec, 82.48 MB] [warning] The "field_block:node:recipe:field_cooking_time" block plugin was not found [4.11 sec, 82.48 MB] [warning] The "field_block:node:recipe:field_preparation_time" block plugin was not found [4.11 sec, 82.48 MB] [warning] The "field_block:node:recipe:field_media_image" block plugin was not found [4.11 sec, 82.48 MB] [warning] The "field_block:node:recipe:field_ingredients" block plugin was not found [4.11 sec, 82.48 MB] [warning] The "field_block:node:recipe:field_recipe_instruction" block plugin was not found [4.11 sec, 82.48 MB] [warning] The "extra_field_block:node:recipe:content_moderation_control" block plugin was not found [4.11 sec, 82.48 MB] [info] demo_umami_content module installed. [5.53 sec, 104.99 MB] [info] demo_umami module installed. [5.6 sec, 100.06 MB] [notice] Performed install task: install_install_profile [5.61 sec, 99.12 MB] [error] Import of string "<span class="visually-hidden">Mostrar </span>@title<span class="visually-hidden"> medios </span> <span class="active-tab visually-hidden"> (seleccionados) </ lapso>" was skipped because of disallowed or malformed HTML. [8.5 sec, 99.81 MB] [notice] Translations imported: 8854 added, 0 updated, 0 removed. [8.93 sec, 99.88 MB] [warning] 1 disallowed HTML string(s) in files: translations://drupal-11.3-dev.es.po. [8.93 sec, 99.88 MB] [notice] Performed install task: install_import_translations [8.93 sec, 99.34 MB] [info] update module installed. [9.11 sec, 110.46 MB] [notice] Performed install task: install_configure_form [9.67 sec, 112.14 MB] [notice] The configuration was successfully updated. 203 configuration objects updated. [10.29 sec, 97.37 MB] [notice] Performed install task: install_finish_translations [10.29 sec, 97.33 MB] [notice] Performed install task: install_finished [10.37 sec, 98.17 MB] [success] Installation complete. User name: admin User password: 3WU4QBARFJ [10.37 sec, 98.16 MB]
Anecdotally, run time and max memory usage do not seem to be affected too much, even with all module service classes being reflected.
Added the CR https://www.drupal.org/node/3526437 β .
The long, long bit about altered and decorated services in #33 should probably be in the CR as well, but I'm a little tired of typing for now. I can update the CR after there's agreement on that behavior makes sense.
It is not possible to use a service modifier to alter
Hook\
class services that are not registered servicesOK, refactored to split the changes in
HookCollectorPass
to a separate pass for hook discovery registered services that runs afterModifyServiceDefinitionsPass
. The change now allowsHook\
class services that are not registered services to be altered by a service modifier in another module. The code changes are not the prettiest, but I think they are OK.That all being said, changes here are crashing into other in-progress work on OOP hooks in π HookCollectorPass registers event listeners but they do not follow the required calling convention Active , π [pp-1] Explore adding an ImplementationList object for hook implmentations Active , and π Move hook_implementations_map generation to late in service container compilation to make altering hooks classes more DX friendly Active (among others I'm not aware of?), so I think some sort of priority of what goes in first probably would make things clearer.
It was good to see this working, though, especially the part of making it possible for kernel tests to subscribe to hooks. I think that will be a big DX win once it's in.
- πΊπΈUnited States nicxvan
Thank you!
I may have to create a meta, but in my mind for now this is the order I think we should work on these:
π HookCollectorPass registers event listeners but they do not follow the required calling convention Active which includes π [pp-1] Explore adding an ImplementationList object for hook implmentations Active
π Move hook_implementations_map generation to late in service container compilation to make altering hooks classes more DX friendly Active may not be needed after that change and we may not want to do that at all we need to explore it a bit more.
This issue can likely go after the decoupling from events I would think.
This issue can likely go after the decoupling from events I would think.
I think this plan is fine. One thing to note is that this issue, or at least this MR, also fixes a bug (or addresses a feature gap) where currently altering or decorating the autodiscovered
Hook\
classes doesn't work quite right. This is because the hook implementation map has the hook class name derived from the PHP file on disk as one of its keys ($this->hookImplementationsMap[$hook][{CLASS NAME}]
). The event dispatcher resolves the callable correctly to the altered/decorator class, so callingget_class()
on that will make the look up in the map fail (and also cause a PHP warning).On a cursory glance, I think π Move hook_implementations_map generation to late in service container compilation to make altering hooks classes more DX friendly Active also tries to address this same bug, but I haven't looked at π HookCollectorPass registers event listeners but they do not follow the required calling convention Active enough to see if it's addressed there separately.
In any way, I think there is some urgency to get this bug addressed, regardless of which issue is done in.
- πΊπΈUnited States nicxvan
Yes, that bug is concerning, the thing that has me hesitating on both is we're adding an extra compiler pass. Then for this one I'm concerned about performance for two reasons.
1. We need to reflect far more, we had thought that symfony was already reflecting and caching it so we dropped that concern, but we don't have the symfony flag set.
2. We are now rechecking on each hook if it's been changed.I haven't looked at the event dispatcher issue with an eye to this gap, but I don't think it addresses it.
I need to get a better idea on timing, if this can go into 11.x now that would be good, but I think with 11.2 coming soon we may have to wait a bit, I'm unclear on timing.