Add optional factory/extension interfaces and dependency inspection

[mindplay-dk]

Preliminary PR under discussion here.

[XedinUnknown]

So, to summarize my current point from the original discussion.

According to my understanding, factories and extensions are both service definitions, and therefore perhaps ServiceDefinitionInterface makes sense for the root type. I think we all agree that it shouls have a getDependencies(): string[] method. Now that it represents a service definition (which both factories and extenisons are), and we know that a service definition is described by being a callable of form (ContainerInterface $c) => mixed, it seems to me that it should also have an __invoke(ContainerInterface $c): mixed. This plays well with the fact that extension definitions differ from factories only by the additional optional $prev = null parameter, as it allows us to have that common type without breaking LSP.

With regard to not declaring __invoke() in ServiceDefintionInterface (gonna be referring to it that way for now), and instead declaring it independently in FactoryInterface and ExtensionInterface. If we are talking about a ServiceDefinitionInterface, this implies that it describes a service defintion, and a service definition is first and foremost described with the unit of code defines the service, and therefore it should have __invoke() IMHO.

The other debatable point was ServiceDefinitionInterface::withDependencies(string[] $serviceNames): self. I agree that it should not be a part of this standar, but also that it should be a part of an interop standard, because this enables multi-boxing. I think the easiest solution for now could be to publish a method like that withh an interface like ConfigurableServiceDefinitionInterface in Dhii, and see how that goes. If this shows promise, we can propose a PSR later on, and handle that separately, in the way that is deemed appropriate by the FIG, whether it is by adding a new PSR, extending the existing one, or simply keeping it out of the PSR space and leaving it in Dhii or elsewhere.

[mecha]

extension definitions differ from factories only by the additional optional $prev = null parameter

I had either forgotten or never bothered to care about the fact that $prev is optional. This means every extension I've ever written does not adhere to the spec lol 🙊

Anyway ... My concern now is this:
If ServiceDefinitionInterface provides the __invoke() method and ExtensionInterface overrides it, what value does FactoryInterface provide? Because currently I'm picturing an empty interface. If that's the case, I'd prefer just 2 interfaces:

FactoryInterface
ExtensionInterface extends FactoryInterface

Regarding withDependencies(), I'm in agreement. I think we all are at this point.

[XedinUnknown]

On the other hand, it's possible that the standard would evolve, and that there would be more types of service definition (theoretically) that just Factory and Extension. Having a separate facet for Service could allow those paths to evolve independently while still giving the ability to build a graph. So, maybe the callable part isn't necessarily a part of every service, but a (possibly empty) list of deps is. One example of this could be a scalar value: it's a service from the container consumer's point of view, but it has no callable definition because it is defined simply by it's value. This gives configs the ability to specify scalars without having to wrap them in a callable, which would otherwise be a small but useless overhead. I guess the most noticeable impact would be on compiled containers, because I guess a serialized int is much smaller than a serialized callable. Anyway, i digress, and so from this point of view it makes sense to have a ServiceDefinitionInterface with just getDependencies(), and add __invoke() independently to the FactoryInterface and ExtensionInterface independently, like @mindplay-dk suggested.

[mecha]

My only concern is that this introduces burden for consumers. A consumer that works with a list of callable|ServiceDefinitionInterface now cannot assume the ability to invoke elements in the list, because the second type in the union is not invocable. The correct element type would have to be callable|FactoryInterface|ExtensionInterface.

And this also introduces disconnect with the current compatibility between factories and extensions.

Consider this: The signature of a factory is compatible with the signature of an extension (since the $prev argument for extensions is optional). In fact, it is currently allowed to reuse a factory as an extension. Consider:

class SP implements ServiceProviderInterface {
	protected $factory;

	public function getFactories() {
		return [
			'foo' => $this->factory,
		];
	}

	public function getExtensions() {
		return [
			'bar' => $this->factory,
		];
	}
}

The interfaces you're proposing wouldn't be compatible in this same way, and I don't think it makes sense to have this kind of disconnect. It makes more sense for them to be compatible in the same way as the callables.

