Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[HttpKernel] Simplifying Bundle/Extension config definition#43701
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
571cb70 to302b564Comparero0NL commentedOct 25, 2021
yceruto commentedOct 25, 2021
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 |
wouterj commentedOct 25, 2021
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 |
ro0NL commentedOct 25, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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)? |
yceruto commentedOct 25, 2021
I updated the implementation to remove the Interface dependency, so it's only about using the |
872e995 toaa65319Compareb892742 to8f84f84Compareyceruto commentedOct 26, 2021
|
Uh oh!
There was an error while loading.Please reload this page.
2c822d0 to1c56490Compareyceruto commentedMar 11, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I confirmed the usefulness of Update!
This is ready for review again. |
Uh oh!
There was an error while loading.Please reload this page.
nicolas-grekas left a comment
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.
One more round of review :)
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/Config/Definition/ConfigurableInterface.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/Config/Definition/Configurator/DefinitionConfigurator.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/Config/Definition/Configurator/DefinitionConfigurator.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
914defd to4118c39Comparesrc/Symfony/Component/DependencyInjection/Extension/ConfigurableExtensionInterface.phpShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
fabpot commentedMar 30, 2022
Thank you@yceruto. |
…ceruto, wouterj)This PR was merged into the 6.1 branch.Discussion----------[DI] Documenting Abstract Bundle and ExtensionFixessymfony#16669PRsymfony/symfony#43701ping `@Nyholm` `@wouterj` as you were interested in this feature/doc. Any suggestion to improve the wording is more than welcome!---I also updated other places to be aligned with the best practices doc.I didn't remove the current convention to create bundles/extensions/configurations because they are still valid until 6.4 becomes the last LTS available. However, it's ok for apps using AbstractExtension.Commits-------7a24f08 typoc426dbb Fix extension/config pathsc85de08 Revert attribute route558b02e Rewrite the bundle docs a bit more7cf9bf4 Add warningd0cba72 Remove note after update the featurec2c47a2 Update bundle.rstc93db0b Jules' review426e289 Documenting Abstract Bundle and Extension
Uh oh!
There was an error while loading.Please reload this page.
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.phpclass to define the bundle config.DependencyInjection/FooExtension.phpextension class and extend fromExtensionExtensionInterface::load()method to implement we have to:Configuration,Processor, etc.*FileLoader&FileLocatorinstances to import services definition (have to deal with bundle path)PrependExtensionInterface.Bundle::$nameto 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
AbstractBundleto 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 fluentContainerConfiguratorhelper: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$configargument is the bundle'sConfigurationthat 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
PrependExtensionInterfacethat you normally implement on extension classes. But using this method has bonus points, you can now use theContainerConfiguratorto append an extension config from an external file in any format (including the new PHP fluent-config feature).Another improvement is about
Configurationdefinition. Here you can manage it directly within the bundle class using theconfigure()method with new possibilities:You don't have to create the
TreeBuilderinstance yourself anymore and remember the proper extension alias. Instead, you will use a newDefinitionConfiguratorwith the possibility to import configuration definitions from an external PHP file, and this config file can now be placed 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
AbstractBundleclass: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
AbstractExtensionclass 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.