- Issue created by @nicxvan
- First commit to issue fork.
- π¨πSwitzerland berdir Switzerland
Created a merge request. Haven't tested yet at all whether this works correctly, other than the reduced number of procedural hooks.
In my install profile, I counted 1040 registered procedural hooks. That is without π Mark core modules as converted to OOP hooks Active so it should go down around 200 then with that.
With this change, it's now 887, so that's 153 less functions stored in the container that need to be checked.
While testing, I noticed a strange thing with the regular expressions, it didn't catch some of the update functions. It turns out that the magic regular expression attributes update functions in submodules that share a prefix with another module are somehow attributed to the wrong module/hook.
if (in_array($function, ['redirect_404_cron', 'redirect_404_update_8101'])) { var_dump($matches); }
results in:
array(7) { [0]=> string(17) "redirect_404_cron" ["function"]=> string(17) "redirect_404_cron" [1]=> string(17) "redirect_404_cron" ["module"]=> string(12) "redirect_404" [2]=> string(12) "redirect_404" ["hook"]=> string(4) "cron" [3]=> string(4) "cron" } array(7) { [0]=> string(24) "redirect_404_update_8101" ["function"]=> string(24) "redirect_404_update_8101" [1]=> string(24) "redirect_404_update_8101" ["module"]=> string(8) "redirect" [2]=> string(8) "redirect" ["hook"]=> string(15) "404_update_8101" [3]=> string(15) "404_update_8101" }
So while the cron hook is attributed correctly to redirect_404 and not redirect, redirect_404_update_8101 is not.
I only realized while writing this one what the problem is. It's that $module_reg already attempts to filter out all update functions but it does not account for the prefix/length thing, so it will simply match the above on the non-existing hook 404_update_8101. I pushed an update that adjusts the original regex, but that definitely needs to be reviewed by someone smarter than me, because that regex is well above my paygrade.
While doing that, I also noticed that it also excludes preprocess functions, which means that a handful of weird hooks in contrib modules that start with preprocess will be broken since their legacy hook implementations won't be found anymore. Might be OK, might not be. Should be a separate issue if so: https://git.drupalcode.org/search?group_id=2&scope=blobs&search=%22invok...
- π¨πSwitzerland berdir Switzerland
This is passing now.
I had to allow hook_hook_info() and hook_module_implements_alter(), they can't be new OOP hooks, but they need to run through the legacy processing to properly register and work. That means there are a few additional functions to process compared to what I wrote in #4.