johnpapa / angular-styleguide

Angular Style Guide: A starting point for Angular development teams to provide consistency through good practices.
http://johnpapa.net
MIT License
23.87k stars 4.15k forks source link

controllerAs syntax name conflicts #577

Open jusefb opened 8 years ago

jusefb commented 8 years ago

Hi,

Something I have discovered today with regards to the controllerAs syntax in Angualr 1. In my Angular 1 application I try to apply the component architecture by using directives to define individual components. The directive has its own controller with controllerAs: 'vm'. I also use angular-ui router for navigating between views. What I have discovered is that if I have a route defined with controllerAs: 'vm' and then I use a directive within the view that defines controllerAs: 'vm', the vm of the parent controller gets overwritten. The behaviour is the same even with isolated scope on the directive. If however I specify different names in the controllerAs parameter for router and the directive (ex controllerAs: 'componentVm') all works fine.

I was wondering if I am doing something wrong? If not and if this is the intended behaviour it would probably be a good idea to add something to the styleguide mentioning potential controllerAs name conflicts when using component architecture in Angular 1 with directive specific controllers.

Kind regards

Jusef

kylecordes commented 8 years ago

The style guide already says:

"Note: When working with larger codebases, using a more descriptive name can help ease cognitive overhead & searchability. Avoid overly verbose names that are cumbersome to type."

So I think @johnpapa has this covered well. Perhaps a slight improvement would be to mention that using nested controllers counts as "larger codebases".

(However... if you're building with well isolated components, you probably have few very explicitly nested controllers even in large projects.)

zachlysobey commented 8 years ago

That section of the style-guide is quite new, and probably could be expanded/improved a bit IMO. See this conversation for more info: https://github.com/johnpapa/angular-styleguide/issues/462

johnpapa commented 8 years ago

Feel free to make a PR to add a short bit on nesting controllers, if you think it wil help

josefromsanjose commented 8 years ago

I'm a bit new to Angular and just transition from angular 1.4.8 to 1.5.0. I had the same problem, when nesting a directive. The parent directive would controllerAs property as 'vm' was overwritten by the child directive. adding an isolate scope to the child fixed the issue for me, but I can see where these can cause a problem.

johnpapa commented 8 years ago

Feel free to make a PR to add a short bit on nesting controllers, if you think it wil help

karlhorky commented 8 years ago

In my experience with naming Angular 1 controllers, out of the available options it makes the most sense to rename them after the component itself. This would only fall down with nesting of the same type of component inside one type, which in experience with a large app does not come up often.

karlhorky commented 8 years ago

So to clarify, I would suggest changing this rule in the style guide to recommend using the name of the component.

johnpapa commented 8 years ago

Use vm until it's not clear. Then use a more descriptive name. That's what I like.

paulashton commented 7 years ago

I had the same issue as the one jusefb reported in November 2015. We had changed some directives to be controllerAs: 'vm'. In one case a controllerAs: 'vm' did not have an isolated scope (but in all other cases the controllerAs: 'vm' directives did have an isoloted scope). Adding scope: {}, to the one controllerAs: 'vm' directive that didn't have an isolated scope fixed the problem.