service-provider icon indicating copy to clipboard operation
service-provider copied to clipboard

Clarification: "The service created by a factory should only depend on the input parameters of the factory"

Open mindplay-dk opened this issue 1 year ago • 6 comments

@moufmouf can you provide some clarification on the following section of the README, please?

https://github.com/container-interop/service-provider/blob/a55e71782c7c19b34112296ac22d2ae42b564469/README.md?plain=1#L344-L370

This section appears to stipulate that service provider implementations must essentially be stateless? But it does not explain why.

I frequently use service providers with constructor arguments - I don't believe most existing DI containers mandate or enforce any such restriction, so my first impulse is to remove this clause, as it appears to potentially clash with common patterns and practices used by existing DI containers, possibly creating interoperability problems.

Do you recall why this was a requirement? My best guess is, you intended for providers to be automatically bootstrapped via some sort of service discovery facility?

If that was the reason, I'm not sure this the right approach. While implementing service discovery might be made easier by this stipulation, it is not made impossible by permitting service providers to have constructors with arguments.

For example, auto-discovered service providers with constructor arguments could load these from configuration files, obtain them from the system environment, or load them from a key vault, and so on.

Things like API keys or other access credentials need to come from somewhere, and baking them into a service provider isn't typically going to make sense - if you want a working service configuration, some things are going to be non-optional, and non-optional constructor arguments are the natural way for any other class to require those things.

It's an unusual requirement, and there is no way to enforce it, so as said, I'm inclined to remove it.

Any objections?

mindplay-dk avatar Dec 22 '23 16:12 mindplay-dk