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

isolated-container should call resolver.normalize(fullName) when resolving needs values. #89

Merged
merged 1 commit into from
Sep 23, 2014

Conversation

workmanw
Copy link

This caused an issue for us when using the ES6 resolver and entities with more than one name which are dasherized. Here's an example of something that was previously problematic:

moduleForComponent('tags-edit', 'Unit: Component tags-edit', {
  needs: [
    'controller:tagItem',
    'component:bc-text',
    'template:components/bc-text'
  ]
});

In this case, I was using an itemController for an {{each}} within my component. The each helper in my template will always try to look up the value as camel case (as is convention). However, because I'm using the ES 6 resolver, all names are dashized on the filesystem. So prior to this change, no attempt was made to allow the resolver to normalize the value before looking it up. IE there was no way for 'controller:tag-item' to be registered as 'controller:tagItem'.

I don't anticipate this being a breaking change, even for those using the ES6 resolver.

@stefanpenner
Copy link
Member

The container actually has a plugable hook for a resolver

@workmanw
Copy link
Author

@stefanpenner Are you referring to the container's normalizeFullName hook?

I think the problem here is that we're not actually tying the container up to the resolver. It's hand picking the pre-defined elements and manually registering them with the container. Since it's using resolver.resolve to preform this manual lookup, it never has a chance to normalize the name.

That said, I'm open to any further suggestions.

Also, I'm not sure why the Travis CI build failed.


Edit: To clarify, I meant that since we're manually calling resolver.resolve there isn't another chance to normalize the fullName that is used for lookup.

To use the attached unit test, when controller:helloWorld is declared as a need, that's the actual value passed to resolver.resolve and the controller is not looked up properly.

@stefanpenner
Copy link
Member

@workmanw ah makes sense, you are correct

@workmanw workmanw force-pushed the isolated-container-normalize-needs branch from 566f5bc to d46ab54 Compare September 12, 2014 12:05
@workmanw
Copy link
Author

On another note, I don't know why this build is stalling. It works locally and I don't know much about Travis CI's environment. I'd be happy to fix it if someone can point me in the right direction.

@marcoow
Copy link

marcoow commented Sep 23, 2014

@workmanw: usually it helots to simply trigger the build again manually. Would be great if this got merged, probably fixes #84 as well.

…uld call `resolver.normalize(fullName)` before attempting to lookup and inject `needs` values into the container.
@workmanw workmanw force-pushed the isolated-container-normalize-needs branch from d46ab54 to 42c5986 Compare September 23, 2014 13:52
@workmanw
Copy link
Author

@marcoow I just bumped the commit timestamp and force pushed. It's building again, but looks as if it'll time out. It says: Executed 0 of 0 SUCCESS.

I too would really like to get this in.

rwjblue added a commit that referenced this pull request Sep 23, 2014
`isolated-container` should  call `resolver.normalize(fullName)` when resolving needs values.
@rwjblue rwjblue merged commit f3f8527 into emberjs:master Sep 23, 2014
@workmanw
Copy link
Author

Thanks @rwjblue

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.

4 participants