Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit 62d514b

Browse files
caitpcaitp
committed
fix($compile): respect return value from controller constructor
The return value of the controller constructor is now respected in all cases. If controllerAs is used, the controller will be re-bound to scope. If bindToController is used, the previous binding $watches (if any) will be unwatched, and bindings re-installed on the new controller.
1 parent 35498d7 commit 62d514b

File tree

3 files changed

+152
-25
lines changed

3 files changed

+152
-25
lines changed

‎src/ng/compile.js

+30-19
Original file line numberDiff line numberDiff line change
@@ -1373,15 +1373,10 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
13731373
if (nodeLinkFn.scope) {
13741374
childScope = scope.$new();
13751375
compile.$$addScopeInfo(jqLite(node), childScope);
1376-
var onDestroyed = nodeLinkFn.$$onScopeDestroyed;
1377-
if (onDestroyed) {
1378-
nodeLinkFn.$$onScopeDestroyed = null;
1379-
childScope.$on('$destroyed', function() {
1380-
for (var i=0, ii = onDestroyed.length; i < ii; ++i) {
1381-
onDestroyed[i]();
1382-
}
1383-
onDestroyed = null;
1384-
});
1376+
var destroyBindings = nodeLinkFn.$$destroyBindings;
1377+
if (destroyBindings) {
1378+
nodeLinkFn.$$destroyBindings = null;
1379+
childScope.$on('$destroyed', destroyBindings);
13851380
}
13861381
} else {
13871382
childScope = scope;
@@ -1955,18 +1950,29 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
19551950
if (controllers) {
19561951
// Initialize bindToController bindings for new/isolate scopes
19571952
var scopeDirective = newIsolateScopeDirective || newScopeDirective;
1953+
var bindings;
1954+
var controllerForBindings;
19581955
if (scopeDirective && controllers[scopeDirective.name]) {
1959-
var bindings = scopeDirective.$$bindings.bindToController;
1956+
bindings = scopeDirective.$$bindings.bindToController;
19601957
controller = controllers[scopeDirective.name];
19611958

19621959
if (controller && controller.identifier && bindings) {
1963-
thisLinkFn.$$onScopeDestroyed =
1960+
controllerForBindings = controller;
1961+
thisLinkFn.$$destroyBindings =
19641962
initializeDirectiveBindings(scope, attrs, controller.instance,
19651963
bindings, scopeDirective);
19661964
}
19671965
}
19681966
forEach(controllers, function(controller) {
1969-
controller();
1967+
var result = controller();
1968+
if (result !== controller.instance &&
1969+
controller === controllerForBindings) {
1970+
// Remove and re-install bindToController bindings
1971+
thisLinkFn.$$destroyBindings();
1972+
thisLinkFn.$$destroyBindings =
1973+
initializeDirectiveBindings(scope, attrs, result,
1974+
bindings, scopeDirective);
1975+
}
19701976
});
19711977
controllers = null;
19721978
}
@@ -2558,12 +2564,8 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
25582564
} else {
25592565
unwatch = scope.$watch($parse(attrs[attrName], parentValueWatch), null, parentGet.literal);
25602566
}
2561-
if (newScope) {
2562-
newScope.$on('$destroy', unwatch);
2563-
} else {
2564-
onNewScopeDestroyed = (onNewScopeDestroyed || []);
2565-
onNewScopeDestroyed.push(unwatch);
2566-
}
2567+
onNewScopeDestroyed = (onNewScopeDestroyed || []);
2568+
onNewScopeDestroyed.push(unwatch);
25672569
break;
25682570

25692571
case '&':
@@ -2574,7 +2576,16 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
25742576
break;
25752577
}
25762578
});
2577-
return onNewScopeDestroyed;
2579+
var destroyBindings = onNewScopeDestroyed ? function destroyBindings() {
2580+
for (var i = 0, ii = onNewScopeDestroyed.length; i < ii; ++i) {
2581+
onNewScopeDestroyed[i]();
2582+
}
2583+
} : noop;
2584+
if (newScope && destroyBindings !== noop) {
2585+
newScope.$on('$destroy', destroyBindings);
2586+
return noop;
2587+
}
2588+
return destroyBindings;
25782589
}
25792590
}];
25802591
}

‎src/ng/controller.js

+10-2
Original file line numberDiff line numberDiff line change
@@ -133,8 +133,16 @@ function $ControllerProvider() {
133133
addIdentifier(locals, identifier, instance, constructor || expression.name);
134134
}
135135

