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

Revert "Support for psr/container v2" #147

Merged
merged 1 commit into from
Jul 20, 2022

Conversation

Ocramius
Copy link
Member

Reverts #127

As found in multiple downstream containers, the lack of a constraint on psr/container:^1 led to upgrades that are completely incompatible with psr/container:^2 signatures.

The problem is that child classes have methods without a : bool return type declaration for ContainerInterface#has(string $name): bool, which means that a fatal error is raised, due to LSP compatibility checks performed by PHP.

Practically, upgrading to psr/container:^2 may really happen in a major version, possibly as an isolated dependency upgrade.

While @boesing has worked hard in trying to implement a BC-compliant solution, the SAT solver of composer will find a way to upgrade to incompatible dependencies anyway, and inheritance is such a massive headache that we may really just want to get rid of it overall, rather than

Ref: laminas/laminas-hydrator#92
Ref: laminas/laminas-di#78
Ref: #146

Copy link
Member

@boesing boesing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simple revert. I don't see a better solution tho.
We should think about closing our implementations while providing proper interfaces to allow composition.

IMHO, v4 should have final ServiceManager and the plugin manager should not implement PSR-11 anymore.
If the plugin manager is as configurable as possible, we might not even need an abstract plugin manager.

@Ocramius
Copy link
Member Author

Can't wait myself for final on the ServiceManager :D

@Ocramius Ocramius self-assigned this Jul 20, 2022
@Ocramius Ocramius merged commit 216f972 into 3.15.x Jul 20, 2022
@Ocramius Ocramius deleted the revert-127-feature/psr-container-2.0 branch July 20, 2022 09:48
@Seldaek
Copy link

Seldaek commented Aug 31, 2022

Due to 3.15.0 existing with psr/container 2 support it means I am now stuck on that version as I have other things requiring psr/container ^2.

I don't think it matters much given the fairly small usage of laminas components I have in the project, but just wanted to let you know that you may be protecting some users from bad upgrades (tho really they can easily pin psr/container:^1 themselves if their code requires it to function) but it is also causing issues for other users that are ready to update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BC Break Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants