davej / angular-classy

Cleaner class-based controllers with Angular 1
http://davej.github.io/angular-classy/
813 stars 27 forks source link

Support for controllerAs? #4

Closed jeffbcross closed 10 years ago

jeffbcross commented 10 years ago

By default, classyController adds custom methods to $scope. It's often useful to use controllerAs to be able to explicitly call a method on a specific controller, rather than think about which scope context I'm executing within. E.g.:

<button ng-click="userController.login()">Log in</button>

Will there be an option to allow custom methods to be set on this instead of $scope?

davej commented 10 years ago

ControllerAs works fine for me like this (calling tc.addTodo()):

<section id="todoapp" ng-controller="TodoCtrl as tc">
  <form id="todo-form" ng-submit="tc.addTodo()">
    <input id="new-todo" placeholder="What needs to be done?" ng-model="newTodo" autofocus>
  </form>
</section>

Or am I misunderstanding what you're trying to do?

jeffbcross commented 10 years ago

Disclaimer: I'm only going off documentation, haven't actually experimented with the lib :)

How do you declare that you only want a method or property on controller's this instead of $scope?

davej commented 10 years ago

At the moment you can't do that. I've toyed with the idea of adding an options object to turn on/off features. Perhaps something like:

app.classyController({
    name: 'AppController',
    inject: ['$scope', '$location'],
    options: {
        autoBindToScope: false
    },
    // ...
)};

Does that work or can you think of a better way of doing it?

jeffbcross commented 10 years ago

The first thought that comes to mind (this may be a stupid idea) is to add a prototype property that would begin construction with the provided prototype.

app.classyController({
  name: 'LoginController',
  prototype: { 
    login: function () {
      this.loginService.login(this.$scope.user);
    }
  }
});
jeffbcross commented 10 years ago

I guess there's kind of a delta between how people are building controllers today and how we're starting to recommend people build controllers (use this and controllerAs for most fields). IMHO, defaulting to this/controllerAs, along with your options object proposal to add fields to $scope would be the best forward-thinking API.

davej commented 10 years ago

Adding a prototype property makes things a little bit complicated for newcomers IMO (as it's another thing to explain). Angular developers are familiar with how $scope works, they may or may not be familiar with how controllerAs and this work together. So I'd like the default to support the lowest common denominator and give power users the option to back out if it suits them.

I didn't realise that controllerAs was the new recommended way of doing controllers. To be honest, I hate the controllerAs feature, not because it's bad by itself (it's not), but because it now means that how you write your controller is tied to how it's being used by the view.

Feel free to make a counter-argument, I haven't 100% made my mind up about this.

jeffbcross commented 10 years ago

Agreed on adding prototype property being a bit much. I don't disagree with your leaning; it's your library, and it should reflect your taste (and shouldn't try to do too many things).

davej commented 10 years ago

Ok, I've added this feature to the develop branch. There are two ways that you can prevent functions from being automatically added to the $scope, you can do it at the module level or the class level.

Module level:

var app = angular.module('app', ['classy']);

app.classy.options.controller = {
    addFnsToScope: false
};
// Classy controllers will no longer automatically add functions to the `$scope`

or at the class level:

app.classy.controller({
    name: 'TodoCtrl',
    inject: ['$scope', '$location', 'todoStorage', 'filterFilter'],
    __options: {
        addFnsToScope: false
    }
    // ...
});

Obviously setting options at the class level will override any options set at the module level.

Demo of TodoMVC using ControllerAs is here: http://davej.github.io/angular-classy/examples/todomvc/index-as-controller.html#/ JS is here: http://davej.github.io/angular-classy/examples/todomvc/js/controllers/todoCtrl-asController.js

It's worth being aware that when you use controllerAs with classy controllers then you will expose your dependencies to the view.

jeffbcross commented 10 years ago

The exposed dependencies tradeoff is already accepted by most folks who use controllerAs (at least the folks I know).

madbarron commented 10 years ago

Using controllerAs, is there a way to make private properties for my controller? When I add them to this, my methods can access them, but I want to prevent my views from seeing them.

davej commented 10 years ago

No, afraid not, that's the trade-off with doing controllerAs. You could manually clone your dependencies to a harder to find object, something like this._deps (i.e. this._deps.$http) and set the existing reference to undefined. If you're working off the develop branch then you could even create a plugin for doing that.