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.

[Narretz]

The requirement for controllerAs might come from the fact that bindToController is most likely / should be used with directive with an isolate scope, which is kind of unlikely to be used without a template.

And I just realized, with bindToController, you can have "bindings" to a directive that doesn't create an isolate scope: http://plnkr.co/edit/INKSyrVyT11I8L8F19Dw?p=preview
Which is cool, because you can define your directive's "dependencies" explicitly, which isn't possible when you bind to scopes. I wonder if this is an unintentional side-effect of the whole bindToController thing?

[thorn0]

And I just realized, with bindToController, you can have "bindings" to a directive that doesn't create an isolate scope

How is it possible that you just realized this? It was you who authored #13681. :)

[Narretz]

That doesn't mean I understood the implications. It was just words. 😁

[rgolea]

Hi there! This feature has broken all the applications that were using this inside their directives. The bindToController property is not binding to the controller anymore. Can you guys fix this? Thanks

[gkalpak]

I don't see how this could have broken anything (since it only allowed a usecase that would throw an error). Can you please create a new issue and be more specific about what has been broken (e.g. provide a reproduction)? Thanks