8000 [DI] ContainerConfigurator vs ContainerBuilder - how to load services of a namespace, and have access to adding compiler passes. · Issue #35554 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[DI] ContainerConfigurator vs ContainerBuilder - how to load services of a namespace, and have access to adding compiler passes. #35554

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
jwillp opened this issue Feb 1, 2020 · 8 comments
Closed

[DI] ContainerConfigurator vs ContainerBuilder - how to load services of a namespace, and have access to adding compiler passes. #35554

jwillp opened this issue Feb 1, 2020 · 8 comments
Labels
DependencyInjection Stalled

Comments

@jwillp
Copy link
jwillp commented Feb 1, 2020 •
edited
Loading

Description
I am facing an issue that I can't seem to resolve nicely.
The code I am working on uses a series of "modules".
Each module has a ModuleConfigurator class that is responsible for configuring the module and its dependencies.

The way these dependencies are configured might depend on complex logic, which is better handled by using PHP.
The way they are loaded looks as follows:

// Kernel.php

protected function configureContainer(ContainerBuilder $container, LoaderInterface $loader): void
{
    CoreModuleConfigurator::configure($container, $loader);
    OtherModuleConfigurator::configure($container, $loader);
}

The issues I am facing are:

  • Given I have a ContainerBuilder, how can I load multiple services of a namespace at once?

The container builder does not provide something like this. Also, it does not provide a way to specify defaults. Meaning I would be better off using a ContainerConfigurator. Fine with me, but as per the documentation the only way is by using a PhpFileLoader that needs a script that returns a function, but I already have a class for this. So, I guess a workaround would be by providing a callback to my static method? But this leads me to my second issue:

  • Given I managed to pass a ContainerConfigurator to my ModuleConfigurator, how can I add compiler passes?

The ContainerConfigurator does not provide a way to add a Compiler Pass to the underlying container. It does not give access to the container builder at all.

I could wire things up so I can pass both the ContainerConfigurator and the ContainerBuilder, but that seems a little repetitive given the ContainerConfigurator has access to the container.

Suggestion
I have seen that in the latest version on master 5.1.0 not released yet, the KernelTraits now allows to type hint the configureContainer method with either a ContainerBuilder or a ContainerConfigurator. Which is pretty nice by the way, but would mean from what I understand so far, that:

  • If I want to load multiple services at once in PHP, I need the ContainerConfigurator
  • If I want to add compiler passes, I need the ContainerBuilder
  • If I want to do both -> Do I need both? Or is there another simpler way?

What I could see, would be to expose the ContainerBuilder using a getContainerBuilder method on the ContainerConfigurator.

Maybe, I am completely off track, but I haven't seen anything in the documentation, that would address my use case.
@jwillp jwillp changed the title ContainerConfigurator vs ContainerBuilder - how to load services of a namespace, and have access to adding compiler passes. [DI] ContainerConfigurator vs ContainerBuilder - how to load services of a namespace, and have access to adding compiler passes. Feb 1, 2020
@nicolas-grekas
Copy link
Member
nicolas-grekas commented Feb 1, 2020 •
edited
Loading

You need a bundle apparently, that's exactly what they are for.
Or if this is only for a specific app, you might want to extend some methods of your Kernel instead.

@jwillp
Copy link
Author
jwillp commented Feb 1, 2020 •
edited
Loading

@nicolas-grekas Hi Nicolas, thank you for answering.
Yes, it is for a single app. Where each module cannot be reused by other apps.
But aren't bundles working similarly to the exposed methods of the kernel?
As in, from what I can see the way to load configurations is the same, I don't see any bundle method exposing a ContainerConfigurator.

Also, the bundle approach would not work for projects that use the DI component as a standalone library outside of Symfony.

I partially managed to do it this way:

protected function loadNamespace(ContainerBuilder $container, Definition $templateDefinition, string $namespace, string $namespacePath): void {
    $loader = new PhpFileLoader($container, New FileLocator($namespacePath));
    $loader->registerClasses($templateDefinition, $namespace, $namespacePath . '/**');
} my que

But it does not load subnamespaces, only the top level one.

EDIT: For the code snippet above I got it to work, removing the /** pattern. But my question remains. :)

@TomasVotruba
Copy link
Contributor

@jwillp You might like local packages approach we use on our Czech PHP community website.

There is ~15 local packages with own config and namespace: https://github.com/pehapkari/pehapkari.cz/tree/master/packages

This is how Kernel loads them: https://github.com/pehapkari/pehapkari.cz/blob/bf29677bf20c022ae6a458aecd2921bf3dcb20f5/src/PehapkariKernel.php#L47-L57

Resources

@carsonbot
Copy link

Thank you for this issue.
There has not been a lot of activity here for a while. Has this been resolved?

@carsonbot
Copy link

Friendly reminder that this issue exists. If I don't hear anything I'll close this.

@nicolas-grekas
Copy link
Member

Reading this again, maybe we could add addCompilerPass() to ContainerConfigurator?
Up for a PR @jwillp?

@carsonbot carsonbot removed the Stalled label Feb 18, 2021
@jwillp
Copy link
Author
jwillp commented Feb 24, 2021

Hi @nicolas-grekas, Yes that would be nice solution!
I will look into it and submit a PR.

This was referenced Mar 3, 2021
Closed
Open
@carsonbot
Copy link
8000

Thank you for this issue.
There has not been a lot of activity here for a while. Has this been resolved?

@nicolas-grekas nicolas-grekas mentioned this issue Sep 13, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
DependencyInjection Stalled
Projects
None yet
Milestone
No milestone
5 participants
@javiereguiluz @nicolas-grekas @TomasVotruba @jwillp @carsonbot
0