Dynamic hook ID specific attributes

Created on 31 May 2022, about 2 years ago
Updated 16 June 2023, about 1 year ago

Where part of the hook name itself is dynamic, consider introducing an attribute just for that class of hooks.

Core hooks could be supplied, but how can contrib do it? Maybe need a attribute class inferface. Attributes can extend. The full hook ID can be expanded and cached as a part of discovery.

E.g node_form_system_themes_admin_form_alter (hook_form_FORM_ID_alter), for we could introduce FormAlter(), so that it becomes FormAlter('system_themes_admin_form')

Something like FormAlter(\Drupal\system\Form\ThemeAdminForm::class) would ultimately be desirable, where a special method on the attribute class can return the full hook string based on uncacheable runtime data provided to it in the hook args. \Drupal\Core\Form\FormInterface::getFormId or baseFormId come to mind

At least

Core

hook_ENTITY_TYPE_access
hook_ENTITY_TYPE_build_defaults_alter
hook_ENTITY_TYPE_create
hook_ENTITY_TYPE_create_access
hook_ENTITY_TYPE_delete
hook_ENTITY_TYPE_field_values_init
hook_ENTITY_TYPE_insert
hook_ENTITY_TYPE_load
hook_ENTITY_TYPE_predelete
hook_ENTITY_TYPE_prepare_form
hook_ENTITY_TYPE_presave
hook_ENTITY_TYPE_revision_create
hook_ENTITY_TYPE_revision_delete
hook_ENTITY_TYPE_storage_load
hook_ENTITY_TYPE_translation_create
hook_ENTITY_TYPE_translation_delete
hook_ENTITY_TYPE_translation_insert
hook_ENTITY_TYPE_update
hook_ENTITY_TYPE_view
hook_ENTITY_TYPE_view_alter
hook_block_build_BASE_BLOCK_ID_alter
hook_block_view_BASE_BLOCK_ID_alter
hook_field_widget_WIDGET_TYPE_form_alter
hook_field_widget_complete_WIDGET_TYPE_form_alter
hook_field_widget_multivalue_WIDGET_TYPE_form_alter
hook_field_widget_single_element_WIDGET_TYPE_form_alter
hook_form_BASE_FORM_ID_alter
hook_form_FORM_ID_alter
hook_migrate_MIGRATION_ID_prepare_row
hook_preprocess_HOOK
hook_query_TAG_alter
hook_theme_suggestions_HOOK_alter
hook_theme_suggestions_HOOK

Contrib

hook_ENTITY_TYPE_embed_alter
hook_ENTITY_TYPE_embed_context_alter
hook_form_entity_browser_ENTITY_BROWSER_ID_form_alter
hook_webform_element_ELEMENT_TYPE_alter
hook_webform_handler_invoke_METHOD_NAME_alter
hook_webform_image_select_images_WEBFORM_IMAGE_SELECT_IMAGES_ID_alter
hook_webform_options_WEBFORM_OPTIONS_ID_alter

✨ Feature request
Status

Active

Version

1.0

Component

Code

Created by

