Skip to content
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

Injecting all tagged services in a service by variadic constructor #54741

Closed
jmediatechnology opened this issue Apr 26, 2024 · 3 comments
Closed

Comments

@jmediatechnology
Copy link

jmediatechnology commented Apr 26, 2024

Description

Injecting all tagged services to a service is possible by declaring an iterable within the constructor.
Example:

namespace App;

class HandlerCollection
{
    public function __construct(iterable $handlers)
    {
    }
}

The services.yaml might look like this:

services:
    App\Handler\One:
        tags: ['app.handler']

    App\Handler\Two:
        tags: ['app.handler']

    App\HandlerCollection:
        # inject all services tagged with app.handler as first argument
        arguments:
            - !tagged_iterator app.handler

Example code found here:
https://symfony.com/doc/current/service_container/tags.html#reference-tagged-services


In practise my code will look like this:

namespace App;

class ChainService
{
    /**
     * @param iterable<AwesomeService> $awesomeServices
     */
    public function __construct(private iterable $awesomeServices) {}

    public function doSomething(string $string): string
    {
        foreach ($this->awesomeServices as $awesomeService) {
            if ($awesomeService->supports($string)) {
                return $awesomeService->doSomething($string);
            }
        }

        throw new InvalidArgumentException(
            sprintf(
                'Failed to do something with "%s".',
                $string,
            )
        );
    }
}

To extinguish the need of the @param annotation in this case I would like to use a typed variable-length argument lists in the constructor instead of an iterable, like this:

namespace App;

class ChainService
{
    private array $awesomeServices;

    public function __construct(AwesomeService ...$awesomeServices) {}

    public function doSomething(string $string): string
    {
        foreach ($this->awesomeServices as $awesomeService) {
            if ($awesomeService->supports($string)) {
                return $awesomeService->doSomething($string);
            }
        }

        throw new InvalidArgumentException(
            sprintf(
                'Failed to do something with "%s".',
                $string,
            )
        );
    }
}

The tagged_iterator is not able to deal with this code because the Dependency Injection component will try to inject an RewindableGenerator to ChainService which implements an IteratorAggregate, thus triggering a type error.

The usage of variadics in a typed constructor argument is possible when services are wired manually in services.yaml.
For example:

services:
    App\ChainService:
        arguments:
            - '@App\AwesomeService1'
            - '@App\AwesomeService2'
            - '@App\AwesomeService3'

Can you make the tagged_iterator operational for typed variadic constructor arguments?
Or can you introduce something else to make the typed variadic constructor arguments possible?

Example

services:
    App\ChainService:
        arguments:
            - !variadic app.handler
@lyrixx
Copy link
Member

lyrixx commented Apr 28, 2024

If I remember correctly, I already opened a PR that has been refused

See #33722

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Apr 29, 2024

There's a critical difference between both implementations: the non-variadic variant will not instantiate services that are listed after the one that "supports" the thing, while the variadic variant will consume the full iterator.

Personally, this style using variadics is a hack and doesn't need to be supported as such. See how the argument "removing an @param" can lead to effective and nasty runtime side effects.

@nicolas-grekas
Copy link
Member

Closing for the reason given in my last comment.

@derrabus derrabus reopened this May 7, 2024
@derrabus derrabus closed this as not planned Won't fix, can't repro, duplicate, stale May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants