angular / angular-hint

run-time hinting for AngularJS applications
362 stars 45 forks source link

Feat: warn about global controllers #8

Closed btford closed 10 years ago

btford commented 10 years ago

$controller will load modules off of window if it can't find the controller elsewhere.

This is okay for tiny, illustrative examples, but otherwise not a great practice.

here's the offending js:

|| getter($window, constructor, true);

You should be able to decorate $controller to check if the || getter($window, constructor, true); code path was taken.

ealtenho commented 10 years ago

@btford, would this functionality be implemented in angular-hint, or should it be a separate module?

caguillen214 commented 10 years ago

@ealtenho @btford

Although this isn't quite the same functionality of hint-dom, they both relate to controller best practices. It seems like to avoid having an unnecessary amount of angular-hint-* modules, we should consider grouping them together a little differently.

I think this should be a feature in erin's -dom and that we should maybe change the name to hint-controller since it deals more with bad practices of the controller and less about the dom itself. Personally, grouping like this makes more sense to me. Thoughts?

ealtenho commented 10 years ago

@caguillen214 @btford Actually, this was why I had asked about it being a separate module, since it does sound related to my implementation. Still, I think it is important for angular-hint-dom to convey that the best practice there is about not manipulating the dom. I'm not sure what the compromise should be there. @btford thoughts?

btford commented 10 years ago

I think it would be better to have this as a separate module for a variety of reasons:

There are a few different "best practices" for controllers, so I'm thinking this could be the start of an angular-hint-controllers repo

ealtenho commented 10 years ago

I think another module sounds right to me. I will look into implementing this feature.

ealtenho commented 10 years ago

My local repository for beginning development: https://github.com/ealtenho/hint-controller

ealtenho commented 10 years ago

Initial module: https://github.com/ealtenho/hint-controller/commit/6f141d9292df982a6a54fd13915a588a7c5d00bb

ealtenho commented 10 years ago

@btford, I think I have a working prototype of this feature, but I'm concerned my tests are insufficient: https://github.com/ealtenho/hint-controller/issues/2. I'm also unsure of how we plan to integrate this with other angular-hint repositories?

ealtenho commented 10 years ago

12 @btford

caguillen214 commented 10 years ago

I believe this has been addressed and should be closed