inpsyde / modularity

A PSR-11 implementation for WordPress Plugins, Themes or Libraries.
https://inpsyde.github.io/modularity/
GNU General Public License v2.0
44 stars 4 forks source link

Not compatible with PSR-11 v1.1 even if constraints say so #24

Closed bartkleinreesink closed 1 year ago

bartkleinreesink commented 1 year ago

https://github.com/inpsyde/modularity/blob/007fb9a6e32bc8c02f310223db466a945fb8e1b2/src/Container/ReadOnlyContainer.php#L103

This line broke my plugin which uses a newer version of Psr\Container\ContainerInterface because it doesn't have a return type of bool. An update would be nice.

Chrico commented 1 year ago

Hey @bartkleinreesink ☕

Thanks for your issue. Which version of psr/container are you using? Our library defines ~1.0 which has no return type defined on has(). https://github.com/php-fig/container/blob/1.x/src/ContainerInterface.php#L35

bartkleinreesink commented 1 year ago

Hi @Chrico,

Yes, I'm aware that's what is causing the error. The interfaces don't match.

I'm on "psr/container": "^1.1 || ^2.0" which is a dependency in "php-di/php-di": "^7.0"

https://github.com/PHP-DI/PHP-DI/blob/master/composer.json#L28

gmazzap commented 1 year ago

@Chrico, the problem is that we require ~1.0, which includes 1.1.0, and that version has type hints for parameters, see https://github.com/php-fig/container/blob/1.1.x/src/ContainerInterface.php

Setup the whole matrix of the PHP version from 7.2 to 8.2 and PSR-11 version from v1.0.0 to 2.0.0 is not possible.

But if we abandon PSR-11 1.0.0 and use 1.1 as the minimum instead (like: "^1.1.2 || ^2.0"), then we can make this work.

@Chrico if you agree, I can work on a PR.

Chrico commented 1 year ago

@gmazzap thanks for the input. I guess it's fine and we might should also increase the PHP min. If you want, create a PR :)

gmazzap commented 1 year ago

PR sent. A note: I did not raise PHP requirement. Considering that v2 supports PHP 7.4, we can consider a new major release in which we go with PHP 7.4+ and only support PSR-11 v2.

But with the PR I sent, we can release the change as a minor.

People that are on PSR-11 v1.0 will not receive it because the Composer constraints prevent that, so they will get the current version without type declarations, and everything will work for them.

People on v1.1, like @bartkleinreesink, would receive the new minor, and everything will also work for them as well.

Finally, because in the code I added return types (we are on PHP 7.2+, so we can have contra-variance in the return types) people who are on v2 will also have no issue because the type declarations are all there.

Chrico commented 1 year ago

We just released version 1.6.0 which should solve your issue https://github.com/inpsyde/modularity/releases/tag/1.6.0 . Please check out if everything works for you 💪🏻

bartkleinreesink commented 1 year ago

Thanks everyone!