- Issue created by @donquixote
- π©πͺGermany donquixote
Challenges from "Hux in core" plans
Single ->invoke()
With Hux in core, it is possible for one module to add more than one implementation per hook.
Normally these are most useful for hooks invoked with->invokeAll()
.But it can also be useful for some hooks invoked with for a single module with
->invoke($module, $hook)
.
One example would behook_schema()
: It can be useful to have multiple services that add to the schema for one module.With the weights and before/after keys, it can be possible to have some methods before and some after the main procedural function.
Method with #[Hook('myhook', module: 'mymodule', weight: -5)] Method with #[Hook('myhook', module: 'mymodule', before: 'mymodule')] function mymodule_myhook Method with #[Hook('myhook', module: 'mymodule', after: 'mymodule')] Method with #[Hook('myhook', module: 'mymodule', after: 'othermodule')] Method with #[Hook('myhook', module: 'mymodule', weight: 5)]
Now the annoying part:
At the time the hook is discovered, the file that containsmymodule_myhook
might not be included yet.
At the time->invoke()
is called, if we find thatmymodule_myhook()
exists, we need to insert that function in the correct position between the other callbacks.
With the current implementation in the MR, the cached lists don't contain the necessary metadata to determine that "would be" position. So instead, the hook needs to be discovered again.
This can be done, but is super annoying, and makes the system more complex and fragile. - π©πͺGermany donquixote
This might all sound a bit overkill.
But I think there is and will be an expectation that->invoke()
behaves consistently with->invokeAll()
, only that the callbacks are filtered down to the given module. - π©πͺGermany donquixote
Hooks I found that are used with
->invoke()
.Core:
- library_info_build
- update_last_removed
- install
- uninstall
- schema
- translate_access
- node_update_index
- views_post_render
- field_views_data
- requirements
- help
- quickedit_render_field
Contrib (from a big project, with no claim for completeness):
- webform_libraries_info
- data_policy_destination_alter
- pathauto_is_alias_reserved
- webform_element_input_masks
- π¬π§United Kingdom catch
:addProfile() ::addModule() ::setModuleList() are as far as I know only used in the installer, we could try to factor their uses out of the installer, then mark them @deprecated with no replacement.
For ->invoke(), we could completely disregard hook_module_implements_alter().
This is reasonable. I also think it's something we can eventually remove (hook_help() is going, hook_schema() is on its way out in core if not contrib etc.), although it will take years, so we need something in the interim.
No longer remove missing functions from the cached list, unless a cache rebuild was explicitly requested.
Instead, remove these functions in every request/process when the list is verified.This seems fine.
- π©πͺGermany donquixote
Thanks @catch!
Now how much of that can we do in a 10.x release? - π©πͺGermany donquixote
No longer remove missing functions from the cached list, unless a cache rebuild was explicitly requested.
Instead, remove these functions in every request/process when the list is verified.I wonder if we should log anything in that case.
- π¬π§United Kingdom catch
@donquixote theoretically all of it - of course it depends exactly what changes end up being necessary.
However deprecating methods and slight changes to what goes into alter hooks are both OK in minor releases.
- πΊπΈUnited States nicxvan
I think some of this is resolved with π OOP hooks using attributes and event dispatcher Needs review
- πΊπΈUnited States nicxvan
I think the only remaining issue is the ::add functionality which has a targeted followup from the previous mentioned issue:
π Investigate ModuleHandler::add Active