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

Skip type normalization #8

Merged
merged 1 commit into from
Dec 11, 2013
Merged

Conversation

teddyzeenny
Copy link
Contributor

Since we cannot normalize a standalone type, type injections are not normalized. However types are being normalized on registration. This causes a mismatch between the type for injection and the registered type.

For example: container.register('dataAdapter:main') gets registered as data-adapter:main, but container.injection('dataAdapter', 'store', 'store:main') injects into dataAdapter and not data-adapter.

@rwjblue
Copy link
Member

rwjblue commented Nov 14, 2013

👍

@stefanpenner
Copy link
Contributor

dataAdapter should be data-adapter to start with.

@teddyzeenny
Copy link
Contributor Author

Valid point, I'm going to update it to dasherized syntax asap. Though that doesn't change the fact that normalization is currently inconsistent. Type is sometimes being normalized and sometimes not.

@stefanpenner
Copy link
Contributor

i sorta want to move to a very strict resolver, that basically throws an error if it gets some invalid name. Rather then doing most of the normalizing. Although I am not sure if this is just going to be annoying for people

@matthewlehner
Copy link

Is there any chance that this can be merged in for now, just while we move towards a more strict resolver?

This PR makes the data portion of the Ember Inspector work with Ember App Kit. I'm coming over to EAK after using Yeoman mainly and this was a bit of a surprise. I'm a HUGE fan of EAK otherwise. Everything just makes sense.

stefanpenner added a commit that referenced this pull request Dec 11, 2013
@stefanpenner stefanpenner merged commit 5b02c01 into ember-cli:master Dec 11, 2013
@stefanpenner
Copy link
Contributor

sounds good for now. We must fix our internal (ember-data and ember's) incorrect naming conventions asap though.

@WMeldon
Copy link

WMeldon commented Jan 7, 2014

Any chance we could get this change pushed to /dist? Would love to get bower working again without manual patching for stefanpenner/ember-app-kit#263

@rwjblue
Copy link
Member

rwjblue commented Jan 7, 2014

@WMeldon - I just checked, and I believe that the latest changes are reflected in /dist. Can you double check?

@WMeldon
Copy link

WMeldon commented Jan 8, 2014

@rjackson Just ran a bower update and the correct version came through. Everything appears to be up to date. Thanks!

@adimitri
Copy link

Do we need to update the chrome extension to a newer version or from master? I'm currently on 0.0.6 and I did bower update but the data tab of the inspector is still empty.

@stefanpenner
Copy link
Contributor

@adimitri we are still working to fix this. @lukemelia and myself discussed this last night, and he has begun a fix. We hope to have it out in the wild soon.

kratiahuja pushed a commit to kratiahuja/ember-resolver that referenced this pull request Aug 5, 2016
add `testem` to run tests more easily.
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.

6 participants