136-
return extend(function() {
137-
$injector.invoke(expression, instance, locals, constructor);
136+
var instantiate;
137+
return instantiate = extend(function() {
138+
var result = $injector.invoke(expression, instance, locals, constructor);
139+
if (result !== instance && (isObject(result) || isFunction(result))) {
140+
instance = result;
141+
if (identifier) {
142+
// If result changed, re-assign controllerAs value to scope.
143+
addIdentifier(locals, identifier, instance, constructor || expression.name);
144+
}
145+
}
138146
return instance;
139147
}, {
140148
instance: instance,

‎test/ng/compileSpec.js

+112-4
Original file line numberDiff line numberDiff line change
@@ -3910,8 +3910,8 @@ describe('$compile', function() {
39103910
'baz': 'biz'
39113911
};
39123912
element = $compile('<div foo-dir dir-data="remoteData" ' +
3913-
'dir-str="Hello, {{whom}}!" ' +
3914-
'dir-fn="fn()"></div>')($rootScope);
3913+
'dir-str="Hello, {{whom}}!" ' +
3914+
'dir-fn="fn()"></div>')($rootScope);
39153915
$rootScope.$digest();
39163916
expect(controllerCalled).toBe(true);
39173917
});
@@ -3950,8 +3950,8 @@ describe('$compile', function() {
39503950
'baz': 'biz'
39513951
};
39523952
element = $compile('<div foo-dir dir-data="remoteData" ' +
3953-
'dir-str="Hello, {{whom}}!" ' +
3954-
'dir-fn="fn()"></div>')($rootScope);
3953+
'dir-str="Hello, {{whom}}!" ' +
3954+
'dir-fn="fn()"></div>')($rootScope);
39553955
$rootScope.$digest();
39563956
expect(controllerCalled).toBe(true);
39573957
});
@@ -3983,6 +3983,114 @@ describe('$compile', function() {
39833983
expect(childScope.theCtrl).toBe(myCtrl);
39843984
});
39853985
});
3986+
3987+
3988+
it('should re-install controllerAs and bindings for returned value from controller (new scope)', function() {
3989+
var controllerCalled = false;
3990+
var myCtrl;
3991+
3992+
function MyCtrl() {
3993+
}
3994+
MyCtrl.prototype.test = function() {
3995+
expect(this.data).toEqualData({
3996+
'foo': 'bar',
3997+
'baz': 'biz'
3998+
});
3999+
expect(this.str).toBe('Hello, world!');
4000+
expect(this.fn()).toBe('called!');
4001+
}
4002+
4003+
module(function($compileProvider, $controllerProvider) {
4004+
$controllerProvider.register('myCtrl', function() {
4005+
controllerCalled = true;
4006+
myCtrl = this;
4007+
return new MyCtrl();
4008+
});
4009+
$compileProvider.directive('fooDir', valueFn({
4010+
templateUrl: 'test.html',
4011+
bindToController: {
4012+
'data': '=dirData',
4013+
'str': '@dirStr',
4014+
'fn': '&dirFn'
4015+
},
4016+
scope: true,
4017+
controller: 'myCtrl as theCtrl'
4018+
}));
4019+
});
4020+
inject(function($compile, $rootScope, $templateCache) {
4021+
$templateCache.put('test.html', '<p>isolate</p>');
4022+
$rootScope.fn = valueFn('called!');
4023+
$rootScope.whom = 'world';
4024+
$rootScope.remoteData = {
4025+
'foo': 'bar',
4026+
'baz': 'biz'
4027+
};
4028+
element = $compile('<div foo-dir dir-data="remoteData" ' +
4029+
'dir-str="Hello, {{whom}}!" ' +
4030+
'dir-fn="fn()"></div>')($rootScope);
4031+
$rootScope.$digest();
4032+
expect(controllerCalled).toBe(true);
4033+
var childScope = element.children().scope();
4034+
expect(childScope).not.toBe($rootScope);
4035+
expect(childScope.theCtrl).not.toBe(myCtrl);
4036+
expect(childScope.theCtrl.constructor).toBe(MyCtrl);
4037+
childScope.theCtrl.test();
4038+
});
4039+
});
4040+
4041+
4042+
it('should re-install controllerAs and bindings for returned value from controller (isolate scope)', function() {
4043+
var controllerCalled = false;
4044+
var myCtrl;
4045+
4046+
function MyCtrl() {
4047+
}
4048+
MyCtrl.prototype.test = function() {
4049+
expect(this.data).toEqualData({
4050+
'foo': 'bar',
4051+
'baz': 'biz'
4052+
});
4053+
expect(this.str).toBe('Hello, world!');
4054+
expect(this.fn()).toBe('called!');
4055+
}
4056+
4057+
module(function($compileProvider, $controllerProvider) {
4058+
$controllerProvider.register('myCtrl', function() {
4059+
controllerCalled = true;
4060+
myCtrl = this;
4061+
return new MyCtrl();
4062+
});
4063+
$compileProvider.directive('fooDir', valueFn({
4064+
templateUrl: 'test.html',
4065+
bindToController: true,
4066+
scope: {
4067+
'data': '=dirData',
4068+
'str': '@dirStr',
4069+
'fn': '&dirFn'
4070+
},
4071+
controller: 'myCtrl as theCtrl'
4072+
}));
4073+
});
4074+
inject(function($compile, $rootScope, $templateCache) {
4075+
$templateCache.put('test.html', '<p>isolate</p>');
4076+
$rootScope.fn = valueFn('called!');
4077+
$rootScope.whom = 'world';
4078+
$rootScope.remoteData = {
4079+
'foo': 'bar',
4080+
'baz': 'biz'
4081+
};
4082+
element = $compile('<div foo-dir dir-data="remoteData" ' +
4083+
'dir-str="Hello, {{whom}}!" ' +
4084+
'dir-fn="fn()"></div>')($rootScope);
4085+
$rootScope.$digest();
4086+
expect(controllerCalled).toBe(true);
4087+
var childScope = element.children().scope();
4088+
expect(childScope).not.toBe($rootScope);
4089+
expect(childScope.theCtrl).not.toBe(myCtrl);
4090+
expect(childScope.theCtrl.constructor).toBe(MyCtrl);
4091+
childScope.theCtrl.test();
4092+
});
4093+
});
39864094
});
39874095

39884096

0 commit comments

Comments
 (0)