Currently the hook cache is an array of the format $module_name => $group.

We also know that the hook system is fragile.
sun suggested to allow an alternative format with Drupal\$module\Hook::foo() (and other proposals existed before that):
#2237831: Allow module services to specify hooks

As a preparational step before any such move, I suggest to change the hook cache so that it can be filled with arbitrary callbacks.
For the start, these can be added with hook_module_implements_alter().

Later we can add patterns like Drupal\$module\Hook::foo() or Drupal\$module\hook_foo(). This could even happen post-release, since it would only extend the API and not break it. (unless a module already defines a class that matches the pattern.. duh)

The change of the hook cache brings only one API break: It changes the structure of hook_module_implements_alter().
hook_hook_info() will continue to work as it does today.

Challenge: We still need a way to store which files should be included for a hook implementation. We may want to store the module name and the file path relative to the module dir. This way, things don't break if modules are moved around.
This means the simple array structure of $module_name => $group no longer does the job.

Note: #2263365: Second loop in module_implements() being repeated for no reason. should be fixed first!

CommentFileSizeAuthor
#25 2299537-nr-bot.txt144 bytesneeds-review-queue-bot
#16 2299537-16.patch7.02 KBshaktik
#8 2299537-2-hook_classes.patch6.12 KBchi

Comments

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

chi’s picture

chi commented
Version: 8.6.x-dev » 8.8.x-dev
Status: Active » Needs review

The attached patch allows to define hooks in classes like follows.

namespace Drupal\example\Hook;

/**
 * Implement hook_form_FORM_ID_alter().
 */
class FormSearchBlockFormAlter  {

  /**
   * Hook callback.
   */
  public function __invoke(&$form) {
    $form['keys']['#placeholder'] = t('Search site');
  }

}

Note that the hook callback is not a static method. So it is possible to inject dependencies via factory ::create() method. Current implementation is extremely simple and does not cover all edge cases. I expect tests will fail because there are a few places where hooks are invoked without using module handler service (example).

chi’s picture

chi commented
Issue tags: +Needs tests
StatusFileSize
new 2299537-2-hook_classes.patch6.12 KB
chi’s picture

chi commented
Related issues: +#2616814: Delegate all hook invocations to ModuleHandler

The patch only supports hook that invoked through module handler.

chi’s picture

chi commented
Related issues: +#2233261: Deprecate hook_hook_info groups, mark hook_hook_info() for deletion

The implementation would be much easier without hook_hook_info().

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Version: 8.9.x-dev » 9.0.x-dev

Too late for 8.9

joachim’s picture

joachim commented
Status: Needs review » Needs work 
  1. +++ b/core/lib/Drupal/Core/Extension/ModuleHandler.php
@@ -756,4 +756,35 @@ public function getName($module) {
+  /**
+   * Builds hook class name.
+   */
+  private function buildClassName($module, $hook) {
+    return '\Drupal\\' . $module . '\\Hook\\' . str_replace('_', '', ucwords($hook, ' _-'));
+  }

    > Later we can add patterns like Drupal\$module\Hook::foo() or

    This change looks like that proposal, which the issue summary says is not being tackled here. 

  2. +++ b/core/lib/Drupal/Core/Extension/ModuleHandler.php
@@ -756,4 +756,35 @@ public function getName($module) {
+      if (is_subclass_of($class, 'Drupal\Core\DependencyInjection\ContainerInjectionInterface')) {
+        return $class::create(\Drupal::getContainer());
+      }
+      else {
+        return new $class;
+      }

    Shouldn't this use the ClassResolver instead?

Also,

> The change of the hook cache brings only one API break: It changes the structure of hook_module_implements_alter().

The patch doesn't have any change to docs which an API change would need.

Though I think it would be better if we can maintain that structure if possible.

xjm’s picture

xjm
she/her
Primary language English
commented
Version: 9.0.x-dev » 9.1.x-dev

This would be a minor-only change. Since 8.9.x and 9.0.x are now in beta, I'm moving this to 9.1.x. Thanks!

shaktik’s picture

shaktik
he/him
Primary language Hindi
Location New Delhi
commented
Assigned: Unassigned » shaktik

#8 patch does not apply success on D9.

Shakti-Kumar:drupal shakti.kumar$ curl https://www.drupal.org/files/issues/2019-07-03/2299537-2-hook_classes.patch | git apply -v
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  6263  100  6263    0     0   6207      0  0:00:01  0:00:01 --:--:--  6490
Checking patch core/lib/Drupal/Core/Extension/ModuleHandler.php...
error: while searching for:
    }
  }

}

error: patch failed: core/lib/Drupal/Core/Extension/ModuleHandler.php:756
error: core/lib/Drupal/Core/Extension/ModuleHandler.php: patch does not apply

I am working on this.

shaktik’s picture

shaktik
he/him
Primary language Hindi
Location New Delhi
commented
Status: Needs work » Needs review
StatusFileSize
new 2299537-16.patch7.02 KB

Status: Needs review » Needs work

The last submitted patch, 16: 2299537-16.patch, failed testing. View results

shaktik’s picture

shaktik
he/him
Primary language Hindi
Location New Delhi
commented
Status: Needs work » Needs review

Testcase is passed on 7.3 and 7.4 kindly review.

Rkumar’s picture

Rkumar commented
Assigned: shaktik » Unassigned

Unassinging it for review.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 2299537-nr-bot.txt144 bytes

The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

Should we close won't fix this in favour of something like drupal.org/project/hux

andypost’s picture

andypost
he/him
Primary language Russian
commented
Related issues: +#329012: Remove call_user_func_array() from ModuleHandler::invoke() + invokeAll(), +#3259716: Replace usages of static::class . '::methodName' to first-class callable syntax static::method(...)

OTOH it makes sense to re-profile invocations as since PHP 8.1 new callable (faster calls) added so there's issues like #329012: Remove call_user_func_array() from ModuleHandler::invoke() + invokeAll() and #3259716: Replace usages of static::class . '::methodName' to first-class callable syntax static::method(...)

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

donquixote’s picture

donquixote commented

Continue here, #3366083: [META] Hooks via attributes on service methods (hux style)

nicxvan’s picture

nicxvan commented

@donquixote, can this be closed?

nicxvan’s picture

nicxvan commented
Status: Needs work » Closed (won't fix)

Pretty sure it can be it was already on the cusp and the issue referenced was also closed due to

#3442009: OOP hooks using attributes and event dispatcher

nicxvan’s picture

nicxvan commented
Status: Closed (won't fix) » Closed (duplicate)