Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.What are the magic functions buying us? In my experience, developers are confused when they start trying to figure out where what is getting called by whom. Plus, we have lots of expensive aparatus for discovering these functions and maintaining reliable lists of them. We don't respond well to state changes (like modules moving to a new directory, or two versions of the same module in the code base. And it's also a relatively inflexible structure. We can't, for instance, declare that I want hook_x to run, but I want it to ran after some other module runs its hook_x (okay we can with crazy alter functions, but it is pretty intense.
This pattern would follow a more typical observer pattern, similar to the one that is used in jQuery with events:
function mymodule_init($drupal) {
$drupal->tellMeAbout('menu', 'mymodule_menu');
// could even...
$drupal->runABeforeB('menu', 'someothermodule','mymodule');
}
Modules would remain the same, they would simply need to add some kind of function to the top of the module file which every module would have to declare. It would be an easy thing to script with coder.
This would solve a lot of our problems IMO, and help DX a lot while only forcing module maintainers to add a very small amount of code. If others are interested in what I admit is a somewhat radical departure of the land of magic functions, I will draft up a patch. I don't think it would take too long.
As an aside, other modules could use this pattern since we are using dependency injection with the event registry / controller thing being passed in. So the views module could let its submodules / includes register hooks that are specifically only for views.
Comments
Comment #2
joachim CreditAttribution: joachim commentedThis would be in a way a generalization of hook_node_info()'s concept of 'base', or hook_forms() system of mapping one form name to a different callback.
A few comments:
- jbrown's proposal for namespaces in D8 addresses this problem in a different -- and more radical -- way
- for modules that just want to do the usual MODULE_HOOKNAME, we should provide a helper to make this easier: tellhelper('MODULE', 'hook_a', 'hook_b'....) and that can produce the more verbose array.
- should probably not be in mymodule_init but in something whose results get cached.
Comment #3
JacobSingh CreditAttribution: JacobSingh commentedI just used hook_init as an example. But really, it could be anything. Basically, you need one magic function to register that it will be responsible for doling out responsibility. You could also just declare hook implementations in an info file I suppose.
Caching could still be done, just like we do for hook_menu.
Best,
Jacob
Comment #4
JacobSingh CreditAttribution: JacobSingh commentedstupid tag added
Comment #5
pillarsdotnet CreditAttribution: pillarsdotnet commented+1
Comment #6
pillarsdotnet CreditAttribution: pillarsdotnet commentedSounds like a job for
hook_hook_info().Comment #7
pillarsdotnet CreditAttribution: pillarsdotnet commentedComment #8
agentrickard[edit] Removing comment because it was OT from this issue. See #1373884: [meta] Core does not use hook_hook_info() instead.
Comment #9
agentrickardThe implementation ticket is #624324: Do not load support code for optional core modules for moving functions to files.
Comment #10
sunNote that #624324: Do not load support code for optional core modules relies on the automatic discovery of hook implementations. It makes all core modules register all hooks they are exposing (which others can implement). This is about API providers.
That's not the same as having every module register which hooks it implements. This is about API consumers.
I'd like to see an example implementation for this in this issue first.
And as these are completely different operations, hook_hook_info() would be a slightly confusing place to implement the registry for API consumers. Therefore, let's discuss the implementation first, before shortcutting it and settling on hook_hook_info().
Lastly, please note that hook_hook_info() is a very poor construct currently, and before advancing on it, we need to solve a range of major issues:
#968264: hook_hook_info() can lead to an infinite loop
#1029158: Let hook_hook_info() handle hooks in .install files (aka "rename MODULE.install to MODULE.install.inc")
#825854: Module_implements should cache the file path and not TRUE/group value for hooks that specify group in hook_hook_info()
#451152: Implementing a hook on the behalf of another module
And eventually, this issue seems to be a duplicate of #983470: Improve lazy-load mechanism for procedural code
Comment #11
agentrickardShould we not implement the existing API first and then clean it up?
Comment #12
agentrickard@sun
I was pointed here from a duplicate issue. Reading the whole thread, it looks like comments 6-7 shifted the focus of this issue.
That said, I'd still like to implement the code we have before refactoring the entire hook system.
Comment #13
sunHm, "Require all hooks to be declared via hook_hook_info()" sounds like a goal that is different to the OP here.
If I get the OP right, then the proposal discussed in this issue is:
In order to implement
hook_foo()in your module and have it invoked by the module system, you have to do:Without this code, yourmodule_foo() is unknown to the system and thus not invoked.
The OP merely uses an OO based approach, but the net gain/proposal is identical.
Comment #14
agentrickardYes. Agreed. I'm going to move my stuff back to my original issue. #1373884: [meta] Core does not use hook_hook_info().
Sorry for the distraction.
Comment #15
ralf.strobel CreditAttribution: ralf.strobel commentedThis is an interesting discussion. Maybe I can contribute by explaining how I first misunderstood the purpose of hook_hook_info(), because the way I wanted it to work seems quite like what you intend...
I thought that hook_hook_info() worked on a module level, so that I could define files to be included when hooks are called specifically for my module. For instance, I wanted to place all the cached definitions like hook_theme(), hook_menu(), hook_block_info() into a separate "mymodule.info.inc", so that this file would only be lazy-loaded whenever the cache has to be rebuilt.
I was really disappointed when I learned that hook_hook_info() only works globally. But maybe a good, backward-compatible solution would be to make it work both ways:
Whenever the 'global' attribute, which defaults to TRUE, is set to FALSE, the hook group definition would be exclusive to mymodule, invoking mymodule.lazy.inc whenever mymodule_some_lazy_hook() is called.
I believe this functionality would only require relatively minor changes to module_hook_info() module_hook(), module_implements() and module_invoke() to make it work.
Of course it remains to be seen how necessary this will still be, as soon as the core finally implements the existing hook_hook_info() to a reasonable degree, as just posted by agentrickard in #14.
Comment #28
andypostClosing in favour of #2233261: Deprecate hook_hook_info()