- π©πͺ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.