- Issue created by @dpi
- Issue was unassigned.
- Status changed to Needs review
10 months ago 1:27pm 6 July 2024 - 🇦🇺Australia dpi Perth, Australia
Sample exception:
BCA found Drupal\Module1\Entity\Document and Drupal\Module2\Entity\Media\Document
vying to be the bundle class media:document, but one does not extend the other, so a winner
could not be determined. Use the `priority` property on Drupal\bca\Attribute\Bundle with a
value larger than 0 to select which bundle should take priority. - 🇦🇺Australia mstrelan
I think I'd like to get ✨ Make $entityType optional: infer from class heirarchy Needs review in first. That refactors the hook implementation out of bca.module and in to its own class. It also has simple logic for setting the plugin ID to the class name, so perhaps we wouldn't need BcaDiscovery?
I think the priority stuff makes sense, I guess without BCA you'd have to put the logic in your own hook_entity_bundle_info_alter and probably also hook_module_implements_alter.
- 🇦🇺Australia mstrelan
Adding ✨ Use class names instead of plugin IDs Active as related, let's shoot for the moon.
- 🇦🇺Australia dpi Perth, Australia
No problem about timing, I just need it for selfish reasons right now. Plan to revisit with whatever goodness we get elsewhere + tests.
- 🇦🇺Australia dpi Perth, Australia
Brought up at #3301682-21: Define bundle classes via attributes →
- 🇨🇭Switzerland berdir Switzerland
Drive-by review without actually using this module.
IMHO, bundle classes should be used by the module that "owns" a certain bundle. E.g. forum could be a use case in contrib, or book. And mostly in custom distributions and custom modules, because it's also a BC problem. And custom module may customize it.
> - A extends B extends Node
This is then a valid scenario, for example a custom module that adds another function to a book bundle class. Or a distribution providing a default and a specific project extending it.> - A & B extend Node. But they dont extend eachother.
This however is not. This can't work. Both module a and b are expecting their class and their methods to be there, one will be broken. This is similar to two modules replacing a service. One of them is going to be broken: 🐛 Fatal error after installing the module (incompatibility with Context & Menu position module) Needs work . Either subtly in that features just don't work because an instanceof check is FALSE or with a fatal error.A priority system won't solve that. In theory, a decorator pattern could be used, but properly using a decorator pattern is hard and complex and really only works if there is a decorator trait/base class that by default passes all methods through to the inner one, or adding a new class on the parent is going to break.
That's why this should IMHO not be supported and we should fail early with an exception. You can still use an alter hook, that's on you, can't prevent that except in the specific alter hook.