πŸ‡¦πŸ‡ΊAustralia dpi Perth, Australia

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

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡¦πŸ‡ΊAustralia dpi Perth, Australia
  • πŸ‡©πŸ‡ͺGermany donquixote

    How would the interface look like?

    It should return the implementations as an array. But how would this array be structured?

    interface HookAttributeInterface {
      public function getDeclarations(): array;
    }
    

    Should it return a list of value objects?
    Perhaps we can use the public properties in this value object:

    final class Declaration {
      public function __construct(
        public string $hook,
        public ?string $moduleName = NULL,
        public int $priority = 0,
      ) {
        // Validation code as currently in Drupal\hux\Attribute\Hook.
        [..]
      }
    }
    

    Note that I am calling this "declaration", not "hook" or "implementation".

    To me, the terms mean the following:

    • A hook is the hook itself.
    • An implementation is a method or function name + the hook name + module name + priority.
    • A "declaration" is the hook name + module name + priority, but without the actual method reference.
      This is a very typical thing for attributes: The attribute object is unaware of the symbol it is attached to.

    ---

    Alternatively we could have something like a visitor/collector, but I don't really like it.

    interface HookAttributeInterface {
      public function collectDeclarations(DeclarationCollectorInterface $collector): void;
    }
    
    interface DeclarationCollectorInterface {
      public function addDeclaration(string $hook, string $module = NULL, int $priority = 0): void;
    }
    

    The problem with "attribute object is unaware of the symbol it is attached to" could be solved like this:

    interface HookAttributeInterface {
      public function getDeclarations(\ReflectionClass $class, \ReflectionMethod $method): array;
    }
    

    or

    interface HookAttributeInterface {
      public function collectDeclarations(DeclarationCollectorInterface $collector, \ReflectionClass $class, \ReflectionMethod $method): void;
    }
    

    The separate parameter for $class is needed because the attribute might be on a method in a parent class.

    This would give attribute classes the opportunity to do special things depending on the symbol they are attached to.
    E.g. they could read other attributes.
    But I guess most of them won't need it.

  • πŸ‡©πŸ‡ͺGermany donquixote

    Also, can we use the same interface and value objects for alter hooks and replacements?
    Or do we need dedicated interfaces for these?

  • πŸ‡¦πŸ‡ΊAustralia dpi Perth, Australia

    Posting notes from DM's.

    Im still not sure we've arrived at a good place, I may need to dedicate some hands on.

    Critically, I think we need a good sample of third-party attributes that cover some good use cases, such as the core list above. And cases such as FieldWidgetFormAlter(WIDGET_TYPE, [β€˜complete’, β€˜multivalue’]]) / FieldWidgetFormAlter(WIDGET_TYPE, complete

    Interface ideas, implement an interface:

    interface HookIf {
    // Wherein iterable = string[], for now. can be expanded to value object later.
    public function getHooks(): iterable;
    

    This method would return the standard hook strings we're familiar with. The return type could change in the future, expanding from strings. So long as its iterable we have a bit more flexibility. If we want to do some kind of value object we can later.

    Add that method to existing Hook, finalise the class.

    The default Hook attr itself would always represent one hook, but any third party could put together their own Hook attr extending the iface

  • πŸ‡¦πŸ‡ΊAustralia dpi Perth, Australia

    Ideas also posted in ✨ Advanced attributes to declare multiple hooks Closed: won't fix , closing other in favor of this.

  • πŸ‡©πŸ‡ͺGermany donquixote

    The MR at πŸ“Œ Combined refactoring and attributes interface Needs review contains a proof of concept implementation for this.

    See https://git.drupalcode.org/project/hux/-/merge_requests/15/diffs#ec48b92...

    This MR also contains lots of other changes, but they are all in distinct commits, so we can isolate them.

  • πŸ‡©πŸ‡ͺGermany donquixote

    Critically, I think we need a good sample of third-party attributes that cover some good use cases, such as the core list above.

    I am not sure I want this big range of 3rd party attributes, or rather a more flexible generic attribute like:

    /**
     * Implements hook_node_insert(), hook_node_update(), hook_user_insert(), hook_user_update().
     */
    #[Hook('(node|user)_(insert|update)')]
    

    (looks like a regex, but only (|) would be allowed, no wildcards or anything)

    Currently the main use case of Hux is one where the developer who uses Hux has control of only some modules.
    Usually for every hook there are multiple modules that implement the hook, one or a few modules that call/invoke the hook, but only one module that "publishes" or "owns" the hook.
    A developer is more likely to be working on implementations, without control of the module that "owns" the hook.

    The natural place for custom attribute classes would be with the module that publishes the hook. Otherwise there is the risk of repetition.
    E.g. multiple modules in contrib might want to provide an attribute class for HookEntityUpdate('node') or HookEntityOp('node', 'update').

    The second best place would be resuable helper modules that provide a bunch of these attribute classes for specific classes of hooks from populare core or contrib modules. This may be fine. But custom module developers might want to get started right away without adding more dependencies and waiting for their favourite hook to be supported.

    A generic syntax like #[Hook('(node|user)_(insert|update)')] could be easily used for any hook without the need for new dedicated attribute classes.

    Luckily it does not really matter much for the design of the interface.
    The interface and mechanism I propose in πŸ“Œ Combined refactoring and attributes interface Needs review would support both the custom attribute classes and more generic multi-hook syntax.
    This reminds me I need to add test cases with custom attribute classes.

  • πŸ‡¦πŸ‡ΊAustralia dpi Perth, Australia

    To be clear, Hux would only have essential attributes, not diverging too far from what exists presently.

    By sample I mean proof that the backend is sufficiently flexible. The actual attributes would not be implemented by Hux, other than in test modules.

  • πŸ‡©πŸ‡ͺGermany donquixote

    Btw in symfony there is also this:
    $container->registerAttributeForAutoconfiguration()
    https://symfony.com/doc/current/service_container/tags.html#reference-ta...
    https://github.com/symfony/symfony/blob/6.3/src/Symfony/Component/Depend...

    This allows to register attributes with custom behavior without having all of that behavior in the attribute class itself.
    With this, the attribute class does not need to implement an interface.

    One disadvantage would be that, by losing the interface, a single call to ->getAttributes(HookAttributeInterface::class, \ReflectionAttribute::IS_INSTANCEOF) is no longer sufficient. We need a separate call for every registered attribute class. Or we get all attributes and then filter by registered class names.

    I think for our case, the interface is fine, because most of these classes will just return a list of hook names.

    The actual attributes would not be implemented by Hux, other than in test modules.

    Yep.
    I did post more thoughts on this in slack, to reduce noise in this issue.

Production build 0.69.0 2024