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

Commit ab7a400

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 7e77e03 commit ab7a400

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;
@@ -1951,18 +1946,29 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
19511946
if (controllers) {
19521947
// Initialize bindToController bindings for new/isolate scopes
19531948
var scopeDirective = newIsolateScopeDirective || newScopeDirective;
1949+
var bindings;
1950+
var controllerForBindings;
19541951
if (scopeDirective && controllers[scopeDirective.name]) {
1955-
var bindings = scopeDirective.$$bindings.bindToController;
1952+
bindings = scopeDirective.$$bindings.bindToController;
19561953
controller = controllers[scopeDirective.name];
19571954

19581955
if (controller && controller.identifier && bindings) {
1959-
thisLinkFn.$$onScopeDestroyed =
1956+
controllerForBindings = controller;
1957+
thisLinkFn.$$destroyBindings =
19601958
initializeDirectiveBindings(scope, attrs, controller.instance,
19611959
bindings, scopeDirective);
19621960
}
19631961
}
19641962
forEach(controllers, function(controller) {
1965-
controller();
1963+
var result = controller();
1964+
if (result !== controller.instance &&
1965+
controller === controllerForBindings) {
1966+
// Remove and re-install bindToController bindings
1967+
thisLinkFn.$$destroyBindings();
1968+
thisLinkFn.$$destroyBindings =
1969+
initializeDirectiveBindings(scope, attrs, result,
1970+
bindings, scopeDirective);
1971+
}
19661972
});
19671973
controllers = null;
19681974
}
@@ -2554,12 +2560,8 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
25542560
} else {
25552561
unwatch = scope.$watch($parse(attrs[attrName], parentValueWatch), null, parentGet.literal);
25562562
}
2557-
if (newScope) {
2558-
newScope.$on('$destroy', unwatch);
2559-
} else {
2560-
onNewScopeDestroyed = (onNewScopeDestroyed || []);
2561-
onNewScopeDestroyed.push(unwatch);
2562-
}
2563+
onNewScopeDestroyed = (onNewScopeDestroyed || []);
2564+
onNewScopeDestroyed.push(unwatch);
25632565
break;
25642566

25652567
case '&':
@@ -2570,7 +2572,16 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
25702572
break;
25712573
}
25722574
});
2573-
return onNewScopeDestroyed;
2575+
var destroyBindings = onNewScopeDestroyed ? function destroyBindings() {
2576+
for (var i = 0, ii = onNewScopeDestroyed.length; i < ii; ++i) {
2577+
onNewScopeDestroyed[i]();
2578+
}
2579+
} : noop;
2580+
if (newScope && destroyBindings !== noop) {
2581+
newScope.$on('$destroy', destroyBindings);
2582+
return noop;
2583+
}
2584+
return destroyBindings;
25742585
}
25752586
}];
25762587
}

‎src/ng/controller.js

+10-2
Original file line numberDiff line numberDiff line change
@@ -125,8 +125,16 @@ function $ControllerProvider() {
125125
addIdentifier(locals, identifier, instance, constructor || expression.name);
126126
}
127127

128-
return extend(function() {
129-
$injector.invoke(expression, instance, locals, constructor);
128+
var instantiate;
129+
return instantiate = extend(function() {
130+
var result = $injector.invoke(expression, instance, locals, constructor);
131+
if (result !== instance && (isObject(result) || isFunction(result))) {
132+
instance = result;
133+
if (identifier) {
134+
// If result changed, re-assign controllerAs value to scope.
135+
addIdentifier(locals, identifier, instance, constructor || expression.name);
136+
}
137+
}
130138
return instance;
131139
}, {
132140
instance: instance,

‎test/ng/compileSpec.js

+112-4
Original file line numberDiff line numberDiff line change
@@ -3899,8 +3899,8 @@ describe('$compile', function() {
38993899
'baz': 'biz'
39003900
};
39013901
element = $compile('<div foo-dir dir-data="remoteData" ' +
3902-
'dir-str="Hello, {{whom}}!" ' +
3903-
'dir-fn="fn()"></div>')($rootScope);
3902+
'dir-str="Hello, {{whom}}!" ' +
3903+
'dir-fn="fn()"></div>')($rootScope);
39043904
$rootScope.$digest();
39053905
expect(controllerCalled).toBe(true);
39063906
});
@@ -3939,8 +3939,8 @@ describe('$compile', function() {
39393939
'baz': 'biz'
39403940
};
39413941
element = $compile('<div foo-dir dir-data="remoteData" ' +
3942-
'dir-str="Hello, {{whom}}!" ' +
3943-
'dir-fn="fn()"></div>')($rootScope);
3942+
'dir-str="Hello, {{whom}}!" ' +
3943+
'dir-fn="fn()"></div>')($rootScope);
39443944
$rootScope.$digest();
39453945
expect(controllerCalled).toBe(true);
39463946
});
@@ -3972,6 +3972,114 @@ describe('$compile', function() {
39723972
expect(childScope.theCtrl).toBe(myCtrl);
39733973
});
39743974
});
3975+
3976+
3977+
it('should re-install controllerAs and bindings for returned value from controller (new scope)', function() {
3978+
var controllerCalled = false;
3979+
var myCtrl;
3980+
3981+
function MyCtrl() {
3982+
}
3983+
MyCtrl.prototype.test = function() {
3984+
expect(this.data).toEqualData({
3985+
'foo': 'bar',
3986+
'baz': 'biz'
3987+
});
3988+
expect(this.str).toBe('Hello, world!');
3989+
expect(this.fn()).toBe('called!');
3990+
}
3991+
3992+
module(function($compileProvider, $controllerProvider) {
3993+
$controllerProvider.register('myCtrl', function() {
3994+
controllerCalled = true;
3995+
myCtrl = this;
3996+
return new MyCtrl();
3997+
});
3998+
$compileProvider.directive('fooDir', valueFn({
3999+
templateUrl: 'test.html',
4000+
bindToController: {
4001+
'data': '=dirData',
4002+
'str': '@dirStr',
4003+
'fn': '&dirFn'
4004+
},
4005+
scope: true,
4006+
controller: 'myCtrl as theCtrl'
4007+
}));
4008+
});
4009+
inject(function($compile, $rootScope, $templateCache) {
4010+
$templateCache.put('test.html', '<p>isolate</p>');
4011+
$rootScope.fn = valueFn('called!');
4012+
$rootScope.whom = 'world';
4013+
$rootScope.remoteData = {
4014+
'foo': 'bar',
4015+
'baz': 'biz'
4016+
};
4017+
element = $compile('<div foo-dir dir-data="remoteData" ' +
4018+
'dir-str="Hello, {{whom}}!" ' +
4019+
'dir-fn="fn()"></div>')($rootScope);
4020+
$rootScope.$digest();
4021+
expect(controllerCalled).toBe(true);
4022+
var childScope = element.children().scope();
4023+
expect(childScope).not.toBe($rootScope);
4024+
expect(childScope.theCtrl).not.toBe(myCtrl);
4025+
expect(childScope.theCtrl.constructor).toBe(MyCtrl);
4026+
childScope.theCtrl.test();
4027+
});
4028+
});
4029+
4030+
4031+
it('should re-install controllerAs and bindings for returned value from controller (isolate scope)', function() {
4032+
var controllerCalled = false;
4033+
var myCtrl;
4034+
4035+
function MyCtrl() {
4036+
}
4037+
MyCtrl.prototype.test = function() {
4038+
expect(this.data).toEqualData({
4039+
'foo': 'bar',
4040+
'baz': 'biz'
4041+
});
4042+
expect(this.str).toBe('Hello, world!');
4043+
expect(this.fn()).toBe('called!');
4044+
}
4045+
4046+
module(function($compileProvider, $controllerProvider) {
4047+
$controllerProvider.register('myCtrl', function() {
4048+
controllerCalled = true;
4049+
myCtrl = this;
4050+
return new MyCtrl();
4051+
});
4052+
$compileProvider.directive('fooDir', valueFn({
4053+
templateUrl: 'test.html',
4054+
bindToController: true,
4055+
scope: {
4056+
'data': '=dirData',
4057+
'str': '@dirStr',
4058+
'fn': '&dirFn'
4059+
},
4060+
controller: 'myCtrl as theCtrl'
4061+
}));
4062+
});
4063+
inject(function($compile, $rootScope, $templateCache) {
4064+
$templateCache.put('test.html', '<p>isolate</p>');
4065+
$rootScope.fn = valueFn('called!');
4066+
$rootScope.whom = 'world';
4067+
$rootScope.remoteData = {
4068+
'foo': 'bar',
4069+
'baz': 'biz'
4070+
};
4071+
element = $compile('<div foo-dir dir-data="remoteData" ' +
4072+
'dir-str="Hello, {{whom}}!" ' +
4073+
'dir-fn="fn()"></div>')($rootScope);
4074+
$rootScope.$digest();
4075+
expect(controllerCalled).toBe(true);
4076+
var childScope = element.children().scope();
4077+
expect(childScope).not.toBe($rootScope);
4078+
expect(childScope.theCtrl).not.toBe(myCtrl);
4079+
expect(childScope.theCtrl.constructor).toBe(MyCtrl);
4080+
childScope.theCtrl.test();
4081+
});
4082+
});
39754083
});
39764084

39774085

0 commit comments

Comments
 (0)