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

Geolocation service injection and looking up the instance in routes #5

Open
igorpreston opened this issue Jul 7, 2015 · 1 comment
Assignees

Comments

@igorpreston
Copy link
Owner

Currently, in order to use geolocation service you should firstly inject it to the route where you want to use it, like so:

geolocation: Ember.inject.service()

And then look up the instance through geolocation property:

this.get('geolocation').getLocation()...

I deicided to go that way at first because there was an Ember.Service object available in official API that is meant to be used for easy dependency injection and service instantiation. But, when using it this way, you are obliged to inject the service directly through the property to each route you want it to use that quickly becomes annoying.

Proposal: We can change it by creating initializer that will register the factory and inject geolocation service into all routes, so you won't need to inject it yourself through the property and could use it directly like the next:

this.geolocation.getLocation()...

Of course this could break existing API of using geolocation service. But, we can just deprecate the old way of looking up the instance and recommend using new way of looking up a service, until some new major version release where it will be completely removed.

What do you think about it?

@igorpreston igorpreston self-assigned this Jul 7, 2015
@edborden
Copy link

I believe the pattern of declaring services on each route/controller/component/whereever is actually the best practice over using an initializer. It's much easier for someone to understand how a specific piece of code works if they can see each service that is being imported.

Regardless, if you wanted to include a initializer with the addon, you can still do that and let the user simply import/extend themselves into their app if they choose to, instead of forcing it. That's how a few other addons do it.

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

No branches or pull requests

2 participants