Make container-interop a forwards-compatible polyfill for PSR-11

[weierophinney]

Per discussion on php-fig/container#38, this patch updates the three shipped interfaces to instead become class aliases for the official PSR-11 interfaces. This allows implementing libraries to immediately switch to PSR-11 typehints/implementations while retaining backwards compatibility with container-interop.

The patch also includes documentation of workflows for moving from container-interop to PSR-11 for both package and application authors.

[weierophinney]

Ping @mnapoli and/or @moufmouf

[boesing]

I wonder if abandoned packages can publish new releases to packagist.

[weierophinney]

I wonder if abandoned packages can publish new releases to packagist.

If they cannot, the maintainers can "un-abandon" it, sync, and then re-abandon.

[mnapoli]

Looks good, thank you!

Would there be any conflict caused by implementing the same interface twice? (now that these classes are aliases)

[boesing]

@mnapoli Good point. For implementing libraries/projects, this will lead to a fatal error:

PHP Fatal error:  Class Laminas\ServiceManager\ServiceLocatorInterface cannot implement previously implemented interface Psr\Container\ContainerInterface in /laminas/laminas-servicemanager/src/ServiceLocatorInterface.php on line 14

Consuming libraries/projects won't have any issues.

[boesing]

maybe releasing v2.0 of this component would be a good idea rather than having a new minor version. This would provide proper migration path for implementing libraries/components.

So the only change n v2 would be the class_alias change. In laminas-servicemanager we could then bump to v2 of this component while dropping container-interop interfaces. Creating new minor versions of those packages currently consuming container-interop interfaces while switch to psr could be possible when conflicting with container-interop less than 2.0 instead of require container-interop directly.

Actually, there are some laminas components consuming container-interop tho.

[weierophinney]

@mnapoli I'd forgotten the whole implements Interop\Container\ContainerInterface, Psr\Container\ContainerInterface issue; thank you @boesing for testing that!

So yes - this would require a v2 release, as implementing libraries still need to make a change. However, as @boesing notes, the changes in implementing libraries can happen in a minor release instead of a major release, which is really the whole goal of doing this.

[Ocramius]

Has anyone tried if this trips psalm or phpstan in any way, before we move on with this approach?

[boesing]

@Ocramius there were issues with the laminas-zendframework-bridge but that was due to the fact that the bridge does its magic during request runtime. When used via autoloader, psalm is able to realize that its the exact same interface.

Thus said, when using composer autoloader to provide the alias, we should be good to go. But I can check that tomorrow in a large application using tons of these interfaces.

[boesing]

I've executed this in our project and I receive exactly 1 static-analysis error for this case:
https://psalm.dev/r/d0893a607e

I am actually unsure if thats a bug in psalm as the container mentioned does definitely implement that interface.
When switching the class_alias back to its original, psalm has no issue:
https://psalm.dev/r/5e5acaed94

[Ocramius]

I think that really needs to be tried out on a playground with an autoloader, not on psalm.dev Marco Pivetta http://twitter.com/Ocramius http://ocramius.github.com/

…

On Tue, Aug 17, 2021 at 11:50 AM Maximilian Bösing ***@***.***> wrote: I've executed this in our project and I receive exactly 1 static-analysis error for this case: https://psalm.dev/r/d0893a607e I am actually unsure if thats a bug in psalm as the container mentioned does definitely implement that interface. When switching the class_alias back to its original, psalm has no issue: https://psalm.dev/r/5e5acaed94 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#97 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABFVEFL34NF6YBD33IEQBLT5IWFZANCNFSM5B4RERSQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email> .

[boesing]

This error is reproduced from the project I have scanned. I just added it to psalm.dev for better understanding on whats going on.

I added this branch to our composer dependencies, updated, removed the interop Interface from the laminas servicemanager servicelocator and ran psalm.

Do you have something specific in mind when you say that this needs to be tried out on a playground with autoloader?

I can create a dedicated github project 🤷🏼‍♂️

[boesing]

It is indeed an error within psalm which is issued by vimeo/psalm#6325 and will be fixed with vimeo/psalm#6327.

[weierophinney]

So, @boesing , that sounds like an endorsement of this approach? If so, can you comment, and then @mnapoli can tag and release v2.0.0. 😄

Thanks, everyone, for working on this - it's nice to finally find a potential way forward that won't cause huge issues for existing users!

[boesing]

@weierophinney Yes, I think from all informations we got, having a new major in here would be the best option for now.
I have a project where another 3rd-party library is using container-interop with the ^1.1 constraint (which is closed source) and thus, we may have issues in our company to migrate BUT I already reached out to the company which does provide that library. Hopefully they either migrate to 2.0 of container-interop as well (or better, getting rid of container-interop as they only provide the container for their own library).

So I am 100% on-board here, if no one has other feedback or doubts, I think we are good to go.

[weierophinney]

@mnapoli We have a green light! Ping me when you've tagged; we'd like to start rolling out as soon as you have!

[weierophinney]

@mnapoli Ping - can you merge, and then tag and release v2.0.0 with these changes, please? Thanks in advance!

[mnapoli]

I just pinged @moufmouf to get a review from him. This is a v2 so I prefer to find a bit of time to review it, will try to do asap 😓

[moufmouf]

We probably need to update the readme to replace references to 1.3.0 with references to 2.0.0.

I have a question though.

Library implementors will require container-interop/container-interop ^2 as a dependency.
This means as an end user, if your project depends on many libraries requiring container-interop/container-interop, they must all migrate in 2.0 version for your project to migrate.

@boesing What is the rationale to tag a 2.0 rather than a 1.3? I'm not sure I understand.

[boesing]

@moufmouf when tagging 1.3, upstream projects implementing both PSR-11 and interop Interfaces will immediately break.
I wont assume that there are too many projects out there still depending on interop and if so, a smooth transition to PSR can be made when requiring 2.0 of interop.

[boesing]

@moufmouf here are code examples on why it should be a major release
before: https://3v4l.org/B5KL1
after: https://3v4l.org/iPLfM

Upstream components can require v2 and fully migrate to PSR-11 in a minor (and then also drop interop while conflicting with it in another minor if they want to) without the need of releasing a new major on their side. Maybe conflicting with v1 of interop could be a way, too but I doubt that projects require interop directly.

[boesing]

@mnapoli @moufmouf Is there anything left open which blocks this PR?
I think @weierophinney should fix the requested changes but is there anything left?
Are there internal discussions about merging this or not or was that already discussed?
I would really love seeing this available to finally get rid of \Interop\Container namespace usage.

[weierophinney]

I think @weierophinney should fix the requested changes but is there anything left?

Done on that front!

[boesing]

I think we have to change this as well, right?

What I do actually wondering about is, how we ensure compatibility for both v1 and v2 here?
Can this be achieved?
Sadly thats months ago that I spent time on that front and thus I can't remember. Maybe I find some time after PHP 8.1 and stuff to outline this.