"Return" in directive's controllers no longer works in 1.3.0-rc.0

[ocombe]

I used to use return in my directive's controllers, but it stopped working in 1.3.0-rc.0.

For example, here is how I used to write a directive:

angular.module('MyApp', []).directive('test', ['$rootScope', function($rootScope) {
        return {
            restrict: 'AE',
            replace: true,
            transclude: true,
            template: '<div>test: {{ test }}</div>',
            controller: ['$scope', '$element', '$attrs', function($scope, $element, $attrs) {
                return {
                    register: function() {
                        $scope.test = 'it works';
                    }
                };
            }],
            link: function($scope, $element, $attrs, testCtrl) {
                testCtrl.register();
            }
        };
    }]);

But now testCtrl in the link function returns an empty object {}.

I fixed my directives by using this but I'm not sure why the return doesn't work anymore ?
Here is how I had to write it:

angular.module('MyApp', []).directive('test', ['$rootScope', function($rootScope) {
        return {
            restrict: 'AE',
            replace: true,
            transclude: true,
            template: '<div>test: {{ test }}</div>',
            controller: ['$scope', '$element', '$attrs', function($scope, $element, $attrs) {
                this.register = function() {
                    $scope.test = 'it works';
                };
            }],
            link: function($scope, $element, $attrs, testCtrl) {
                testCtrl.register();
            }
        };
    }]);

Here is the plnkr: http://plnkr.co/edit/sSJGIQIM2D9vIZmD42ya?p=preview

[Narretz]

@caitp Could this be because of 5f3f25a? Since it works in beta.19 (I didn't check the commit, just a guess)

[ocombe]

I checked the changelog and didn't see any indication of a modification on directive's controllers, but this one seems like a good candidate.
The problem is that this syntax is also used in multiple libs that I use, I get undefined is not a function everywhere now, my app is unusable... If it was just me, I would update my own code.

[caitp]

@Narretz yes, that commit is responsible for this.

@ocombe don't return new objects from your constructor. this.register = ... or extend(this, ...) instead, thanks

[ocombe]

That's what I'll do from now on, but if you intend to keep it this way it is a serious breaking change. A lot of libs won't be compatible with 1.3.0 because of that...

[caitp]

Any library which is doing that, is doing it wrong. You don't "return a value from a constructor", that is not how we do things. You aren't going to write a C++ class with a constructor which returns some other object, because the compiler won't let you, and it would be a really weird and incorrect thing to do anyways.

So basically, don't do the wrong thing, do the right thing and construct your instance correctly

Also, they won't likely be compatible with 1.3.0 without some effort anyways, since a lot of things have changed

[Narretz]

It's still a breaking change, although I agree the usage is weird. But we have put breaking change notes for undocumented / unusual behavior in the changelog before, so I guess we should put a note in.

[caitp]

Yeah, I'll add a note

[caitp]

We're not going to call this a regression, because it's not --- honestly it never made sense to support that feature in the first place, and it definitely doesn't make sense to use that feature, under any circumstances. No serious programming language would ever let you do this.

But I've added a detailed note about this, so lets get that in the changelog asap. @Narretz can you give that a quick look and see if it looks ok?