Can't use a controller factory with ng-controller and 'as' syntax

[Morgul]

In upgrading from 1.3.0-beta.17 to 1.3.1, I had part of my application stop working. Digging into it, it turned out that when I used ng-controller="Controller as ctrl", none of the properties or functions on the object were defined (though, the controller itself seemed to be; this needs a bit more investigation).

Playing around in plunkr, I discovered that this bug only appears when you define your controller inside a factory function:

http://plnkr.co/edit/EaMKReAalnAWVLKPpPz5?p=info

As you can see, a standard controller function works fine. However, all of our controllers are written with factory functions, so we don't have to add any injectables as members of the controller.

As far as I'm aware, this has worked in Angular 1.2.X. That being said, I have only tested with 1.3.0-beta.27, 1.3.1 and 1.3.4.

---

Confirmed Broken Versions: 1.3.1, 1.3.4 Known Working: 1.3.0-beta.17

---

[pkozlowski-opensource]

This is a known breaking change in 1.3.0-RC.0 (https://github.com/angular/angular.js/blob/master/CHANGELOG.md#breaking-changes-9):

$compile: due to 5f3f25a1, The returned value from directive controller constructors are now ignored, and only the constructed instance itself will be attached to the node's expando. This change is necessary in order to ensure that it's possible to bind properties to the controller's instance before the actual constructor is invoked, as a convenience to developers.

[Morgul]

I'm not sure I follow, actually. First, this isn't in a directive, so it wasn't clear that change applied.

If I follow what you're saying here, then what's going on is that ctrl from my example is being set to the factory function, not it's return value? So, in reality, creating a controller inside a factory function simply is no longer supported?

Am I missing something?

[pkozlowski-opensource]

@Morgul what you can't do any more is to return an object from a controller's constructor function. What you pass to the .controller call is a constructor function, not a factory function.

[Morgul]

Alright, I get that (though I think the docs should be a bit more explicit about the implications of this change, as it's turning into really large change for me, and may be for others).

My question now becomes, how should I handle the following?

// This is a generalized version of one of the controllers from my application
function ExampleControllerFactory($scope, $timeout, MyService1, MyService2, MyService3, $modal)
{
    function ExampleController()
    {
        this.results = [];
        this.foobar = "stuff!";
    } // end ExampleController

    ExampleController.prototype.doStuff = function()
    {
        // Let's assume this is a pretty long function
        MyService1.doMoreStuff(this.foobar, function(results)
        {
            $scope.thing = {
                prop1: MyService2.foo,
                results: results.concat(this.results)
            };

            $timeout(function()
            {
                $modal.$close();
            }, 250);
        });
    }; // end doStuff

    ExampleController.prototype.doOtherStuff = function()
    {
        // Let's assume another long function
        MyService3.counter++;
    }; // end doOtherStuff
} // end ExampleControllerFactory

angular.module('example.controllers').controller('ExampleController', [
    '$scope',
    '$timeout',
    'MyService1',
    'MyService2',
    'MyService3',
    '$modal',
    ExampleControllerFactory
]);

The simple solution (which we had been using previously) is to set all of the injectables on this in the constructor function. However, that leads to needing to manage this issues, and clutters the constructor function(some of our controllers have 15 injectables).

Using a class created inside a factory function solved all of our issues, and made the code very clean, less error prone, and easily testable.

Now, unless I'm missing something, the only other option would be for me to put doStuff and doOtherStuffon the scope, but that can make the code somewhat more difficult to read, and technically defeats the purpose of using as.

[pkozlowski-opensource]

The simple solution (which we had been using previously) is to set all of the injectables on this in the constructor function. However, that leads to needing to manage this issues, and clutters the constructor function(some of our controllers have 15 injectables)

This would be the way to go. I know that you wan't like what I'm going to say but having 15 injectables in one controller sounds like this controller is way too big / does way too much.

[Morgul]

I won't disagree; it's some rather complex code. I suppose a lot of it could be moved into a singular service, and the controller could become a thin wrapper around service calls.

It just really sucks that this was changed like this; I can understand the desire for the new feature, but it feels like a fundamental change to something that's always worked, but I'm getting the feeling that this style of usage is in the minority.