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

Conversation

Projects
None yet
4 participants
@deleugpn
Contributor

deleugpn commented Jun 29, 2017

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 added some commits Jun 29, 2017

@thecrypticace

This comment has been minimized.

Show comment
Hide comment
@thecrypticace

thecrypticace Jun 29, 2017

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.

Contributor

thecrypticace commented Jun 29, 2017

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.

@deleugpn

This comment has been minimized.

Show comment
Hide comment
@deleugpn

deleugpn Jun 29, 2017

Contributor

@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.

Contributor

deleugpn commented Jun 29, 2017

@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

This comment has been minimized.

Show comment
Hide comment
@thecrypticace

thecrypticace Jun 29, 2017

Contributor

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

Contributor

thecrypticace commented Jun 29, 2017

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

@taylorotwell taylorotwell merged commit 3077e58 into laravel:master Jun 29, 2017

1 check passed

continuous-integration/styleci/pr The StyleCI analysis has passed
Details
@antonkomarev

This comment has been minimized.

Show comment
Hide comment
@antonkomarev

antonkomarev Jun 29, 2017

Contributor

Thanks!

Contributor

antonkomarev commented Jun 29, 2017

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment