8000 [RFC] [DependencyInjection] Custom container configurators (fluent PHP format) · Issue #25630 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[RFC] [DependencyInjection] Custom container configurators (fluent PHP format) #25630

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.

Sign up for GitHub

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

Jump to bottom
Closed
unkind opened this issue Dec 30, 2017 · 9 comments
Closed

[RFC] [DependencyInjection] Custom container configurators (fluent PHP format) #25630

unkind opened this issue Dec 30, 2017 · 9 comments
Labels
DependencyInjection Feature RFC RFC = Request For Comments (proposals about features that you want to be discussed)

Comments

@unkind
Copy link
Contributor
unkind commented Dec 30, 2017 •
edited
Loading
Q A
Bug report? no
Feature request? yes
BC Break report? no
RFC? yes
Symfony version 4.1 (?)

So far we have extensions for custom and simplified configurations, e.g.

return function (ContainerConfigurator $c) {
    $c->extension('monolog', [
        'handlers' => [
            'main' => [
                'type'  => 'rotating_file',
                'path'  => '%kernel.logs_dir%/%kernel.environment%.log',
                'level' => 'debug',
                // max number of log files to keep
                // defaults to zero, which means infinite files
                'max_files' => 10,
            ],
        ],
    ]);
};

which is fine in many cases, but it doesn't use all capabilities of PHP. I'd be happy to have native custom configurator like the following, for example:

return function (MonologContainerConfigurator $c) {
    $c->handlers()->register(
            'main',
            MonologHandlerType::rotatingFile()
                ->path('%kernel.logs_dir%/%kernel.environment%.log')
                ->maxFiles(10)
        )
        ->level(LoggerInterface::DEBUG);
};

I see at least 3 benefits:

  1. It allows provide self-descriptive configurators, simplifies learning DSL of the extension (by playing with autocomplete options).

  2. Such configurators prevent many typos because we work with methods, not custom arrays. So far only XML has self-validation, but this format is painful for many people.

  3. It helps to introduce your own DSL in your project (e.g. I can add ->anonymous() helper like I suggested in [DependencyInjection] Anonymous services in PHP DSL #24632). @nicolas-grekas mentioned it as possible solution for me:

    It should be quite easy to define your own DSL, by creating a different set of configurator classes, and type-hinting for them in the closure.

    Unfortunately, Symfony doesn't support it out-of-box.

The idea is simple: bundle/extension should be able to register it's own container configurator factory:

$container->registerFluentContainerConfigurator(
    MonologContainerConfigurator::class,
    function (ContainerConfigurator $c) {
        return new MonologContainerConfigurator($c);
    }
);

Then we can detect closure argument type and inject appropriate container configurator.
@unkind unkind changed the title [RFC] Custom container configurators (fluent PHP format) [RFC] [DIC] Custom container configurators (fluent PHP format) Dec 30, 2017
@unkind unkind changed the title [RFC] [DIC] Custom container configurators (fluent PHP format) [RFC] [DependencyInjection] Custom container configurators (fluent PHP format) Dec 30, 2017
@linaori
Copy link
Contributor
linaori commented Dec 30, 2017

Though I really like the idea, won't this be double bookkeeping with the configuration nodes?

@mvrhov
Copy link
mvrhov commented Dec 30, 2017

Maybe then we can deprecate the configuration

@nicolas-grekas
Copy link
Member

I think you can already achieve what you want very easily:

return function (ContainerConfigurator $c) {
    $myBundle = new MyBundleConfigurator($c);

    $myBundle->setWhatever();
}

Nothing required from core. WDYT?

@javiereguiluz javiereguiluz added DependencyInjection Feature RFC RFC = Request For Comments (proposals about features that you want to be discussed) labels Dec 30, 2017
@unkind
Copy link
Contributor Author
unkind commented Dec 30, 2017 •
edited
Loading

I think you can already achieve what you want very easily:

I think it's possible that bundle might want to inject some additional dependencies/parameters.
Also we have about hundred config files in our project, if we want to use our custom configurator we have to copy-paste it many times, it's annoying.

@unkind
Copy link
Contributor Author
unkind commented Dec 30, 2017 •
edited
Loading

Though I really like the idea, won't this be double bookkeeping with the configuration nodes?

There is no violation of DRY if I understand your point correctly. Configuration nodes is attempt to make abstraction layer over all formats, but even in the current state it is leaky abstraction (e.g. there is fixXmlConfig method). It is OK for array-like configs, but fluent PHP is different. They may have similar keywords, nesting, etc., but it doesn't mean they are the same, so it's OK to support them separately. Strictly speaking, all formats (XML, YAML, fluent PHP, ...) are different [bounded] contexts and DRY makes sense within a single bounded context only.

By the way, XML schema is sort of "double bookkeeping" as well if you want.

@unkind unkind mentioned this issue Jan 1, 2018
Closed
@nicolas-grekas
Copy link
Member

Fixed by #27065?

@unkind
Copy link
Contributor Author
unkind commented Apr 26, 2018

Fixed by #27065?

How do I type hint against AcmeProjectConfigurator in the method signature?

I see $myBundle = new MyBundleConfigurator($c);, but I pointed out why it doesn't cover all the cases: #25630 (comment)

@nicolas-grekas
Copy link
Member

Maybe you could extend a base class that would provide the logic?

@unkind
Copy link
Contributor Author
unkind commented Apr 26, 2018 •
edited
Loading

Oh, I see, technically it would work. It's just matter of taste I guess. Anonymous class with extends (vs anonymous function) looks a little bit clumsy, but OK.

Still, I'd implement that feature (#25650) as well, but it's up to you.

@aurimasniekis aurimasniekis mentioned this issue Apr 26, 2018
Merged
fabpot added a commit that referenced this issue Apr 27, 2018
@fabpot
feature #27065 [DI][Routing] Allow invokable objects to be used as PH…
a9d12d2 
…P-DSL loaders (aurimasniekis)

This PR was squashed before being merged into the 4.1-dev branch (closes #27065).

Discussion
----------

[DI][Routing] Allow invokable objects to be used as PHP-DSL loaders

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #26583, #25630
| License       | MIT
| Doc PR        | none

Changed DI/Router PHPFileLoader to check is_object && is_callable instead of instance of Closure

Commits
-------

662ff7e [DI][Routing] Allow invokable objects to be used as PHP-DSL loaders
@fabpot fabpot closed this as completed Apr 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
DependencyInjection Feature RFC RFC = Request For Comments (proposals about features that you want to be discussed)
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@fabpot @javiereguiluz @nicolas-grekas @mvrhov @unkind @linaori
0