[Module Unification] Introduce source to injections #16240
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mixonic
changed the title
mixonic
force-pushed
the
injection-sources
branch
from
d4c02aa to
3f89eb9
Compare
mixonic
force-pushed
the
injection-sources
branch
12 times, most recently
from
3065c44 to
27a3b80
Compare
mixonic
force-pushed
the
injection-sources
branch
from
27a3b80 to
bd1c638
Compare
|
I think this is ready to go in. All the work is behind a feature flag. The implementation is in line with the Module Unification Namespaces RFC emberjs/rfcs#309 I'm going to look at rebasing #15967 on top of this. |
rwjblue
approved these changes
Mar 4, 2018
| @@ -35,5 +36,10 @@ function injectedPropertyGet(keyName) { | |||
| assert(`InjectedProperties should be defined with the inject computed property macros.`, desc && desc.type); | |||
| assert(`Attempting to lookup an injected property on an object without a container, ensure that the object was instantiated via a container.`, owner); | |||
|
|
|||
| return owner.lookup(`${desc.type}:${desc.name || keyName}`); | |||
| let specifier = `${desc.type}:${desc.name || keyName}`; | |||
| if (desc.source) { | |||
There was a problem hiding this comment.
I don’t fully grok why we need this conditional. Seems to be that calling lookup like owner.lookup(specifier, { source: undefined }) (which is what would happen if desc.source is not present) would be fine. What am I missing?
| * configuration (such as a singleton type that has local lookup). | ||
| */ | ||
| ['@test Services can be injected with same name, one with source one without, and share an instance'](assert) { | ||
| let routeASource = 'service:src/ui/routes/route-a/controller'; |
There was a problem hiding this comment.
Seems odd to have the source here be prefixed with service: (the actual type is controller)
Ember templates use a `source` argument to the container. This hints where to look for "local lookup", and also what namespace to look in for a partial specifier. For example a template in an addon's `src` tree can invoke components and helpers in that tree becuase it has a `source` argument passed to lookups. This introduces the same functionality for service injections. A service can now accept a `source` argument and its lookup will apply to the namespace of that file. Additionally, refactor away from using a `source` argument to the `resolve` API of resolvers. Instead leveral the `expandLocalLookup` API to pass the source and get an identifier that can be treated as canonical for that factory.
mixonic
force-pushed
the
injection-sources
branch
from
bd1c638 to
28aa862
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ember templates use a
sourceargument to the container. This hints where to look for "local lookup", and also what namespace to look in for a partial specifier. For example a template in an addon'ssrctree can invoke components and helpers in that tree because it has asourceargument passed to lookups.This introduces the same functionality for service injections. A service can now accept a
sourceargument and its lookup will apply to the namespace of that file.Additionally:
expandLocalLookupto for the resolution of local item instead of a new argument toresolveon resolvers.resolverCacheKey, no longer needed.