- 🇦🇺Australia mstrelan
#3 might not be necessary if we adopted something like https://github.com/olvlvl/composer-attribute-collector.
- @mstrelan opened merge request.
- Status changed to Needs work
over 1 year ago 2:34am 30 October 2023 - 🇦🇺Australia mstrelan
Needs work for @todo comment in system_entity_bundle_info_alter
- Status changed to Needs review
over 1 year ago 3:10am 30 October 2023 - Status changed to Needs work
over 1 year ago 4:08pm 6 November 2023 - 🇺🇸United States smustgrave
Left a comment on the MR.
But new service seems like something that could use a CR, examples always welcome.
- 🇩🇪Germany geek-merlin Freiburg, Germany
Also, after having groked hook attribute discovery in a compoler pass, this should also run in a compiler pass (not as plugins).
- 🇺🇸United States dww
Nice! Love it. Not sure how I missed this issue, but glad to see it mentioned in Slack just now.
- 🇦🇺Australia mstrelan
@geek-merlin any chance you'd be able to put up an MR with that approach, even if it's only a WIP / POC?
- 🇩🇪Germany geek-merlin Freiburg, Germany
I doubt that i'll have time soon-ish, but how i'd approach it:
- In ServiceProvider, register a CompilerPass
- In CompilerPass, iterate classes in the blessed directory, check attributes, then set the collected classes as a constructor parameter of BundleClassAltererHooks service
- In BundleClassAltererHooks, alter the bundles - 🇨🇭Switzerland berdir Switzerland
With Hooks, we now have an implementation in core that doesn't use plugin discovery to find stuff, we could something like that here as well? Could then also be inlined into the EntityTypeBundleInfo, that would also make it clear that you're not meant to use this directly as an API.
The discovery should probably make sure that it's not possible to register multiple classes for the same bundle. Can also happen with the alter hook, where it's much harder to detect, but at least here we could?
What about subclasses? Lets say a contrib module defines a node type and adds a bundle class for it. In a custom project you want to subclass and change that. I guess you still have the option to use the alter hook then and set it there.
- 🇦🇺Australia dpi Perth, Australia
The discovery should probably make sure that it's not possible to register multiple classes for the same bundle. Can also happen with the alter hook, where it's much harder to detect, but at least here we could?
What about subclasses? Lets say a contrib module defines a node type and adds a bundle class for it. In a custom project you want to subclass and change that. I guess you still have the option to use the alter hook then and set it there.
Maybe you'd be interested in reviewing ✨ Add logic to determine which bundle class should take effect when multiple classes extend the same bundle Active and associated PR.
- 🇦🇺Australia mstrelan
Added a new branch (MR !11545) that adds a container pass for collecting bundle classes and registers them in EntityTypeBundleInfo. It's very rough, so I'm looking for advice on how to improve it, e.g. with caching and just general improvements, as well as pointing out everything I did wrong.