- π«π·France andypost
Removed
field_hook_info()
from IS as it was moved toviews_hook_info()
in #2399931: Generic entity api field handler should live in views module not in field module βSo only tokens and views using this hook but it needs to deprecate
\Drupal\Core\Extension\ModuleHandlerInterface::getHookInfo()
as wellFiled follow-up to remove broken ref π Remove stray reference to field_hook_info() Fixed
- π«π·France andypost
Closed as duplicates
- π [meta] Core does not use hook_hook_info() Closed: duplicate
- β¨ Stop using magic functions and force modules to declare the hooks they want to implement Closed: duplicate
- β¨ Let hook_hook_info() handle hooks in .install files (aka "rename MODULE.install to MODULE.install.inc") Closed: duplicate - π¬π§United Kingdom catch
If we replace hooks completely, fine. But we are not there yet I think.
Until then, something like hook_hook_info() is useful for code organization, similar to autoloading for classes (not autoloading itself, but the PSR-4 file organization that is made necessary by autoloading).Just to answer #19 - a lot of modules and some of core too already puts the logic of their hook implementations into classes, so that the hook implementation in .module is just three lines or so. This pattern is available for every hook, not just the ones that have hook_hook_info() implemented for them.
- π¨πSwitzerland berdir Switzerland
Definitely no reason to keep this now that π OOP hooks using event dispatcher Needs review landed.
But doing so is tricky and needs two layers. We can't just remove hook_hook_info() implementations, because that would immediately break hooks that live in those files. First we need to deprecate hook implementations that live in those files. I don't think we need to do that one-one and introduce a deprecated flag on the hook info, we can do it for everything. But to avoid moving the token hook implementations twice, we probably want to convert those in core to services first.
- π¬π§United Kingdom catch
I wonder if we want to roll this into π [Plan] Determine how to deprecate procedural hooks. Active .
First deprecate procedural hooks that don't have the @LegacyHook attribute.
This could still leave @LegacyHook hook implementation in hook_hook_info() files.
But then when we're ready to drop Drupal 10 support, we can deprecate @LegacyHook too. And at the same time, we can deprecate hook_hook_info().
- π¨π¦Canada Charlie ChX Negyesi πCanada
Actually π OOP hooks using event dispatcher Needs review removed this functionality. That turned out to be a mistake.
I can already see the future thanks to test issue where nicxvan is testing converting everything everywhere all at once (or, in short, the Oscar patch). The views hooks can be converted without a problem and almost all *.views.inc becomes empty and is deleted. However, these includes not only contain hooks but helpers as well.:
datetime_type_field_views_data_helper
,views_entity_field_label
,views_field_default_views_data
So for now, π Hook info must be procedural and module handler needs to handle hook_info properly Active restores this functionality. These three helpers will need to be deprecated manually before hook_hook_info can be laid to rest.
- First commit to issue fork.
- πΊπΈUnited States nicxvan
Also we'll have to deprecate this but keep the system and views one around since contrib uses those so much.
The views hook classes support both types, but contrib will have to move those LegacyHook calls into the .module files at the least.
- π¨πSwitzerland berdir Switzerland
Per #31, we can't really deprecate this, not like we usually do.
> Also we'll have to deprecate this but keep the system and views one around since contrib uses those so much.
This doesn't just apply to system and views. It applies to all implementations. We can't tell modules to stop implementing this hook, because other modules that implement their hooks might still be in those files.
I think we should add a mention to hook_hook_info() docblock that this should no longer be implemented but existing implementations must be kept for BC.
And then we can just drop it in D12 (or whenever we remove the old hooks), no deprecation needed, it won't do anything anymore. It doesn't need to be converted, so no deprecation needed.
So IMHO, this task is actually not blocked on anything and it's just a documentation issue.
- πΊπΈUnited States nicxvan
@berdir had a discussion in slack here about this: https://drupal.slack.com/archives/C079NQPQUEN/p1732497552707689
I copied my thoughts to the IS.
His approach is slightly different than mine, but I think they mesh well and will cause the least disruption.
The main issue with hook_hook_info is this:
If we deprecate it then modules will remove them.
Any modules that used the include files will no longer be autoloaded and broken, but they got no notice it was deprecated.@berdir proposes that we update the documentation that it will be removed in 12.
When it's removed we'll have deprecated procedural hooks and modules will have already moved theirs hooks except hook hook info.
Modules that support 10, 11 and 12 can keep the hook hook info function since it provides the inc files (which presumably only have legacy hook implementations) This will break D12 sites that still have helpers in the .inc files.The approach I propose in case the summary is updated.
We do what @berdir proposed, but also in 11.2 we deprecate the include files themselves.
This will force any modules that utilize for example mymodule.tokens.inc to move all of the functions out of the .inc, whether it's a hook or a helper.
Then when hook hook info is removed no modules will break because we've already deprecated the effect the hook hook info has.
The include files can be deprecated when we add them in module handler. This should also hit all custom modules as well. - πΊπΈUnited States nicxvan
I pushed up a POC, tons of tests should fail cause we have not removed the three helpers that core still has and this will trigger deprecations.
- πΊπΈUnited States nicxvan
I pushed up a first attempt at documenting the expected lifecycle of hook_hook_info.
I also moved views.views.inc and datetime.views.inc so I could see which tests really fail.
Commit f21048ab5d3d4c88733a1a1aeab3c9f9efddb955 will need to be reverted when the two issues postponing this one are in.
- πΊπΈUnited States nicxvan
Turns out I missed content moderation views inc I'll add that to the deprecation issue.
- πΊπΈUnited States nicxvan
The only failure is the one we need to add an expect deprecation for because it tests the functionality we're deprecating.