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

Add a command to replace needs with injection #87

Merged
merged 1 commit into from
Oct 6, 2015

Conversation

paddyobrien
Copy link
Collaborator

Relates to #85

Replace:

needs: ['foo'],
fooModel: Em.computed.alias('controllers.foo.model')

With

fooController: Em.inject.controller('foo'),
fooModel: Em.computed.alias('fooController.model')

I had a need for this in our internal project so I took it on today. I'm completely new to the recast AST API so I may be using it sub-optimally. Also some of the decisions I've made with regard to renaming might only make sense in the context of our project.

I was unsure how to approach the template aspect of this so I've ignored it.

I'm happy to change anything that seems incorrect.

@abuiles
Copy link
Owner

abuiles commented Oct 5, 2015

Thanks! I'll review this latter today and get it in :)

@abuiles
Copy link
Owner

abuiles commented Oct 5, 2015

@paddyobrien

Also some of the decisions I've made with regard to renaming might only make sense in the context of our project.

Could you elaborate more about those ones?

@GavinJoyce
Copy link
Collaborator

@abuiles I think he's referring to both the addition of the Controller suffix to avoid naming collisions with other properties and the support for handling duplicate leaf route names:

import Em from 'ember';

export default Ember.Controller.extend({
  fooBarController: Em.inject.controller('foo/bar'),
  bazBarController: Em.inject.controller('baz/bar')
});

@paddyobrien
Copy link
Collaborator Author

Yep exactly that.

On Monday, October 5, 2015, Gavin Joyce [email protected] wrote:

@abuiles https://github.com/abuiles I think he's referring to both the
addition of the Controller suffix (to avoid naming collisions) and
support for handling duplicate leaf route names:

import Em from 'ember';
export default Ember.Controller.extend({
fooBarController: Em.inject.controller('foo/bar'),
bazBarController: Em.inject.controller('baz/bar')
});

[image: image]
https://cloud.githubusercontent.com/assets/2526/10291604/a6f19d5c-6ba2-11e5-9328-30a1b83533ce.png


Reply to this email directly or view it on GitHub
#87 (comment).

@abuiles
Copy link
Owner

abuiles commented Oct 6, 2015

@paddyobrien finally went through this and it looks great, I didn't realized but I've been also using somethingController: Ember.inject.controller('something') so let's ship it like it.

Can you do me a last favor, can you add the command to the readme with a brief description, I'll release after that, 👍

@paddyobrien
Copy link
Collaborator Author

@abuiles done, thanks for the quick review!

abuiles added a commit that referenced this pull request Oct 6, 2015
Add a command to replace needs with injection
@abuiles abuiles merged commit 15c5eec into abuiles:master Oct 6, 2015
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.

3 participants