Remove laminas/service-manager dependency and factory stubs

[marcelthole]

Q	A
Documentation	yes
BC Break	yes

Description

Background:
@froschdesign comment here: #126 (comment)

You are adding support for laminas-servicemanager with version 4 here. Do you see a change to remove the dependency on the service manager?

In this PR i removed the servicemanager dependency completly. The configuration for the Laminas-servicemanager usage is still there. The biggest change here is the removal of the ContainerAbstractServiceFactory. This change is also documented in the migration guide from v2 to v3

[froschdesign]

Why does this need to be removed? laminas-servicemanager can use this factory without any interface.

[marcelthole]

If i remove the AbstractFactoryInterface this class will maybe not be valid anymore for a future implementation of Laminas Servicemanager v5 because the interface could be changed and we didn't know it, because we don't have a version constraint for that.

That would work now, but more by chance. Than we would implement an interface without knowing the interface.

Overall the only benefit of this class is to avoid some boilercode if i want to use multiple containers if i understand this correctly.
Instead of adding them to the session_containers array config, i need to add a own factory for every container i want.

But overall: if you still want this ContainerAbstractServiceFactory without the Interface but with the public function canCreate(ContainerInterface $container, string $requestedName): bool; method i could revert this class. But than i don't get the point why we should remove the servicemanager dependency at all

[froschdesign]

Overall the only benefit of this class is to avoid some boilercode if i want to use multiple containers if i understand this correctly.

Correct and a simple use or another simple use is a must.

But than i don't get the point why we should remove the servicemanager dependency at all

It was a question if this would work. Therefore, this should be checked.
So the question remains, does the service manager helps here?

[marcelthole]

Personally i didn't use laminas-session. So i'm not an expert in using it in a project. Overall i like the idea to reduce the dependency at all because this package include only a little bit of boilercode to configure it in the laminas servicemanager world.

If we keep it, a new user need to read the documentation because they need to add a session_containers config entry. Otherwise it will not work.

if we drop it, the Laminas\Session\Container::class was defined as a default in a laminas-servicemanager world and could be used directly or the user need to add a own factory for that like in the migration guide. (to be fair, that could also be an patch to the existing code to add this as a default factory)

In my point of view i would remove the dependency here. But this is a RFC. You guys are the maintainer / TSC Member and should decide if this change would be fine or not.
If you say that you want to keep the dependency it's also fine for me and we could close this PR.
This is a Proof of Concept in first Place how it would look like if the dependency would be droped.

[froschdesign]

@gsteel
Do you see a problem if we keep the abstract factory and remove the interface and dependency to service manager? It would still work with the service manager.

[arhimede]

I presume this library will be used in some sort of Mezzio setup, which anyway have the Service Manager 3 or 4 required.
So SM will be there anyway in vendor folder, required or not by laminas-session .

So i think is useless to remove this SM dependency.

[froschdesign]

I presume this library will be used in some sort of Mezzio setup, which anyway have the Service Manager 3 or 4 required.

Not mandatory because any PSR-11 container can be used.

[arhimede]

@froschdesign @gsteel what should we do with this draft ?
As i want to prepare the release of version 3

[froschdesign]

@arhimede
I am still in favour of removing the dependency.

[arhimede]

@Jurj-Bogdan can you create a PR with this one ?

[gsteel]

Having factories that import symbols such as FactoryInterface is good for type safety, even if they are unnecessary in practice.

Not many of the shipped factories will work with a generic container AFAIK

[froschdesign]

Mezzio supports 4 different DI container implementations out of the box:

• Laminas Servicemanager
• Symfony DI Container
• PHP-DI
• Chubbyphp Container

But unfortunately, I cannot say whether everyone behaves in the same way or whether the factories are called up in the same way. 🤷🏻‍♂️

[Jurj-Bogdan]

One thing to note: with PR-168 it was decided that laminas/laminas-validator is to remain as a dependency, which will require laminas-servicemanager itself. As such, a user opting for a different DI container implementation will still be forced to add it as well.

[arhimede]

@laminas/tsc-reviewers

I am proposing to not follow this PR, close it and release version 3.0 of this library.
The only open item is the documentation .

[gsteel]

I am proposing to not follow this PR, close it and release version 3.0 of this library.
The only open item is the documentation .

Docs need to be done before release though or they will never get done.

The Psalm error baseline is also still massive - there are a lot of potentially small BC breaks that could/should be made before releasing a major to improve types and quality across the codebase