`bindToController` should work without `controllerAs`

[thorn0]

Do you want to request a feature or report a bug?

bug

What is the current behavior?

If I create a directive with bindToController and without controllerAs, I get $compile:noident: Controller identifier is required.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://plnkr.co or similar (template: http://plnkr.co/edit/tpl:yBpEi4).

http://plnkr.co/edit/lhg3NJABx2DQWnF56rzg

What is the expected behavior?

controllerAs shouldn't be required. If the directive doesn't create a scope, adding controllerAs to it will pollute the outer scope.

What is the motivation / use case for changing the behavior?

Use case: directives that have controller bindings, but have neither a template nor an own scope. What do they need controllerAs for?

Which versions of Angular, and which browser / OS are affected by this issue? Did this work in previous versions of Angular? Please also test with the latest stable and snapshot (https://code.angularjs.org/snapshot/) versions.

1.5.8

[FesterCluck]

Memory leaks is why. There'd be no reference to destroy the controller when the element died. Besides, the result would simply be an isolated scope.

[thorn0]

@FesterCluck Controllers have the $onDestroy hook which is called when the containing scope is destroyed (see #14376). Also it's a supported use case when bindings to a controller are created without creating an isolate scope (e.g. see #13681): ... bindToController: { ...bindings... }, scope: true /* not isolate! */, ... So I'm not asking why, I'm reporting a bug.

[FesterCluck]

Reviewed. You're right. retracted.

[gkalpak]

This is probably an oversight (from when making bindToController work with any kind of scope.
I also wonder why we check for controller.identifier in compile.js#L2712, but not in compile.js#L2721-L2729 😕

[gkalpak]

This is probably an oversight (from when making bindToController work with any kind of scope).

Actually, this has been introduced right from the start (787c5a7), when bindToController was born (and carried over since then). There doesn't seem to be any technical reason for requiring controllerAs.

From what I can understand, this restriction was a way to prevent people from misusing the feature (how?), by making sure that the bindings could still be referenced from the template (why?). From the commit message:

This feature requires the use of controllerAs, so that the controller-bound properties may still be referenced from binding expressions in views.

I would be fine removing this restriction, unless someone can come up with a good reason not to (that I may have missed). PRs welcome 😄

I also wonder why we check for controller.identifier in compile.js#L2712, but not in compile.js#L2721-L2729 😕

It seems like it was accidentally removed in 1c13a4f (although it doesn't have any impact, since without controller.identifier we would never get to that point due to the noident error thrown earlier.