-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[RFC][DX] Add possibility to provide bundle configuration (definition) inside the bundle class #40259
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
Comments
What about |
Sorry, but this could easily lead to configuration becoming more magic and less understandable, as using more php instead of declarative languages (yaml, xml) allows developers to embed more logic in the "creation of the final configuration values". |
Conceptually, I have always felt that AppBundle had basically been replaced with src/Kernel.php. So besides a configureContainer method, it might be interesting to add a configureRoutes(RoutingConfigurator $routes) method as well. You could even go crazy and allow implementing things like the CompilerPassInterface. |
I might be slightly off topic, but here is an idea: Currently, to create a bundle, you need a You could however make this process a bit simpler by adding everything in the namespace Acme;
use Symfony\Component\Config\Definition\Builder\TreeBuilder;
use Symfony\Component\Config\Definition\ConfigurationInterface;
use Symfony\Component\Config\Definition\Processor;
use Symfony\Component\Config\FileLocator;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Extension\ConfigurationExtensionInterface;
use Symfony\Component\DependencyInjection\Extension\ExtensionInterface;
use Symfony\Component\HttpKernel\Bundle\Bundle;
use Symfony\Component\DependencyInjection\Loader;
class AcmeHelloBundle extends Bundle implements ExtensionInterface, ConfigurationExtensionInterface, ConfigurationInterface
{
public function getConfigTreeBuilder()
{
$treeBuilder = new TreeBuilder($this->getAlias());
$root = $treeBuilder->getRootNode();
$root
->ch
8000
ildren()
->scalarNode('some_option')->isRequired()->cannotBeEmpty()->end()
->end();
return $treeBuilder;
}
public function load(array $rawConfigs, ContainerBuilder $container)
{
$processor = new Processor();
$config = $processor->processConfiguration($this, $rawConfigs);
$loader = new Loader\YamlFileLoader($container, new FileLocator(__DIR__.'/config'));
$loader->load('services.yaml');
// Do something with $config['some_option']
// Add service definitions
}
public function getConfiguration(array $config, ContainerBuilder $container)
{
return $this;
}
public function getContainerExtension()
{
return $this;
}
public function getXsdValidationBasePath()
{
return false;
}
public function getAlias()
{
return 'acme_hello';
}
} This is okey "light" bundles. For most bundles they can be just simple libraries + recipes and for "heavy" bundles, you probably want the |
If I recall correctly, loading code even if not executed adds an small overhead on each requests (it might have changed with preloading) and since the bundles classes are loaded on each requests. This means that if we go this route, the configuration and building code of bundles will be loaded on every requests, even when it's not needed because the container is already built. |
Excellent observation. You are correct. The I have not measured the impact, but Im sure it is really neglectable. And as you mention, the time penalty will go away with with preloading. Also note (in case it was not obvious), the code I posted is already working with sf 5.2 and most likely with sf 4.4. |
@gggeek Magic is all perspective. The way Symfony does configuration is problematic. Unless I am missing something, In the bundle, I can easily configure services/parameters but not routes or configuration. There are several use cases that I have where it would be much better to import routes from bundles so that I can manage the route configuration at the bundle level. This is especially helpful when using locale based route configurations like
Routes in bundles are also super helpful for managing host for subdomain usage for multi-tenant, api, dev, mobile version, etc. For config, I would like to configure Twig and Doctrine, from within bundles as well. /bundle//Resources/config/packages/Twig.yml would be appended to config/packages/Twig.yml. This is similar to how templates are overwritten but in this case it is an append. It is Symfony, I think configuration options are great including php, yaml, xml! I would like to add a configuration provider so we could store configuration in the database and I could alter configurations using SQL. If I happen to be missing either of these options (config and routes in bundles) and (configurations stores in DB similar to a user provider) then please. let me know! Until then, I agree with @mahono that we need to move towards best practices, because what we have right now is not it in my opinion. |
This is by design: routes belong to the "public API" of web apps. Not to bundles. Bundles should not create routes on their own - the dev should wire the routes for a bundles. Recipes provides working routes out-of-the-box, which is the best DX to me.
Since Symfony's container is compiled, it'd mean finding a way to connect to the DB before compiling the container. Since the DB connection is defined as a service, there is a chicken-n-egg situation. But this is going a bit off topic :) |
@nicolas-grekas sorry for introducing the off-topic topic ;-) I have a tendency to do that in comments to PRs as those provide an extremely convenient, universally available way to give feedback (thanks github for that btw!). |
@gggeek you can start new discussions (concrete & abstract) by creating new issues (https://github.com/symfony/symfony/issues/new/choose) or commenting on issues that are about the topic already. Or join the Symfony slack if you want to have a more conversation-type discussion within the community. Anyway, back to the topic here. I think @Nyholm already showed that bundle classes are already able to provide DI configuration. Is there anything to do here? |
This comment has been minimized.
This comment has been minimized.
FYI with #43701 you can manage the DI extension of the bundle within the bundle class itself with all the new stuff you currently like and more: class AcmeFooBundle extends MicroBundle
{
public function configuration(DefinitionConfigurator $definition): void
{
$definition->import('../config/definition.php');
}
public function prependExtension(ContainerConfigurator $container, ContainerBuilder $builder): void
{
$container->import('../config/packages/cache.php');
}
public function loadExtension(array $config, ContainerConfigurator $container, ContainerBuilder $builder): void
{
$container->import('../config/services.php');
}
} Further, you can load Config definitions and extension configs from an external file, or if desired, everything can be defined in place. |
…ition (yceruto) This PR was merged into the 6.1 branch. Discussion ---------- [HttpKernel] Simplifying Bundle/Extension config definition | Q | A | ------------- | --- | Branch? | 6.1 | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | Fix #40259, #42647, #43080 | License | MIT | Doc PR | - 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: * Create the `DependencyInjection/` directory * Create the `DependencyInjection/Configuration.php` class to define the bundle config. * Create the `DependencyInjection/FooExtension.php` extension class and extend from `Extension` * In the `ExtensionInterface::load()` method to implement we have to: * Process the bundle configuration yourself `Configuration`, `Processor`, etc. * Create the specific `*FileLoader` & `FileLocator` instances to import services definition (have to deal with bundle path) * Prepend/append configs for other extensions requires implementing `PrependExtensionInterface`. * Redefine `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 fluent `ContainerConfigurator` helper: ```php class FooBundle extends AbstractBundle { public function loadExtension(array $config, ContainerConfigurator $container, ContainerBuilder $builder): void { $container->parameters() ->set('foo', $config['foo']); $container->import('../config/services.php'); if ('bar' === $config['foo']) { $container->services() ->set(Parser::class); } } } ``` This new method `loadExtension()` (a same goal that `ExtensionInterface::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's `Configuration` that you usually process on `ExtensionInterface::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: ```php class FooBundle extends AbstractBundle { public function prependExtension(ContainerConfigurator $container, ContainerBuilder $builder): void { // prepend $builder->prependExtensionConfig('framework', [ 'cache' => ['prefix_seed' => 'foo/bar'], ]); // append $container->extension('framework', [ 'cache' => ['prefix_seed' => 'foo/bar'], ]) // append from file $container->import('../config/packages/cache.php'); } } ``` 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 the `ContainerConfigurator` 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 the `configuration()` method with new possibilities: ```php class FooBundle extends AbstractBundle { public function configure(DefinitionConfigurator $definition): void { // loads config definition from a file $definition->import('../config/definition.php'); // loads config definition from multiple files (when it's too long you can split it) $definition->import('../config/definition/*.php'); // defines config directly when it's short $definition->rootNode() ->children() ->scalarNode('foo')->defaultValue('bar')->end() ->end() ; } } ``` You don't have to create the `TreeBuilder` instance yourself anymore and remember the proper extension alias. Instead, you will use a new `DefinitionConfigurator` with the possibility to import configuration definitions from an external PHP file, and this config file can now live outside the `src/` directory of the bundle if desired: ```php // Acme/FooBundle/config/definition.php use Symfony\Component\Config\Definition\Configurator\DefinitionConfigurator; return static function (DefinitionConfigurator $definition) { $definition->rootNode() ->children() ->scalarNode('foo')->defaultValue('bar')->end() ->end() ; }; ``` 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: ```php class AcmeFooBundle extends AbstractBundle { protected string $extensionAlias = 'foo'; // alias used during the extension config loading // ... } ``` 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. Commits ------- 7e8cf5d Simplifying bundle extension/config definition
…ition (yceruto) This PR was merged into the 6.1 branch. Discussion ---------- [HttpKernel] Simplifying Bundle/Extension config definition | Q | A | ------------- | --- | Branch? | 6.1 | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | Fix #40259, #42647, symfony/symfony#43080 | License | MIT | Doc PR | - This PR aims to simplify DI extension/configuration definitions at the Bundle level (based on @Nyholm symfony/symfony#40259 (comment)) Currently, the services and configuration definitions have to deal with some conventions: * Create the `DependencyInjection/` directory * Create the `DependencyInjection/Configuration.php` class to define the bundle config. * Create the `DependencyInjection/FooExtension.php` extension class and extend from `Extension` * In the `ExtensionInterface::load()` method to implement we have to: * Process the bundle configuration yourself `Configuration`, `Processor`, etc. * Create the specific `*FileLoader` & `FileLocator` instances to import services definition (have to deal with bundle path) * Prepend/append configs for other extensions requires implementing `PrependExtensionInterface`. * Redefine `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 fluent `ContainerConfigurator` helper: ```php class FooBundle extends AbstractBundle { public function loadExtension(array $config, ContainerConfigurator $container, ContainerBuilder $builder): void { $container->parameters() ->set('foo', $config['foo']); $container->import('../config/services.php'); if ('bar' === $config['foo']) { $container->services() ->set(Parser::class); } } } ``` This new method `loadExtension()` (a same goal that `ExtensionInterface::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's `Configuration` that you usually process on `ExtensionInterface::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: ```php class FooBundle extends AbstractBundle { public function prependExtension(ContainerConfigurator $container, ContainerBuilder $builder): void { // prepend $builder->prependExtensionConfig('framework', [ 'cache' => ['prefix_seed' => 'foo/bar'], ]); // append $container->extension('framework', [ 'cache' => ['prefix_seed' => 'foo/bar'], ]) // append from file $container->import('../config/packages/cache.php'); } } ``` 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 the `ContainerConfigurator` 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 the `configuration()` method with new possibilities: ```php class FooBundle extends AbstractBundle { public function configure(DefinitionConfigurator $definition): void { // loads config definition from a file $definition->import('../config/definition.php'); // loads config definition from multiple files (when it's too long you can split it) $definition->import('../config/definition/*.php'); // defines config directly when it's short $definition->rootNode() ->children() ->scalarNode('foo')->defaultValue('bar')->end() ->end() ; } } ``` You don't have to create the `TreeBuilder` instance yourself anymore and remember the proper extension alias. Instead, you will use a new `DefinitionConfigurator` with the possibility to import configuration definitions from an external PHP file, and this config file can now live outside the `src/` directory of the bundle if desired: ```php // Acme/FooBundle/config/definition.php use Symfony\Component\Config\Definition\Configurator\DefinitionConfigurator; return static function (DefinitionConfigurator $definition) { $definition->rootNode() ->children() ->scalarNode('foo')->defaultValue('bar')->end() ->end() ; }; ``` 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: ```php class AcmeFooBundle extends AbstractBundle { protected string $extensionAlias = 'foo'; // alias used during the extension config loading // ... } ``` 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. Commits ------- 7e8cf5df10 Simplifying bundle extension/config definition
Description
With all the recent changes towrds configuring things using PHP I wonder why not providing the possibility to provide bundle configuration directly inside the bundle class. Currently developers need to create an extension class and then a Resources/services.php file to return the configuration definition.
I think it could be a great improvement to provide an empty method in Symfony\Component\HttpKernel\Bundle\Bundle that can be overridden to just return that same configuration you would return in the Resources/services.php file.
Example
From developer perspective it could look like this in a bundle class:
Or I'm not 100% sure how the system works maybe the method needs to return Closure to work with the container building system.
I just wanted to report this idea. :o)
The text was updated successfully, but these errors were encountered: