-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
5.next: port over thephpleague/container
#18090
base: 5.next
Are you sure you want to change the base?
Conversation
a1e0732
to
abc4888
Compare
use Cake\Container\ServiceProvider\ServiceProviderInterface; | ||
use Psr\Container\ContainerInterface; | ||
|
||
class Container implements DefinitionContainerInterface |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would we switch the base class of Cake\Core\Container
in a future 5.x release or leave that to 6.x?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be a 6.x only move since major changes have already happened here + more major changes are going to happen according to ADmad's christmas wishlist bellow.
If we are internalizing the
These are few things that bugged me when using the container. |
Currently I just added the ReflectionContainer by default as you can see here I always thought there were reasons why one want's to manually add services to a container instead of relying on Reflection. |
Disabling reflection/autowiring should be an option but available by default IMO. |
Due to our code style some BC changes had to be done (mostly return types for interface methods) to this lib to get it all green, therefore it is NOT a 1:1 replacement of
thephpleague/container
I added it to
5.next
and not6.x
since it can be seen as an opt-in feature to use the built-in container instead of the already existing container implementation. If not suited for a minor update this can still be easily rebased onto6.x
It of course needs to be connected to the rest of the framework (e.g. the Testsuite).