[Contracts][DependencyInjection] Add `ServiceCollectionInterface` #53163

kbond · 2023-12-20T18:28:23Z

Q	A
Branch?	7.1
Bug fix?	no
New feature?	yes
Deprecations?	no
Issues	n/a
License	MIT

I commonly require a service locator which is also an iterator. I noticed during @weaverryan's most recent livestream he could have benefited from this as well. There are currently three ways to achieve:

Decorate a ServiceLocator and use ServiceLocator::getProvidedServices() in the decorator's getIterator()
Decorate a service iterable and add your own ContainerInterface::get() method which iterates over all items until it finds a match (this is what Ryan did in his livestream)
Inject a tagged iterator in addition to your locator.

This PR proposes a new ServiceCollectionInterface which is a countable, iterable, ContainerInterface.

I'm implemented on the current ServiceLocator so any place this is used in your code, you can now type-hint ServiceCollectionInterface:

public function someAction(
    #[TaggedLocator('some.tag')]
    ServiceCollectionInterface $container,
) {
    foreach ($container as $id => $item) {
        // do something with each service/id
    }

    $container->count(); // # of services
    
    $container->get('some.id');
}

src/Symfony/Component/DependencyInjection/ServiceCollectionInterface.php

GromNaN

I commonly require a service locator which is also an iterator.

Could you provide examples where you need the service locator to be iterable. Maybe a link to the video with a point in time where "he could have benefited from this".

src/Symfony/Component/DependencyInjection/ServiceLocator.php

kbond · 2023-12-20T19:24:08Z

Could you provide examples where you need the service locator to be iterable.

Here's a good example: https://github.com/symfony/ux/pull/1347/files#diff-6487ef7c39e106956904930b49958df26d90de6e025cd0c69a1a9a84e31ed064 (the entire class could be replaced with a ServiceLocator). He needs to iterate over a collection of component services for the index page, then, on each component page, he needs to get a specific one.

kbond · 2023-12-20T21:37:31Z

Ok, I've removed the interface and just made ServiceLocator iterable.

derrabus · 2023-12-21T08:55:27Z

There are currently two ways to achieve:

Inject a tagged iterator in addition to your locator.

derrabus · 2023-12-21T09:02:12Z

src/Symfony/Component/DependencyInjection/ServiceLocator.php

+        foreach (array_keys($this->getProvidedServices()) as $id) {
+            yield $id => $this->get($id);
+        }


This basically means that by iterating over the locator you initialize all of its services. To me, this kinda defeats the purpose of a service locator: I could've just injected a tagged iterator and called iterator_to_array() on it.

If we really want to make the locator traversable (not sure if we should yet), maybe we'd rather yield a service closure instead?

but because this is a generator, this still does it kinda lazily so it's OK?

I don't know, this still looks like a pitfall to me. I wouldn't do it.

@kbond WDYT of this concern? I don't feel the need for this extra safety myself but maybe your experience could help best decide?

I don't know, this still looks like a pitfall to me. I wouldn't do it.

Does the new interface alleviate the concern at all?

src/Symfony/Component/DependencyInjection/ServiceLocator.php

nicolas-grekas

The added method is just a helper: userland can do this iteration with any ServiceProviderInterface.

IoC principle would encourage using the interface.

Still worth it?

nicolas-grekas · 2023-12-21T09:47:11Z

symfony/ux#1347 (files) (the entire class could be replaced with a ServiceLocator)

that's quite common needless overhead - we already simplified many components by using a container
this PR is not needed to kill that class :)

kbond · 2023-12-22T01:41:58Z

that's quite common needless overhead - we already simplified many components by using a container
this PR is not needed to kill that class :)

I guess you mean, inject a tagged iterator into the index action and the tagged locator into the show method? Indeed that would work.

The added method is just a helper: userland can do this iteration with any ServiceProviderInterface.

This is true, and in fact what I've been doing, but in userland, ServiceProviderInterface::getProvidedServices() is weird. It's keyed by the service id which, imo, is 95% of the reason to call by end users (the array values aren't important at this stage). It's a bit of a papercut imo as I don't think it's entirely clear this is possible (iterating over all the services).

What about a ServiceProviderInterface::getProvidedServiceIds(): string[] method?

Still worth it?

I do still feel this improves the DX as the need for a service locator/iterator is a common pattern I'm seeing (but let's close if there isn't agreement).

nicolas-grekas · 2023-12-26T15:02:56Z

To make this full "IoC", one can type hint ServiceProviderInteface&Traversable, isn't it? Then LGTM.

kbond · 2023-12-26T16:19:57Z

As discussed with Nicolas on Slack, I've switched back to a ServiceCollectionInterface (in symfony/service-contracts).

nicolas-grekas

Works for me, thanks.
Please update the root composer.json for 3.5 also.

kbond · 2023-12-28T16:10:03Z

Please update the root composer.json for 3.5 also.

Done, but please confirm it's updated correctly.

src/Symfony/Component/DependencyInjection/ServiceLocator.php

chalasr · 2024-01-02T14:18:05Z

Thank you @kbond.

…ctionInterface when declaring a service subscriber (nicolas-grekas) This PR was merged into the 7.2 branch. Discussion ---------- [DependencyInjection] Fix missing binding for ServiceCollectionInterface when declaring a service subscriber | Q | A | ------------- | --- | Branch? | 7.2 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | - | License | MIT Looks like we forgot this in #53163 I figured it out when reviewing symfony/symfony-docs#20961 Commits ------- 6174d09 [DependencyInjection] Fix missing binding for ServiceCollectionInterface when declaring a service subscriber

carsonbot added Status: Needs Review DependencyInjection Feature labels Dec 20, 2023

carsonbot added this to the 7.1 milestone Dec 20, 2023

kbond force-pushed the feat/service-collection-interface branch from b801081 to 18cf8fe Compare December 20, 2023 18:30

stof reviewed Dec 20, 2023

View reviewed changes

src/Symfony/Component/DependencyInjection/ServiceCollectionInterface.php Outdated Show resolved Hide resolved

kbond force-pushed the feat/service-collection-interface branch from 18cf8fe to 6e48c4c Compare December 20, 2023 18:38

ro0NL reviewed Dec 20, 2023

View reviewed changes

src/Symfony/Component/DependencyInjection/ServiceCollectionInterface.php Outdated Show resolved Hide resolved

GromNaN reviewed Dec 20, 2023

View reviewed changes

src/Symfony/Component/DependencyInjection/ServiceLocator.php Outdated Show resolved Hide resolved

kbond force-pushed the feat/service-collection-interface branch from 6e48c4c to ca47fd0 Compare December 20, 2023 21:35

kbond changed the title ~~[DependencyInjection] Introduce ServiceCollectionInterface~~ [DependencyInjection] Make ServiceLocator iterable Dec 20, 2023

kbond force-pushed the feat/service-collection-interface branch from ca47fd0 to 8db521a Compare December 20, 2023 21:43

yceruto approved these changes Dec 21, 2023

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Dec 21, 2023

derrabus reviewed Dec 21, 2023

View reviewed changes

nicolas-grekas reviewed Dec 21, 2023

View reviewed changes

src/Symfony/Component/DependencyInjection/ServiceLocator.php Outdated Show resolved Hide resolved

nicolas-grekas reviewed Dec 21, 2023

View reviewed changes

kbond force-pushed the feat/service-collection-interface branch from 8db521a to 33d14d0 Compare December 26, 2023 16:09

kbond changed the title ~~[DependencyInjection] Make ServiceLocator iterable~~ [DependencyInjection] Add ServiceCollectionInterface Dec 26, 2023

kbond added the Contracts label Dec 26, 2023

carsonbot changed the title ~~[DependencyInjection] Add ServiceCollectionInterface~~ [Contracts][DependencyInjection] Add ServiceCollectionInterface Dec 26, 2023

nicolas-grekas approved these changes Dec 27, 2023

View reviewed changes

kbond force-pushed the feat/service-collection-interface branch from 33d14d0 to a47e84f Compare December 28, 2023 16:09

kbond force-pushed the feat/service-collection-interface branch from a47e84f to 578a99a Compare December 28, 2023 16:34

nicolas-grekas approved these changes Dec 28, 2023

View reviewed changes

chalasr approved these changes Dec 29, 2023

View reviewed changes

xabbuh reviewed Jan 1, 2024

View reviewed changes

src/Symfony/Component/DependencyInjection/ServiceLocator.php Outdated Show resolved Hide resolved

[DependencyInjection] Add ServiceCollectionInterface

4a9e874

kbond force-pushed the feat/service-collection-interface branch from 578a99a to 4a9e874 Compare January 2, 2024 14:07

chalasr merged commit 82e0eff into symfony:7.1 Jan 2, 2024

chalasr mentioned this pull request Jan 2, 2024

[Contracts][DependencyInjection] Add ServiceCollectionInterface symfony/symfony-docs#19368

Closed

fabpot mentioned this pull request May 2, 2024

Release v7.1.0-BETA1 #54820

Merged

nicolas-grekas mentioned this pull request May 15, 2025

[DependencyInjection] Fix missing binding for ServiceCollectionInterface when declaring a service subscriber #60428

Merged

Uh oh!

[Contracts][DependencyInjection] Add ServiceCollectionInterface #53163

[Contracts][DependencyInjection] Add ServiceCollectionInterface #53163

Uh oh!

Conversation

kbond commented Dec 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

GromNaN left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kbond commented Dec 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kbond commented Dec 20, 2023

Uh oh!

derrabus commented Dec 21, 2023

Uh oh!

derrabus Dec 21, 2023

Choose a reason for hiding this comment

Uh oh!

nicolas-grekas Dec 21, 2023

Choose a reason for hiding this comment

Uh oh!

derrabus Dec 21, 2023

Choose a reason for hiding this comment

Uh oh!

nicolas-grekas Dec 26, 2023

Choose a reason for hiding this comment

Uh oh!

kbond Dec 26, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nicolas-grekas left a comment

Choose a reason for hiding this comment

Uh oh!

nicolas-grekas commented Dec 21, 2023

Uh oh!

kbond commented Dec 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nicolas-grekas commented Dec 26, 2023

Uh oh!

kbond commented Dec 26, 2023

Uh oh!

nicolas-grekas left a comment

Choose a reason for hiding this comment

Uh oh!

kbond commented Dec 28, 2023

Uh oh!

Uh oh!

chalasr commented Jan 2, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

[Contracts][DependencyInjection] Add `ServiceCollectionInterface` #53163

[Contracts][DependencyInjection] Add `ServiceCollectionInterface` #53163

kbond commented Dec 20, 2023 •

edited

Loading

kbond commented Dec 20, 2023 •

edited

Loading

kbond commented Dec 22, 2023 •

edited

Loading