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.6] Adds bulk binding ability to service providers during registration #21961

Merged
merged 1 commit into from
Nov 10, 2017

Conversation

dallincoons
Copy link
Contributor

@dallincoons dallincoons commented Nov 5, 2017

This provides a convenient way to add many trivial bindings at once which may clean up service providers which involve many container bindings.

@dallincoons dallincoons changed the title Adds bulk binding ability to service providers during registration [5.5] Adds bulk binding ability to service providers during registration Nov 5, 2017
@Aferz
Copy link
Contributor

Aferz commented Nov 5, 2017

Simple & Easy. Good one!

@sisve
Copy link
Contributor

sisve commented Nov 5, 2017

You describe this feature as if it can be used to bind many things at once. So, ...

  1. How do I use this with factory functions?
  2. How do I specify the $shared parameter for $app->bind($abstract, $concrete = null, $shared = false)

@Aferz
Copy link
Contributor

Aferz commented Nov 5, 2017 via email

@dallincoons
Copy link
Contributor Author

Exactly, it's meant to be a clean way to create many trivial bindings.

@taylorotwell
Copy link
Member

No plans to add this to core.

@taylorotwell taylorotwell reopened this Nov 6, 2017
@taylorotwell
Copy link
Member

Re-opening this after sleeping on it. Reconsidering.

@Aferz
Copy link
Contributor

Aferz commented Nov 6, 2017

This could be a good feature to simplify code in ServiceProviders as EventsServiceProvider do with listeners and subscribers.

@taylorotwell taylorotwell changed the base branch from 5.5 to master November 10, 2017 17:03
@taylorotwell taylorotwell merged commit b260212 into laravel:master Nov 10, 2017
@Aferz
Copy link
Contributor

Aferz commented Nov 10, 2017

Good addition! Congratulations @dallincoons

@devcircus
Copy link
Contributor

Seems like "singletons" and "bindings" would be more consistent here, instead of "singletons" and "bind".

@roberto-aguilar
Copy link
Contributor

@dallincoons i was about to use this feature, but it didn't work and after some time, i found that this was merged into 5.6 instead of 5.5, can you update your PR title?, thanks!

/cc @taylorotwell

@dallincoons dallincoons changed the title [5.5] Adds bulk binding ability to service providers during registration [5.6] Adds bulk binding ability to service providers during registration Dec 3, 2017
@antonkomarev
Copy link
Contributor

@devcircus it seems to be changed right as you proposed (I haven't seen who contributed it, sorry if it was you).

if (property_exists($provider, 'bindings')) {
foreach ($provider->bindings as $key => $value) {
$this->bind($key, $value);
}
}
if (property_exists($provider, 'singletons')) {
foreach ($provider->singletons as $key => $value) {
$this->singleton($key, $value);
}
}

@devcircus
Copy link
Contributor

Yeah I noticed. Wasn't me, but I like it.

@roberto-aguilar
Copy link
Contributor

@a-komarev and @devcircus, it was @taylorotwell in 81e29b1 and i agree with you guys, it is way better 🤘

And by the way, thanks for the title update 😄

MilesPong added a commit to MilesPong/indigo that referenced this pull request Mar 26, 2018
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.

None yet

7 participants