So for now I'm going to keep on advocating this design:

FactoryInterface
ExtensionInterface extends FactoryInterface

[XedinUnknown]

Yup, i see the problem. The question is therefore: are service definitions necessarily callable? I think we need a lot more input here. @samdark @romm

[mindplay-dk]

My only concern is that this introduces burden for consumers. A consumer that works with a list of callable|ServiceDefinitionInterface now cannot assume the ability to invoke elements in the list, because the second type in the union is not invocable.

I'm not sure what you mean - the second type will be invokable.

I will push the change in a minute, so we have something concrete to discuss, inline comments, etc.

[mecha]

I'm not sure what you mean - the second type will be invokable.

If ServiceDefinitionInterface does not provide __invoke(), it is not invocable. And it would make factories not compatible with extensions, which they should be.

[mindplay-dk]

I'm not sure what you mean - the second type will be invokable.

If ServiceDefinitionInterface does not provide __invoke(), it is not invocable. And it would make factories not compatible with extensions, which they should be.

I still don't know what you mean.

It's just a facet - just what the factory and extension types have in common, which is listing dependencies.

Both types are invokable, of course.

I've implemented support for factories and extensions, with dependency validation via the ServiceDefinitionInterface facet - it's passing the tests, have a look:

https://bitbucket.org/mindplaydk/mindplay-funbox/src/provider-interop/src/

The only place it depends on the facet is for validation, here.

The actual type-checks for factories and extensions are against their respective interfaces.

[mecha]

I still don't know what you mean.
It's just a facet - just what the factory and extension types have in common, which is listing dependencies.

I don't know what you mean by "facet". My point is you can't invoke it. Demonstration:

function foo(ServiceDefinitionInterface $def) {
	// Can't do this - it's not invocable
	$definition(new Container());
}

/** @param callable|ServiceDefinitionInterface $def */
function foo($def) {
	// Can't do this - union contains a non-invocable type
	$definition(new Container());
}

/** @param callable|FactoryInterface|ExtensionInterface $def */
function foo($def) {
	// This is correct type requirement
	// all types in the union are invocable
	$definition(new Container());
}

Maybe by "facet" you mean that it's not meant to be depended on? And that it only exists to store the getDependencies() method which is common to both of its descendants? If so, I don't like that kind of interface design. If an interface exists, a consumer can depend on it. Saying that one should not defeats the purpose of the interface.

And this is still not addressing the fact that compatibility between factories and extensions is broken with this design. Extensions are valid factories. Demonstration:

class MySp implements ServiceProviderInterface {
	protected $ext;

	public function __construct() {
		$this->ext = fn($c, $p = null) => /* ... */;
	}

	public function getFactories() {
		// This can be done with callables
		// because their signatures are compatible
		return ['foo' => $this->ext];
	}

	public function getExtensions() {
		return ['bar' => $this->ext];
	}
}

However:

class MySp implements ServiceProviderInterface {
	protected $ext;

	public function __construct() {
		$this->ext = new Extension(
			['some_dep'],
			fn($prev, $dep) => /* ... */
		);
	}

	public function getFactories() {
		// This can't be done because extensions don't
		// inherit from factories
		return ['foo' => $this->ext];
	}

	public function getExtensions() {
		return ['bar' => $this->ext];
	}
}

So this design is, quite literally, incompatible with the current spec.

Please note: this is just to demonstrate the incompatibility, not to bring attention to the use case of reusing extensions in this way.

[mindplay-dk]

Maybe by "facet" you mean that it's not meant to be depended on?

No, you would type-hint against this when you're only interested in the thing that factories and extensions have in common: being able to list their dependencies. The definition type defines a common facet of the other two types. This is a completely normal and ordinary type relationship.

Extensions are valid factories.

What?

How?

Why?

[mecha]

Sorry, I misunderstood what you meant by "facet". I understand now.

What? How? Why?

What: You can pass an extension to something that expects a factory

How: type fn(Container $c, $prev = null): mixed is a subtype of fn(Container $c): mixed

Why: Liskov Substitution Principle

[mindplay-dk]

Well, when we're talking about closures, you can literally pass anything you want to any function.

But that doesn't mean it's going to work.

If you pass a $previous argument to a factory, it's not going to do what you expect.

If you don't pass the $previous argument to an extension, well, some extension functions might do something, but probably most of them will fail, unless they're expecting a null argument.

The fact that these situations aren't type-checked, in my opinion, is a problem with the current spec - or with PHP, really, where we don't have function types.

I just read through the entire spec again - it clearly presents factories and extensions as being two different things with different arguments:

I don't see anything that suggests factories and extensions should be interchangeable.

By the way, PHP only checks for missing arguments - it doesn't care about extra arguments:

https://3v4l.org/uGoeK

So if you're dealing with an extension, you have to pass the second argument - but if you know you're expecting an extension, you would be passing the second argument, or it's probably a bug.

Your implementation of the interface of course can accept closures with any number of arguments - it's up to your implementation how you deal with those. You can take the same closure and wrap it in implementations of the factory or extension interfaces, if your implementation supports that.

The difference between an extension and a factory, is with extensions you should be passing the second argument - even if it was explicitly defined as null, or if no factory was defined, an extension caller should be passing that second argument.

Otherwise, they're not treating extensions correctly.

Why would we deliberately invite that?

[mindplay-dk]

Just to clarify:

class MySp implements ServiceProviderInterface {
	protected $ext;

	public function __construct() {
		$this->ext = fn($c, $p = null) => /* ... */;
	}

	public function getFactories() {
		// This can be done with callables
		// because their signatures are compatible
		return ['foo' => new Factory($this->ext)];
	}

	public function getExtensions() {
		return ['bar' => new Extension($this->ext)];
	}
}

If that's what you really want for some reason, you can.

Having to be explicit about it is probably helpful in the 99% cases where you're not doing that deliberately.

[mecha]

Aah! I see where the misunderstanding is. It's how we to interpret this signature:

function (ContainerInterface $c, $prev [= null]): mixed

Is $prev nullable or is it optional?

I interpret it as optional. Reason: the argument has no type hint, so null values are already acceptable. The signature doesn't need to specify = null to allow null values to be passed. So because it does specify the = null part, I interpret is as optional.

