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

Avoid Ember.inject.service() for optional i18n #153

Merged
merged 1 commit into from
Mar 2, 2017

Conversation

rwjblue
Copy link
Contributor

@rwjblue rwjblue commented Mar 2, 2017

When an object that utilizes an Ember.inject.service() is looked up the first time, Ember's DI system confirms that the service being injected is actually present and known to the system. If the service is not present it throws an error.

In the use case of this addon, i18n is intended to be an optional service and may not be present. If we use Ember.inject.service() in this context we loose the ability for it to be optional.

This commit changes things around to avoid using Ember.inject.service and instead uses getOwner(this).lookup('service:i18n') (which returns undefined if the service is not found).

When an object that utilizes an `Ember.inject.service()` is looked up the 
first time, Ember's DI system confirms that the service being injected is
actually present and known to the system. If the service is not present
it throws an error.

In the use case of this addon, `i18n` is intended to be an optional service
and may not be present. If we use `Ember.inject.service()` in this context
we loose the ability for it to be optional.

This commit changes things around to avoid using `Ember.inject.service`
and instead uses `getOwner(this).lookup('service:i18n')` (which returns
`undefined` if the service is not found).
@spruce
Copy link
Member

spruce commented Mar 2, 2017

Thanks for the PR 👍

@spruce spruce merged commit 0a11653 into piceaTech:master Mar 2, 2017
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.

2 participants