Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[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

Merged
fabpot merged 1 commit intosymfony:6.1fromyceruto:feature/micro_bundle
Mar 30, 2022

Conversation

@yceruto
Copy link
Member

@ycerutoyceruto commentedOct 25, 2021
edited
Loading

QA
Branch?6.1
Bug fix?no
New feature?yes
Deprecations?no
TicketsFix#40259,#42647,#43080
LicenseMIT
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 theDependencyInjection/ directory
  • Create theDependencyInjection/Configuration.php class to define the bundle config.
  • Create theDependencyInjection/FooExtension.php extension class and extend fromExtension
  • In theExtensionInterface::load() method to implement we have to:
    • Process the bundle configuration yourselfConfiguration,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 implementingPrependExtensionInterface.
  • RedefineBundle::$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 classAbstractBundle 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 usingloadExtension() method and the fluentContainerConfigurator helper:

class FooBundleextends AbstractBundle{publicfunctionloadExtension(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 methodloadExtension() (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 theprependExtension() method:

class FooBundleextends AbstractBundle{publicfunctionprependExtension(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 toPrependExtensionInterface 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 aboutConfiguration definition. Here you can manage it directly within the bundle class using theconfigure() method with new possibilities:

class FooBundleextends AbstractBundle{publicfunctionconfigure(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 theTreeBuilder 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 be placed outside thesrc/ directory of the bundle if desired:

// Acme/FooBundle/config/definition.phpuseSymfony\Component\Config\Definition\Configurator\DefinitionConfigurator;returnstaticfunction (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 theAbstractBundle class:

class AcmeFooBundleextends AbstractBundle{protectedstring$extensionAlias ='foo';// alias used during the extension config loading// ...}

The default alias will be determined from your bundle name (in this caseacme_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 newAbstractExtension 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.

andrewmy, ro0NL, garak, alexander-schranz, kaznovac, ismail1432, ruudk, kbond, GromNaN, chalasr, and 9 more reacted with thumbs up emojiismail1432, ruudk, yceruto, tacman, apfelbox, jschaedl, and ging-dev reacted with hooray emojiro0NL, ging-dev, kaznovac, ruudk, kbond, Nyholm, ju1ius, zmitic, sstok, apfelbox, and 2 more reacted with heart emojikaznovac, ismail1432, ruudk, Koc, rosier, apfelbox, jschaedl, ging-dev, and yceruto reacted with rocket emoji
@carsonbotcarsonbot added Status: Needs Review DependencyInjection Deprecation DXDX = Developer eXperience (anything that improves the experience of using Symfony) Feature HttpKernel labelsOct 25, 2021
@carsonbotcarsonbot added this to the5.4 milestoneOct 25, 2021
@carsonbotcarsonbot changed the title[HttpKernel][DependencyInjection][DX] Simplifying DI extension/config definition[DependencyInjection][HttpKernel] Simplifying DI extension/config definitionOct 25, 2021
@ro0NL
Copy link
Contributor

I like this goal :) did you see#42754 also? I believe we can get rid of a lot of boilerplate generally, making everything first-class.

I tend to find the wiring as done in#43080 rather elegant, eg. no need for a newMicroBundleInterface IMHO.

@yceruto
Copy link
MemberAuthor

I like this goal :) did you see#42754 also? I believe we can get rid of a lot of boilerplate generally, making everything first-class.

Hi@ro0NL I did, at first glance it makes sense, let me think a bit more about it.

I tend to find the wiring as done in#43080 rather elegant, eg. no need for a new MicroBundleInterface IMHO.

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 theContainerConfigurator. From the DX point of view, it's faster and intuitive.

An alternative I was thinking about is creating a newExtensionBundle orMicroBundle subclass ofBundle, so we could get rid of this new interface and the required trait as well.

@wouterj
Copy link
Member

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 theBundle andXxxExtension classes themselves. I.e. you can customize both configuration and extension class name(space), see e.g. the SecurityBundle of Symfony (which usesMainConfiguration).

@ro0NL
Copy link
Contributor

ro0NL commentedOct 25, 2021
edited
Loading

From the DX point of view, it's faster and intuitive.

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
Copy link
MemberAuthor

I updated the implementation to remove the Interface dependency, so it's only about using theMicroBundleTrait. It feels better now :)

@ycerutoycerutoforce-pushed thefeature/micro_bundle branch 10 times, most recently from872e995 toaa65319CompareOctober 26, 2021 03:39
@ycerutoycerutoforce-pushed thefeature/micro_bundle branch 2 times, most recently fromb892742 to8f84f84CompareOctober 26, 2021 04:39
@yceruto
Copy link
MemberAuthor

  • I updated the proposal, now it's specific toMicroBundleTrait only
  • Basic tests added

@ycerutoyceruto changed the title[HttpKernel] Simplifying Bundle extension/config definition[HttpKernel] Simplifying Bundle/Extension config definitionMar 11, 2022
@ycerutoycerutoforce-pushed thefeature/micro_bundle branch 2 times, most recently from2c822d0 to1c56490CompareMarch 11, 2022 23:48
@yceruto
Copy link
MemberAuthor

yceruto commentedMar 11, 2022
edited
Loading

I confirmed the usefulness ofMicroExtension class with other developers, it's especially useful for those applications applying the bundle-less approach. They still need to define an extension to create the app configuration (probably more than one definition in big projects), so here we go with the same benefits asMicroBundle.

Update!

  • AddedMicroExtension class
  • DeprecatedConfigurableExtension class in favor ofMicroExtension
  • Refactoring solution to avoid code duplication

This is ready for review again.

Copy link
Member

@nicolas-grekasnicolas-grekas left a 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 :)

yceruto reacted with heart emoji
@ycerutoycerutoforce-pushed thefeature/micro_bundle branch 3 times, most recently from914defd to4118c39CompareMarch 27, 2022 10:49
@fabpot
Copy link
Member

Thank you@yceruto.

yceruto and GromNaN reacted with hooray emoji

@fabpotfabpot merged commit4e6b803 intosymfony:6.1Mar 30, 2022
@ycerutoyceruto deleted the feature/micro_bundle branchMarch 30, 2022 22:46
@fabpotfabpot mentioned this pull requestApr 15, 2022
javiereguiluz added a commit to javiereguiluz/symfony-docs that referenced this pull requestMay 27, 2022
…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
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@GromNaNGromNaNGromNaN left review comments

@NyholmNyholmNyholm left review comments

@fabpotfabpotfabpot approved these changes

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

+2 more reviewers

@ro0NLro0NLro0NL left review comments

@ogizanagiogizanagiogizanagi left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

DXDX = Developer eXperience (anything that improves the experience of using Symfony)FeatureHttpKernelStatus: Reviewed

Projects

None yet

Milestone

6.1

Development

Successfully merging this pull request may close these issues.

[RFC][DX] Add possibility to provide bundle configuration (definition) inside the bundle class

9 participants

@yceruto@ro0NL@wouterj@fabpot@nicolas-grekas@GromNaN@Nyholm@ogizanagi@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp