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

[5.5] More consistent PSR-11 implementation #21327

Closed
wants to merge 4 commits into from
Closed

[5.5] More consistent PSR-11 implementation #21327

wants to merge 4 commits into from

Conversation

RemiCollin
Copy link

@RemiCollin RemiCollin commented Sep 22, 2017

This modify the behavior of the container has() method to return true if a class that has not been explicitly bound to the container can still be resolved by it.

PSR-11 doesn't impose an explicit binding of concrete class beforehand, so we can consider returning true for any concrete class that can be actually resolved as it's a better reflection of how laravel's container works internally.

See discussion in laravel internals : laravel/ideas#803

@taylorotwell
Copy link
Member

So with this change how do you tell if a custom implementation was bound in the container or it is just going to use the default? Also, just because a class is concrete doesn't mean the Laravel container can resolve it. What if it has an interface dependency that isn't configured?

@RemiCollin
Copy link
Author

So with this change how do you tell if a custom implementation was bound in the container or it is just going to use the default?

I think that would be the role of the bound() method().

Also, just because a class is concrete doesn't mean the Laravel container can resolve it. What if it has an interface dependency that isn't configured?

Having an abstract bound to a concrete class doesn't guarantee that Laravel container can resolve it as well.

In that regard, the isResolvable() method is misnamed, isConcrete() would be more correct, I guess.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants