thephpleague / container

Small but powerful dependency injection container
http://container.thephpleague.com
MIT License
843 stars 102 forks source link

Allow installation with psr/container 1.1.0 #234

Closed Art4 closed 2 years ago

Art4 commented 3 years ago

With #226 it was possible to use league/container with "psr/container": "^1.0.0 || ^2.0.0", but while releasing v4, the requirement was changed to "psr/container": "^2.0.0", see https://github.com/thephpleague/container/commit/67cba1a184e9ebbf082e37d3c9337e755a90a58f

I couldn't find any reason why this was made and I propose to bring the compatibilty with "psr/container": "^1.0.0 || ^2.0.0" back again.

The main reason is, that Symfony 5.4 (LTS) will only allow "psr/container": "^1.0.0" for BC reasons. Starting with Symfony 6.0 (not LTS) it will be possible to use "psr/container": "^1.0.0 || ^2.0.0", see https://github.com/symfony/symfony/issues/42683. The next LTS version will be Symfony 6.4 and will be released November 2023. I would like to follow the LTS path, and this means that I cannot update to league/container v4 until Nov 2023.

This PR would allow to use both Symfony 5.4 and league/container 4 today.

philipobenito commented 3 years ago

this actually causes issues due to differences in method signatures between the two, it caused no end of problems trying to maintain for both

Art4 commented 3 years ago

No, it will not. league/container is already implementing psr/container v2.0.0, so it is also implementing v1.0.0. No other actions or maintenance are needed. Please look at the successful tests for PHPUnit on 7.2 --prefer-lowest: https://github.com/thephpleague/container/pull/234/checks

In fact this procedure is the recommended way to upgrading the PSR interfaces: https://www.php-fig.org/blog/2019/10/upgrading-psr-interfaces/

philipobenito commented 3 years ago

It may have been that it was an issue with the previous major that supported PHP 7.1 but it's been a while and can't remember for certain. I'll want to double check and thoroughly test manually before merging, should be able to get to it next week.

Art4 commented 3 years ago

Thank you. Yes, it would be a problem for PHP <7.2, but because league/container already requires "php": "^7.2 || ^8.0" this isn't a problem here. This topic is also mentioned in the blog post:

An implementing library, in its current version, is automatically compatible with the current v1 of the spec as well as v2, as long as it's running on PHP 7.2 or later. That's because it can safely "drop" the type hints and still be syntactically valid.

Art4 commented 3 years ago

I've looked deeper into this issue and found the errata at the PSR-11 meta website:

https://www.php-fig.org/psr/psr-11/meta/

11. Errata Type additions

The 1.1 release of the psr/container package includes scalar parameter types. The 2.0 release of the package includes return types. This structure leverages PHP 7.2 covariance support to allow for a gradual upgrade process.

Implementers MAY add return types to their own packages at their discretion, provided that:

  • the return types match those in the 2.0 package.
  • the implementation specifies a minimum PHP version of 7.2.0 or later.

Implementers MAY add parameter types to their own packages in a new major release, either at the same time as adding return types or in a subsequent release, provided that:

  • the parameter types match those in the 1.1 package.
  • the implementation specifies a minimum PHP version of 7.2.0.
  • the implementation depends on "psr/container": "^1.1 || ^2.0" so as to exclude the untyped 1.0 version.

Implementers are encouraged but not required to transition their packages toward the 2.0 version of the package at their earliest convenience.

It is recommended to exclude the untyped 1.0 version, so I will update the PR to require "psr/container": "^1.1 || ^2.0" instead of ^1.0.0

craig-mcmahon commented 2 years ago

It would be great if this could be released, I am currently implementing league/container in a project with a symfony dev dependency and have to use "psr/container": "1.1.1 as 2.0.1" to get around the conflicting dependency on psr/container

ADmad commented 2 years ago

Please merge this. As mentioned above a lot of symfony packages still have psr/container dependency as ^1.0 which is causing problems to a numbers of users since lot of projects/libs have symfony packages as direct or indirect dependencies.

ADmad commented 2 years ago

Thank you.