igorpreston / ember-cli-geo

Geolocation service for Ember.js web apps
MIT License
50 stars 16 forks source link

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

Open igorpreston opened 9 years ago

igorpreston commented 9 years ago

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?

edborden commented 8 years ago

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.