New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[HttpKernel] Simplifying Bundle/Extension config definition #43701
Conversation
Hi @ro0NL I did, at first glance it makes sense, let me think a bit more about it.
Yep, I didn't want to add a new interface, but it should be simpler than working around the extension interface yourself. Also, because I needed a chance to instantiate the An alternative I was thinking about is creating a new |
Quick note (that doesn't invalidate any of this PR - I like the way this PR heads into): while these are based on "convention", they are all configured within the |
Perhaps just a MicroBundleTrait is sufficient ... to shortcut as done in #43080 But i'd consider this a final step :) I mean, why aim for "bundle extends extension", rather than composition (using anon. classes)? |
I updated the implementation to remove the Interface dependency, so it's only about using the |
872e995
to
aa65319
Compare
b892742
to
8f84f84
Compare
|
src/Symfony/Component/Config/Definition/Loader/PhpFileLoader.php
Outdated
Show resolved
Hide resolved
FYI: This new feature is already available for Symfony 5.4 in this separate package https://github.com/yceruto/micro-symfony. Also, with I will update this proposal to include |
src/Symfony/Component/Config/Definition/Configurator/DefinitionConfigurator.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Config/Definition/Loader/DefinitionFileLoader.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Config/Definition/Loader/DefinitionFileLoader.php
Outdated
Show resolved
Hide resolved
Thanks @nicolas-grekas for your review.
I'm not completely sure about |
I do like the fact that MicroBundle removes a lot of the boilerplate code.
Just a small question on a thing I haven’t seen before.
2c822d0
to
1c56490
Compare
I confirmed the usefulness of Update!
This is ready for review again. |
src/Symfony/Component/Config/Definition/ConfigurableInterface.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Config/Definition/Configurator/DefinitionConfigurator.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Config/Definition/Configurator/DefinitionConfigurator.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/DependencyInjection/Extension/MicroExtension.php
Outdated
Show resolved
Hide resolved
914defd
to
4118c39
Compare
interface ConfigurableExtensionInterface extends ConfigurableInterface | ||
{ | ||
/** | ||
* Allow an extension to prepend the extension configurations. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* Allow an extension to prepend the extension configurations. | |
* Allows an extension to prepend the extension configurations. |
Thank you @yceruto. |
This PR aims to simplify DI extension/configuration definitions at the Bundle level (based on @Nyholm #40259 (comment))
Currently, the services and configuration definitions have to deal with some conventions:
DependencyInjection/
directoryDependencyInjection/Configuration.php
class to define the bundle config.DependencyInjection/FooExtension.php
extension class and extend fromExtension
ExtensionInterface::load()
method to implement we have to:Configuration
,Processor
, etc.*FileLoader
&FileLocator
instances to import services definition (have to deal with bundle path)PrependExtensionInterface
.Bundle::$name
to change the extension alias.Although it may not be a big problem to follow all these conventions (indeed, we have been doing it for years) it's true that there are limitations and it requires extra work to achieve them.
Note: The following improvements don't pretend to deprecate the actual convention (at least so far) but simplify it with some benefits.
To start using the following improvements your bundle must extend from the new abstract class
AbstractBundle
to autoconfigure all hooks and make this possible inside a bundle class.The first improvement offers the possibility to configure your bundle DI extension within the bundle class itself using
loadExtension()
method and the fluentContainerConfigurator
helper:This new method
loadExtension()
(a same goal thatExtensionInterface::load()
) contains now all new benefits you currently love for service definition/import/etc. Keep in mind that this configurator still works with a temporal container, so you can't access any extension config at this point (as before). And, the$config
argument is the bundle'sConfiguration
that you usually process onExtensionInterface::load()
but here it's given to you already merged and processed (ready to use).The next improvement comes when you want to prepend/append an extension config before all extensions are loaded & merged, then use the
prependExtension()
method:This is the improved alternative to
PrependExtensionInterface
that you normally implement on extension classes. But using this method has bonus points, you can now use theContainerConfigurator
to append an extension config from an external file in any format (including the new PHP fluent-config feature).Another improvement is about
Configuration
definition. Here you can manage it directly within the bundle class using theconfiguration()
method with new possibilities:You don't have to create the
TreeBuilder
instance yourself anymore and remember the proper extension alias. Instead, you will use a newDefinitionConfigurator
with the possibility to import configuration definitions from an external PHP file, and this config file can now live outside thesrc/
directory of the bundle if desired:And why not, you could also split your definition into several files if it's too long, or simply define the config directly in the method if it's short.
Last but not least you can change the extension alias by redefining a new property that now belongs to the MicroBundle class:
The default alias will be determined from your bundle name (in this case
acme_foo
), so the new way allows you to change that alias without either touching your bundle name or overriding any method.Note: The same feature has been implemented in a new
AbstractExtension
class for those applications applying the bundle-less approach and want to define configuration through an extension.Combining all these benefits I believe we gain a more simplified bundle structure while decreasing the learning curve.