-
Notifications
You must be signed in to change notification settings - Fork 104
Resolve Geocoder dependency via Class name #125
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
Conversation
1 similar comment
Hi @anam-hossain , thank you for submitting this. I will take a look at it, but it might take a week or two, as I won't be available this weekend because of holidays. |
Any news on this? Thank you! |
@sololance Sorry for the delay. Taking a look at it now. |
Looks like this doesn't work perfectly.
However this does work:
|
@sololance Please provide a complete code snippet and stack trace of the error in a new issue, as this PR is already merged. :) Thanks! |
@mikebronner well, this only doesn't work in laravel tinker, anyways thank you ;) |
But it works when you use it in an app? I'm unclear now if you have a problem or not? |
@mikebronner exactly, it does work when called in controllers, however doesn't work when called in laravel tinker. I'm using tinker to see what responses are returned and handle them properly before writing some code in controllers. |
Thanks for clarifying. Yea, tinker is a bit special, no guarantees there. :) |
@sololance The reason it's not working because you are trying to create a new instance of The following should work: $providerAndDumperAggregator = (new ProviderAndDumperAggregator)
->registerProvidersFromConfig(collect(config("geocoder.providers"))); Or if you want to avoid the above settings, use Laravel container to retrieve the geocoder instance: use Geocoder\Laravel\ProviderAndDumperAggregator as Geocoder;
$geocoder = app(Geocoder::class); |
Thanks @anam-hossain ! So while you're already retrieving it from the IoC, you might as well use the original |
The PR will enable resolving dependency via class name. This is a minor fix and will not break any existing code.
At the moment,
app('geocoder')
is the only way to resolve the geocoder dependency and does not support dependency injection via controller.The PR will allow the following:
The PR also tests friendy as it will allow developer to swap dependency when needed.