(Of course, I understand that writing extensions that cater for null is ... weird. Personally I've never cared to cater for null values. But I accepted that as being my own laziness.)

I could be wrong. If the 2nd argument is not optional, and is just nullable, I will conceded in 3.1 milliseconds and accept the current design.

---

Edit: the spec says that extensions "accept the following parameters". The terminology here is a bit ambiguous - it doesn't mention what arguments consumers SHOULD pass, only what CAN be passed. So there's room for interpretation.

Edit 2: @mindplay-dk I didn't see your new replies on the issue when I wrote this. Seems like there's no misunderstanding at all. Still, I'll leave my reply here for record-keeping.

[mecha]

I gave this some more thought and concluded that you're right @mindplay-dk.

From purely a type compatibility perspective, factories and extensions may be compatible. But from a consumer's point-of-view, invocation would lead to undefined behavior. In a way, ServiceDefinitionInterface reflects this fact by omitting the ability to invoke it.

So I'm in agreement with the current design. Thanks for the debate 🤚 😄

[mindplay-dk]

@mecha challenging the ideas is part of the process, I appreciate the opportunity to really think things through 😄

I'm not sure where this goes from here. Going to mark this as "ready for review", I guess. 🙂

[XedinUnknown]

I'm still not sure about this. I appreciate how a "service" should be its own thing, and that extensions and factories, while compatible, are not the same thing - in fact, in later revisions of the standard they may evolve independently, and it's great that we put this ability in at this point.

What is confusing for me is this comment. @mindplay-dk says that the services will be invocable, and @mecha says they are not. I'm not sure where the confusion is, as a ServiceDefinitionInterface with only a getDependencies() is not invocable. Therefore, the callable|ServiceDefinitionInterface type does not guarantee that all of those things can be invoked. On the other hand, perhaps something like callable|(callable&ServiceDefinitionInterface) would guarantee that, but it's more complex, and is not currently supported by PHP itself (although union types are supported in PHP 8, but not intersection types, yet I think we can look forward to that).

[mindplay-dk]

I think I misread that comment at the time. 😄

When I said "the second type will be invokable", I assumed the second type would be one of two invokable types.

callable|ServiceDefinitionInterface would be confusing, because ServiceDefinitionInterface is not callable.

If you want something callable, you would type-hint as either:

1. callable|FactoryDefinitionInterface or callable|ExtensionDefinitionInterface - asking for something callable, so that your code knows how to call it.

2. callable|FactoryDefinitionInterface|ExtensionDefinitionInterface - this would be what you probably really meant by callable|ServiceDefinitionInterface in your early conceptions of ServiceDefinitionInterface where this would somehow be either factories or extensions.

I'd be surprised if there is really a use-case for accepting both factories and extensions?

But if you must, you can of course type-hint them as described in (2) and then type-check them with instanceof before calling them - which would help with static analysis anyhow, ensuring you pass the right number of arguments to factories and extensions.

And of course, type-hinting as ServiceDefinitionInterface is perfectly valid to - if you have a function that only cares about enumerating dependencies, and doesn't need to invoke anything (as in the use-case of validating dependencies) using this type independently of the others makes sense.

[mindplay-dk]

There is maybe one thing left to consider.

The getDependencies method currently returns a list of keys - this is possibly not very helpful to someone who gets an error-message from a validating container, about a missing dependency.

ContainerException: Missing dependency "user-connection"

You would know the name of the missing dependency, but not it's type.

Of course, this might just be considered a non-feature of the proposal at large - I mean, we also don't specify the expected return-types of either factories or extensions, it's all dynamic, and relies on type-checking in constructor declarations.

A missing dependency of course doesn't have a constructor declaration, so maybe that's fine?

Though it would have been more useful to see an error message like:

ContainerException: Missing dependency "user-connection" of type MyNastyORM\Connection

I don't know. Just something to ponder. 🤔

(EDIT: This is one of the problems I'm attempting to solve with the experimental "funbox" container mentioned above...)

[mecha]

I'm not sure about this.

On the one hand, I can see its utility. It allows an inspector, be it a container or otherwise, to detect wrongly wired services and make sure that a service graph is 100% valid.

On the other hand, it doesn't really change much in practice. An exception is going to be thrown either way; either a ContainerExceptionInterface from a container that validated a dependency's type, or a TypeError from PHP that checked a passed argument against a type hint.

So I'd say, unless there is a stronger reason than just exception messages, we should probably hold off on this.

[XedinUnknown]

Ah, bugger. I shouldn't have deprecated container-aware exceptions then, I guess 😆

On a serious note, this is up to the implementation. A container-aware exception would be nice, but it is not part of PSR-11, so I doubt this kind of interface will have a lot of popularity. Yet still, implementations can use reflections or static analysis to determine this. Also, static analysis can be used by tools like IDEs to deduce the type of a returned dependency, and in fact @Biont has done something like this already.

[XedinUnknown]

Hang on, though. The ContainerInterface implementation most likely has access to the service provider. Therefore, it has access to service definitions. Therefore, it can use reflections to retrieve the type of a service under any circumstances. So no problem.

Also, multiple providers can declare overriding/extending services. In the case of extensions, because there can be many, it is not clear what type to report. But in any case, all this can be deduced, it seems.

[mindplay-dk]

I can also start working on a meta document around the end of this week.

I'm sitting on some preliminary drafts for the PSR and meta documents, I will post these later.

One issue I'd like to discuss is that of the factory and extension interfaces extending ServiceProviderInterface - this gets a little weird when we use these in type-hints:

interface ServiceProviderInterface
{
    /**
     * Returns a list of all container entries registered by this service provider.
     *
     * - the key is the entry name
     * - the value is a callable that will return the entry, aka the **factory**
     *
     * A factory is an instance of {@see FactoryDefinitionInterface}, or a `callable` with the following signature:
     * 
     *     function(\Psr\Container\ContainerInterface $container)
     *
     * @return (callable|FactoryDefinitionInterface)[] 👈
     */
    public function getFactories(): array;

The type-union here is peculiar - it was fine when FactoryDefinitionInterface used to be just an optional, formal callable, but it gets weird because that type also includes the ServiceDefinitionInterface::getDependencies method, which a regular callable can't have.

So an implementation of the factory or extension interface must include getDependencies, while a callable can't.

This doesn't seem quite right?

My other suggestion here doesn't have this problem, but also relies entirely on "informal" types, which (to my recollection) was part of what we were trying to improve with these changes? It's been a few years...

[mindplay-dk]

I've written an initial draft for the PSR and meta documents.

https://github.com/mindplay-dk/service-provider/pull/1/files?diff=unified&w=0

This was written under the assumption of this PR eventually getting merged - I realize this PR might not be the final destination, and more discussions are definitely required (including some new topics with TODOs in the document) but I figured we need to start somewhere.

@moufmouf @mnapoli @XedinUnknown is anyone still interested, or is it perhaps time to officially dissolve the working group? 🙂

Entirely up to you, folks - if you're interested and can spare the time, that would be great!

If not, I will most likely try posting in the FIG group in the coming weeks and try to find interested parties to form a new working group and reboot the conversation.

Alternatively, you might consider making me a contributor of this group, so I can start going over all the issues, close out the stale ones, etc. - and perhaps switch from issues to threaded discussions, before we go back to the FIG group and try to restart the conversation.

Whatever you'd like to do, I'm on board, but as you can see, I'm already quite invested in this again, and going to do something. 😄

[XedinUnknown]

I think we should bring in people who have the expertise and passion to bring this project to a conclusion, whether release or archive. @mindplay-dk, in practical terms, what do you require? My authority spans as wide as the Admin privileges on this repo.

Perhaps, what we should do is: issue a call and get all workgroup members who respond to provide feedback on the draft PSR in this PR, then finalize and merge it, and then show it to David and Matthieu. We will be able to exclude non-responsive members, and gather feedback. At this point I hope to understand whether this standard is in fact needed, or are people interoperating in a different way on this level, or do they even want to.

[XedinUnknown]

I don't know how FIG sees Psalm at this point, and whether Psalm declarations may comprise a formal part of the spec. But usually, PSRs are a handful of interfaces, accompanied by some meta documentation. It's that documentation that explains the nuances of the standard, and is really the authoritative source of truth, IMHO, not the PHP formalization which may be lacking in specificity due to language limits. So, I'm wondering if we can use Psalm declarations here, and thus enable consumers to use them in their projects.

[mindplay-dk]

@mindplay-dk, in practical terms, what do you require?

Participation. 😄

As you can see, I am now working my way down the issues list - it'll take a while.

Perhaps, what we should do is: issue a call and get all workgroup members who respond to provide feedback on the draft PSR in this PR, then finalize and merge it, and then show it to David and Matthieu. We will be able to exclude non-responsive members, and gather feedback. At this point I hope to understand whether this standard is in fact needed, or are people interoperating in a different way on this level, or do they even want to.

Agreed, but let's wait until things look more presentable - once the issue list is clean, once we have any unresolved discussions laid out, and once we have at least a starting point for the PSR documents, it will make more sense to start inviting more people to the discussion then.

I'm wondering if we can use Psalm declarations here, and thus enable consumers to use them in their projects.

Yes, this is on my laundry list. 👍

It looks like both PHPStan and Psalm have the same callable notation - I will add this soon.

[mindplay-dk]

@XedinUnknown with the types in place, what do you think, can we merge this and then start discussing and fixing other issues individually? This thread is a bit overwhelming at this point. 😅

Specifically, we could open separate issues for this:

The ability to have dependencies is indeed a trait of all service definitions. However, a service definition is primarily defined as something that can be resolved to a service, and the dependencies part seems secondary to that. So while splitting the Service facet into Factory and Extension is an elegant way to separate concerns, I'm not sure whether it represents the nature of the problem.

And I'd also like to talk more about this:

So far, the things in FIG have had formal PHP interfaces.

This keeps ringing in my mind. It's great we have static analysis with the callable types, but these aren't of course checked at run-time. Type-safety in general needs more discussion, I think. (#62)

Again, my goal is not to release this, just to have an offset for discussion, then making smaller and more focused changes from there. 🙂

[mindplay-dk]

(of course, everyone is welcome to comment on this, but it doesn't really seem like anybody else wants to get involved at this time. 😄)

[mindplay-dk]

@moufmouf @mnapoli @prisis @Biont @gbprod @XedinUnknown any takers?

If not, I might go ahead and merge this, merge in the draft PSR, and announce a call for new participation on the FIG list.

I have two weeks of vacation now, and I'm willing to put in some work here. :-)

[mindplay-dk]

Okay, I've done some testing and benchmarking with regards to type safety - you can see some code and benchmarks here: #62 (comment)

I am now very unsure about FactoryDefinitionInterface and ExtensionDefinitionInterface having any practical value - in my experiments, they added no real type-safety benefits, because PHP lacks function types, and so I'm leaning towards a revised position, that maybe we should err on the side of simplicity, and avoid adding unnecessary complexity, if it doesn't buy us anything practical in terms of interoperability.

At best, these seem like "feel good" types, or perhaps "types as documentation" - but even then, the only time these types would get checked, is at implements, which, if the underlying implementation of the factory/extension is a callable anyway, the implements clause isn't actually asserting anything meaningful anyway.

I might feel different if PHP had function types - but even if you're hoping that some day it might, betting on that doesn't buy us any real type-safety or type-checking at the level of either implements or type-hints anyway; the function type would take care of that. And since we don't know what function types would look like, or how they'd work, it's probably silly to expect an interface would even help us in that direction.

Now, if we were to drop these interfaces, this would then beg the question, how does ServiceDefinitionInterface::getDependencies fit into that story?

In the current state of this PR, ServiceDefinitionInterface is inherited by FactoryDefinitionInterface and ExtensionDefinitionInterface, so where/how would we be able to offer dependency enumeration?

Off the type of my head, I guess we could just throw it in here:

interface ServiceProviderInterface
{
    /**
     * @return FactoryDefinitionInterface[]
     */
    public function getFactories(): array;

    /**
     * @return ExtensionDefinitionInterface[]
     */
    public function getExtensions(): array;

    /**
     * @return string[] list of dependency service IDs of the given service ID
     */
    public function getDependencies(string $id): array; // 👈
}

This is arguably simpler - you no longer need to run-time type-check anything, there's no optional second interface, no reason why factories/extensions would need to be factored back and forth between classes and callables, it just seems a lot simpler.

For DI containers that do not perform any ahead-of-time validation, they can simply ignore this method and not call it. For providers/builders that can't enumerate their dependencies, returning an empty array of course would be permitted - which would also make migrating existing providers very easy.

Any thoughts?

[mindplay-dk]

@moufmouf @mnapoli @prisis @Biont @gbprod @XedinUnknown happy new year 😄🍾🥂

⚠️ long comment: I've highlighted the most important points in bold.

I've done a lot of work on this over the past couple of weeks - I'm deep in this project now, and I'd really like to get the ball rolling again.

It might just be the season (xmas, new years) and maybe no one had the time to get really involved - some of you may have lost interest over time, some of you may just not have the time or interest, and that's completely okay.

I don't want you to feel like I'm pressuring or harassing you to get involved again - I'd like to be respectful of your time, so this will be the last time I @ mention everyone.

Before I go back to the FIG list and try to drum up some attention for this project, I'd like to wrap up this PR though, so I can wrap up the PSR and meta document drafts I've been working on - so that any potentially new involvements can have something more concrete review and discuss.

Any objections to my previous comment about adding getDependencies to the ServiceProviderInterface?

It's the same feature we've been discussing in this PR and elsewhere - it has just taken on a different and simpler form.

But I already have sections of content written up in the draft PSR describing this feature in it's current form, so I'd need to update that as well.

It's the last thing I feel I can really do on my own, without getting more people involved - pretty much everything else is open questions and decisions I'm not comfortable making without the participation of other community members.

Thank you for your participation this far! I realize it may seem out of the blue to double down on this, but I didn't fully understand the need for this PSR back then, and now I very much do.

If there are no comments in the next couple of days, I will probably just take the reigns and start merging things in - I just wanted to make sure you don't take this the wrong way: I am not trying to force my own opinions onto a potential future standard, it's actually the opposite. I'd like to get other people involved, and see where we can take this idea in collaboration. In order to do that, there needs to be an actual PSR and a meta document - they don't need to be finished, not by any means, there just needs to be something to drive forward a conversation.

If no one else comments, it's also possible I might bench this PR for the time being, and remove mentions of this feature in the draft PSR before merging that in - and then start a new, separate discussion about this feature, before inviting new participants. I don't know yet, but I need to do something - you gave me contributor rights to this repo, so if I hear nothing else, I'm just going to assume you're okay with me trying to push things along. 🫡

Best wishes for the new year, guys! 😄

[mindplay-dk]

I've thought long and hard about this, trying to close this issue and PR.

I'd like to simplify this - listing available dependencies could be much simpler than what I've proposed here. It really only needs to be a method on the provider itself, like getDependencies(): array - this provides the exact same functionality with fewer functions calls.

Containers are either interested in this information, or they're not - so they might as well get all the dependencies with one call, rather than inquiring with each service definition.

So I think this feature needs to be optional, both for containers and providers.

Not all providers will be able to enumerate their dependencies, and not all containers will be able to consume that information, so this approach seems better in terms of interoperability.

With that in mind, what do you think of this?

interface ServiceDependencyInterface
{
    /**
     * @return array<string,string[]> map where entry ID => list of dependency IDs
     */
    public function getDependencies(): array;
}

EDIT: pasted the wrong interface! corrected.

This would be a second interface that a provider can optionally implement.

The consuming container, if it supports enumerated dependencies, would perform a type-check on a ServiceProviderInterface instance to see if it also implements ServiceDependencyInterface.

This removes the need for a "dud" boilerplate return [] implementation of the method by providers that can't actually enumerate their dependencies, and instead makes it "truly" optional, and explicitly opt-in by providers that do support it.

In addition, this avoids a breaking change - it's a new, optional interface, so this would be backwards compatible with current 0.4 providers.

Note that I also considered e.g. ServiceDependencyInterface extends ServiceProviderInterface, which might make it more self-explanatory in some sense - but this coupling isn't strictly necessary, and I don't think we would want to encourage anyone writing a container that only supports ServiceDependencyInterface and not ServiceProviderInterface. (Which would be pointless anyway, since they can just implement it as return [] anyway.)

@moufmouf @mnapoli @prisis @Biont @gbprod @XedinUnknown any objections? I know this is very different from what I originally proposed with this PR. But really, only the shape of it is different - it's offering the same feature, but backwards compatible, and fully optional, so this seems like a true win/win?

If no one objects, I will go ahead with this today, update the upcoming PSR draft and meta-documents accordingly, and then hit the PSR mailing list and try to reboot the conversation. It's time. 🙂

Just to reiterate: this is of course all still open for discussion! In no way am I suggesting any of this would be final. I am just trying to push the proposal into a form more suitable for discussion, so we can get the conversation started again.

[mindplay-dk]

FYI, the interface I pasted initially was wrong - the correct function signature is getDependencies() and the return type is array<string, string[]>.

[mindplay-dk]

The PR has been updated with this change, and I also improved the type-hinting.

[mindplay-dk]

By the way, I decided to keep the FactoryDefinitionInterface and ExtensionDefinitionInterface types for now - I only removed the extends ServiceDefinitionInterface clause, since ServiceDefinitionInterface itself was removed in favor of ServiceDependencyInterface.

These will of course remain optional - callables may of course still be used, but the interfaces might be useful for containers that want to implement them for their own internal safety. (it's quite possible that that's not our responsibility, but I don't think they're harmful, so just leaving them for now.)