Add a feature flag for the procedural hooks bc layer

Created on 22 October 2024, 2 months ago

Problem/Motivation

Follow-up for πŸ“Œ OOP hooks using event dispatcher Needs review .

The bc layer for procedural hooks is quite expensive - needs to find and parse every file that could have a hook in it etc.

Once core is converted, it will be possible for a site to run without any procedural hook implementations at all.

I think we can add a feature flag, probably a container parameter would be best since it's used in container building, so that sites that are running fully converted to OOP hooks can disable the procedural hook discovery.

This in turn could let us keep the bc layer around longer, maybe until Drupal 13, maybe with the default reversed and instructions to enable it (or a feature flag module in the future).

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

πŸ“Œ Task
Status

Active

Version

11.0 πŸ”₯

Component

base system

Created by

πŸ‡¬πŸ‡§United Kingdom catch

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @catch
  • πŸ‡·πŸ‡ΊRussia Chi

    Once core is converted, it will be possible for a site to run without any procedural hook implementations at all.

    This requires that none of the installed contributed modules implement procedural hooks. Right?

  • πŸ‡¬πŸ‡§United Kingdom catch

    @chi yes that's right, possible not likely ;) I don't think any sites would be able to actually disable procedural hooks until 11.3 at the earliest or even later, but the end of the 11.x cycle maybe. Also if we want to leave the bc layer in until Drupal 13, we could keep it in 12.x this way, but with a status report mention that it's possible to disable (maybe even with some detection of whether it's being relied upon).

  • πŸ‡«πŸ‡·France andypost

    ++ to option to skip procedural hooks parsing

  • πŸ‡―πŸ‡΅Japan ptmkenny

    This sounds great. One request for clarification: the change record β†’ notes that several hooks will "remain procedural", including theme hooks like hook_preprocess() and install hooks like hook_schema() and hook_update(). So does "disable procedural hook discovery" mean "disable all procedural hooks which can be implemented with the OO pattern" or does it mean "disable all procedural hooks period"?

    At a minimum I would imagine hooks like hook_update() need to still be supported or sites will break when a contrib module has an update hook.

  • πŸ‡¬πŸ‡§United Kingdom catch

    That's a good question. It needs to be "disable all procedural hooks which can be implemented with the OO pattern" but we will need to make sure that's actually the case when we implement this.

    Install hooks are invoked by module installer and don't use module handler at all - so they'd just stay the same, they don't use the bc layer, it's an entirely different system.

    Theme hooks I think are in a similar situation but would need to actually double check. However generally if something's relying on the bc layer, it ought to be possible to implement the OOP version. πŸ“Œ [PP-1] Explore converting Install hooks to OOP Active is looking at OOP hook support, at which point any added OOP hook support would also fall under this.

    There are a couple of specific additional exceptions - hook_hook_info() and hook_module_implements_alter(). hook_hook_info() is incompatible with OOP hooks so it won't matter if it's not fired when the feature flag is off, even if a module implements it, but it will complicate detecting when it's OK to switch off. hook_module_implements_alter()... don't know will need to check.

  • πŸ‡ΊπŸ‡ΈUnited States nicxvan

    ✨ Explore ways to mark hook conversion status per module or file Active is related and ready for review.

Production build 0.71.5 2024