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] [Concept] PSR-11 compliant container #19822

Merged
merged 3 commits into from
Jun 29, 2017
Merged

Conversation

deleugpn
Copy link
Contributor

Following up on laravel/ideas#550 I drafted up this commit to draw traction to the debate of whether Laravel container should be PSR-11 compliant or not.

Given that Laravel container already offers a complete set of features, the compliance would probably warrant just the exposure of has and get method. I'm not convinced that the benefits of being compliant out-weight the necessity of maintaining the dependency.

I know it may sound weird to open a PR and discredit it, but like I said my goal is to bring traction to the subject now that PSR-11 was accepted.

@deleugpn deleugpn force-pushed the psr-11 branch 3 times, most recently from fc00f73 to 6de2138 Compare June 29, 2017 12:20
@thecrypticace
Copy link
Contributor

Does the spec explicitly prevent has() from returning true for autowired objects? Ex: I didn't bind an object to the container but it can still be built by it.

If so then this implementation looks fine to me. If not, I wonder if it might be worth checking to see if an object is buildable (idk try to create it?) by catching the BidingResolutionException and turning it into the EntryNotFoundException if need be. Not sure how much of a perf. hit that'd be though.

*/
public function has($id)
{
return $this->getConcrete($id) != $id;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably call ->bound instead.

@deleugpn
Copy link
Contributor Author

@thecrypticace from the PSR

or throw a NotFoundExceptionInterface if the identifier is not known to the container.

I'm interpreting this as: If you want to build something concrete, just build it yourself. The container will throw exception if the given identifier is unknown to itself.

I'm not sure if my interpretation is correct.

@thecrypticace
Copy link
Contributor

Yeah, based on that, your interpretation seems sensible. 👍

@taylorotwell taylorotwell merged commit 3077e58 into laravel:master Jun 29, 2017
@antonkomarev
Copy link
Contributor

Thanks!

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.

4 participants