- Issue created by @nicxvan
- π¬π§United Kingdom catch
We'll need to convert all core hook implementations before we do this, and would be good if the rector rule was available to contrib too given 99.9% of modules will need it.
There are a couple of things I'm not sure about here:
At some point we will want to stop discovering and invoking procedural hooks, this could be either Drupal 12.0 or Drupal 13.0 - it's quite an expensive bc layer.
At that point, any remaining hooks will be 'dead code' and not run. This is a bit different from some deprecations where there is a clear error message when the thing is removed, that code will just stop running instead. We would not want to keep the discovery around just to throw an exception.
Given the eventual removal of bc will be 'silent' rather than a hard break, that makes me wonder if we should deprecate in 11(.2?) for removal in 13.0, then there is plenty of time for noisy deprecations. The disadvantage of this is that we'd have to keep the expensive bc layer around, but maybe we can put it behind a feature flag so that sites can turn it off, if they know they're 100% converted over
It is really great that we came up with LegacyHook for 10.x support since this means modules can convert to using the new stuff immediately and still support 10.3 or even 10.2, then one day
rm example.module
when they drop Drupal 10 support. - πΊπΈUnited States nicxvan
I'd vote for deprecation in the next minor version after all of core is converted.
- πΊπΈUnited States nicxvan
π Deprecate Procedural hook implementations Active is a super naive implementation of this. Chx pointed out this is to expensive to really do it this way most likely.
I plan on cleaning up my conversion script for contrib over the next week or so.
- πΊπΈUnited States nicxvan
Also, only slightly related, but I think I'm going to create a cleanup meta now that core is converted to clean up some inc and module files that only really have constants or a single helper function.
- π¬π§United Kingdom longwave UK
Are we almost at the stage where we can deprecate .module files entirely? It would be very cheap to throw a deprecation warning if the .module file still exists, and convert that to an error in the next major.
- πΊπΈUnited States nicxvan
Not quite, but close.
I think we need to figure out follow up 2 here: https://www.drupal.org/project/drupal/issues/3472165 π [PP-1] Explore converting Install hooks to OOP Active
Then we need to figure out how in themes / on behalf of themes.
Hook hook info
Hook module implements alterAnd where to put cost and define statements.
Then we can.
I think we're close to deprecating .inc
We just have to figure out the three helpers for views and Hook Hook info for that I think https://www.drupal.org/project/drupal/issues/2233261 π Deprecate hook_hook_info() Active
- π¬π§United Kingdom catch
I think we need to decide whether we want to remove support for procedural hooks in Drupal 12 or 13. For me, even if it's automated, it's probably going to be too much to remove support in Drupal 12 - there are a lot of sites with a lot of old custom modules.
Also even though I know the reasons why, it feels a bit hard to say you can't have a .module file but you have to have a .install file - giving ourselves a bit more time might mean we can figure out alternatives for hook_install() et al by then.
We've got two main problems keeping the bc layer in:
1. The performance cost of legacy hook discovery. If π Add a feature flag for the procedural hooks bc layer Active works we could make that opt-out in Drupal 11, opt-in in Drupal 12, gone in Drupal 13.
2. Hook ordering and etc. if we can come up with an all-OOP hook ordering system, I think it would be good and fine to deprecate hook_module_implements_alter() for removal in Drupal 12 - if you have procedural hooks in Drupal 12, you don't get to order them and it's a bug report/task against the module to convert if it breaks something.
If the above is roughly OK, then deprecating .module files in their entirety, for removal in Drupal 13, starts to sound pretty good then - we give people a lot of warning and then do a clean sweep, and they've got until the end of Drupal 12 support to get ready.
- π¬π§United Kingdom joachim
Hook documentation needs to be converted to OO as well before we do this.
- π¨π¦Canada Charlie ChX Negyesi πCanada
api module support is progressing. We have generic attribute support just needs a test -- this has been so for weeks now, I have been helping nicxvan with the Oscar patch instead of api work -- and we have a proof of concept for #Hook support in specifics.
Once π [PP-1] Determine how to implement Form Alters with attributes. Active is done we can write a very simple Rector rule which splits hook docs onto relevant attribute classes.
As far as I am concerned π Investigate ModuleHandler::add Active is the most urgent here, it needs to be gone as soon as possible, that method is a complete lie now as it only works with procedural hooks, it can't work with anything else and so it subtly blocks progress.
- πΊπΈUnited States nicxvan
Also, if we want to prioritize .install etc, there is a path we can take where we just have a special serviceProvider like class for the install time ones, we don't get the true benefits of OOP, but it would still give some organization options and we could deprecate .install files at that point.
I think these are the ones in .install still:
* - hook_install()
* - hook_post_update_NAME()
* - hook_schema()
* - hook_uninstall()
* - hook_update_last_removed()
* - hook_update_N()These are sometimes in .install, but they already support OOP
* - hook_module_preinstall()
* - hook_module_preuninstall()
* - hook_modules_installed()
* - hook_modules_uninstalled()Once that is broken down it would make it easier to scope out how to truly convert those hooks.
The really big hurdle I think just from an amount of code in .module files is handling:
- hook_preprocess_HOOK
- hook_process_HOOK
- hook_theme_suggestions_HOOK
As well as handling hooks called from themes, if we solve those two things that covers like 80% (made up percentage) of the remaining code in .module files I think, most everything else in my experience are helper methods that are in .module for convenience and can be moved, or things for batch / queues, but those already have a way to move.
- πΊπΈUnited States nicxvan
Oh yeah we also have to get rid of hook info and replace hmia too.
- πΊπΈUnited States nicxvan
Can't forget hook_requirements which has π Replace hook_requirements with tagged services Active
It hook_install_tasks and alter.
I think realistically if we want to really deprecate procedural hooks we will need to complete follow up 3 and 4 here π [PP-1] Explore converting Install hooks to OOP Active
That likely will involve specific classes for most install time hooks so we can load them manually.
If we decide to ignore hooks that do not go through modulehandler (e.g. ones invoked directly by moduleinstaller install.core.inc and during update)
I think that leaves:
hook_hook_info π Deprecate hook_hook_info() Active
hook_module_implements_alter π Hook ordering across OOP, procedural and with extra types Active
hook_requirements π Replace hook_requirements with tagged services ActiveInfo and requirements have a solid plan with some follow ups ready for review.
Hmia needs a plan. - π¬π§United Kingdom catch
I think it's fine to deprecate procedural hooks without sorting out ones that can't be converted yet, it will just need clear documentation on the change record etc.
Ideally it would be good if we had solutions for install hooks and etc. before we completely disallow procedural hooks in Drupal 13 because I think that will be harder to explain. But all those things can be worked on in parallel.
- πΊπΈUnited States nicxvan
Once we solve those three we can likely just deprecate: https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Co... directly and that should filter everything.
- πΊπΈUnited States nicxvan
Note to self expand on #3409372-23: [meta] Replace hook_requirements with OOP classes β