Controller prototype / instance methods

[vojtajina]

There is a different this (inside controller method - different scope object), if method is defined in instance than if defined in prototype. And the method is invoked from child scope element (i.e. element created by repeater).

At least we should mention that in doc, as it's bit confusing.

There is a gist with example: https://gist.github.com/907914

Reason

Prototype methods are binded, so that they are always called with root (for given controller) scope as this. See https://github.com/angular/angular.js/blob/master/src/Scope.js#L583
Instance methods (defined in constructor) are only applied, so that they are called with related scope as this. (that's child scope inside repeater)

Possible solution

Unify the API - do we need to bind the prototype methods ??? I hope we don't.
User has to access parent scope through $parent or $root property.

[IgorMinar]

whoa... this is crazy...

I guess it makes sense when you think about how method dispatch and this binding in methods works in JavaScript, but it's not intuitive at all:

function MyCtrl(id) {
  this.$id = id;
  this.getId = function() { return this.$id }
}
> undefined

p = new MyCtrl(23)
> MyCtrl

c = {$id: 444}
> Object

c.__proto__ = p
> MyCtrl

c.$id
> 444

c.getId()
> 444

p.getId()
> 23

A solution to this problem would be using var self = this in constructor and using self in all instance methods instead of this.

I think that binding proto methods makes sense, because that's the behavior you expect not the crazy inherited instance method behavior.

[IgorMinar]

We've been playing with the idea of injecting scope into controllers instead of binding it. I think that would solve this problem as well:

function MyCtrl(scope) {
  this.instanceFn = function() {
    return scope.$id;
  }

  //for proto methods
  this.scope = scope;
}

MyCtrl.prototype.protoFn = function() {
  return this.scope.$id;
}

[vojtajina]

I hope the first code example is easy to understand for everyone who knows, how JS works.
Agree, that using var self = this would solve the problem.

Injecting scope would solve it as well, as it's almost the same approach (instead of using var self = this, you get the variable self(scope) as a parameter)

Actually, I like the idea "injecting the scope" quit a bit. I believe it would solve the problem with testing controllers without scope ! You can inject easily whatever you want then !

(It would make a sense to set the scope property outside the controller - automatically, when controller is being instantiated. (to save one line for users, when writing their controllers and want to access scope in prototype methods))

I haven't thought about injecting scope enough, this is really quick response, so maybe I'm missing something, but I will think about it more, it looks promising. Not because of this problem, but because of testing controllers.

[mhevery]

So injecting scope, is a long term solution, which looks attractive, but we need to think about all of the implications.

We have to bind the methods on the prototype, since we have to copy them to scope. The binding is optional, but the copying is a must.

I think that we could make the parser smarter, which would fix this problem. Currently the parser just dispatches the method as it finds it, but it could do extra work for functions, and dispatch them on the instance which they are defined, and I think that would solve the issue.

Here is where the function call is applied: https://github.com/angular/angular.js/blob/master/src/Scope.js#L76
if we made this code a bit smarter, so that it would search for the correct instance and then apply, it would solve our problems once and for all, and then we would not have to bind the prototype functions and the two would behave in the same predictable way.

[vojtajina]

It's funny, but I've found this "feature" useful sometimes - if you know, how it works, you can take advantage of it...
Maybe, we should just document it as feature :-D

[mhevery]

no, its very confusing, especially to people who are new to JS.