Skip to content

Commit 50557a6

Browse files
gkalpakpetebacondarwin
gkalpak
authored and
petebacondarwin
committed
fix($compile): evaluate against the correct scope with bindToController on new scope
Previously, the directive bindings were evaluated against the directive's new (non-isolate) scope, instead of the correct (parent) scope. This went unnoticed most of the time, since a property would be eventually looked up in the parent scope due to prototypal inheritance. The incorrect behaviour was exhibited when a property on the child scope was shadowing that on the parent scope. This commit fixes it. Fixes angular#13021 Closes angular#13025
1 parent 1c13a4f commit 50557a6

2 files changed

Lines changed: 124 additions & 3 deletions

File tree

‎src/ng/compile.js‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2084,7 +2084,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
20842084
}
20852085

20862086
function nodeLinkFn(childLinkFn, scope, linkNode, $rootElement, boundTranscludeFn) {
2087-
var linkFn, isolateScope, elementControllers, transcludeFn, $element,
2087+
var linkFn, isolateScope, controllerScope, elementControllers, transcludeFn, $element,
20882088
attrs, removeScopeBindingWatches, removeControllerBindingWatches;
20892089

20902090
if (compileNode === linkNode) {
@@ -2095,8 +2095,11 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
20952095
attrs = new Attributes($element, templateAttrs);
20962096
}
20972097

2098+
controllerScope = scope;
20982099
if (newIsolateScopeDirective) {
20992100
isolateScope = scope.$new(true);
2101+
} else if (newScopeDirective) {
2102+
controllerScope = scope.$parent;
21002103
}
21012104

21022105
if (boundTranscludeFn) {
@@ -2133,7 +2136,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
21332136

21342137
if (controller.identifier && bindings) {
21352138
removeControllerBindingWatches =
2136-
initializeDirectiveBindings(scope, attrs, controller.instance, bindings, controllerDirective);
2139+
initializeDirectiveBindings(controllerScope, attrs, controller.instance, bindings, controllerDirective);
21372140
}
21382141

21392142
var controllerResult = controller();
@@ -2144,7 +2147,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
21442147
$element.data('$' + controllerDirective.name + 'Controller', controllerResult);
21452148
removeControllerBindingWatches && removeControllerBindingWatches();
21462149
removeControllerBindingWatches =
2147-
initializeDirectiveBindings(scope, attrs, controller.instance, bindings, controllerDirective);
2150+
initializeDirectiveBindings(controllerScope, attrs, controller.instance, bindings, controllerDirective);
21482151
}
21492152
}
21502153

‎test/ng/compileSpec.js‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4591,6 +4591,124 @@ describe('$compile', function() {
45914591
});
45924592

45934593

4594+
it('should evaluate against the correct scope, when using `bindToController` (new scope)',
4595+
function() {
4596+
module(function($compileProvider, $controllerProvider) {
4597+
$controllerProvider.register({
4598+
'ParentCtrl': function() {
4599+
this.value1 = 'parent1';
4600+
this.value2 = 'parent2';
4601+
this.value3 = function() { return 'parent3'; };
4602+
},
4603+
'ChildCtrl': function() {
4604+
this.value1 = 'child1';
4605+
this.value2 = 'child2';
4606+
this.value3 = function() { return 'child3'; };
4607+
}
4608+
});
4609+
4610+
$compileProvider.directive('child', valueFn({
4611+
scope: true,
4612+
controller: 'ChildCtrl as ctrl',
4613+
bindToController: {
4614+
fromParent1: '@',
4615+
fromParent2: '=',
4616+
fromParent3: '&'
4617+
},
4618+
template: ''
4619+
}));
4620+
});
4621+
4622+
inject(function($compile, $rootScope) {
4623+
element = $compile(
4624+
'<div ng-controller="ParentCtrl as ctrl">' +
4625+
'<child ' +
4626+
'from-parent-1="{{ ctrl.value1 }}" ' +
4627+
'from-parent-2="ctrl.value2" ' +
4628+
'from-parent-3="ctrl.value3">' +
4629+
'</child>' +
4630+
'</div>')($rootScope);
4631+
$rootScope.$digest();
4632+
4633+
var parentCtrl = element.controller('ngController');
4634+
var childCtrl = element.find('child').controller('child');
4635+
4636+
expect(childCtrl.fromParent1).toBe(parentCtrl.value1);
4637+
expect(childCtrl.fromParent1).not.toBe(childCtrl.value1);
4638+
expect(childCtrl.fromParent2).toBe(parentCtrl.value2);
4639+
expect(childCtrl.fromParent2).not.toBe(childCtrl.value2);
4640+
expect(childCtrl.fromParent3()()).toBe(parentCtrl.value3());
4641+
expect(childCtrl.fromParent3()()).not.toBe(childCtrl.value3());
4642+
4643+
childCtrl.fromParent2 = 'modified';
4644+
$rootScope.$digest();
4645+
4646+
expect(parentCtrl.value2).toBe('modified');
4647+
expect(childCtrl.value2).toBe('child2');
4648+
});
4649+
}
4650+
);
4651+
4652+
4653+
it('should evaluate against the correct scope, when using `bindToController` (new iso scope)',
4654+
function() {
4655+
module(function($compileProvider, $controllerProvider) {
4656+
$controllerProvider.register({
4657+
'ParentCtrl': function() {
4658+
this.value1 = 'parent1';
4659+
this.value2 = 'parent2';
4660+
this.value3 = function() { return 'parent3'; };
4661+
},
4662+
'ChildCtrl': function() {
4663+
this.value1 = 'child1';
4664+
this.value2 = 'child2';
4665+
this.value3 = function() { return 'child3'; };
4666+
}
4667+
});
4668+
4669+
$compileProvider.directive('child', valueFn({
4670+
scope: {},
4671+
controller: 'ChildCtrl as ctrl',
4672+
bindToController: {
4673+
fromParent1: '@',
4674+
fromParent2: '=',
4675+
fromParent3: '&'
4676+
},
4677+
template: ''
4678+
}));
4679+
});
4680+
4681+
inject(function($compile, $rootScope) {
4682+
element = $compile(
4683+
'<div ng-controller="ParentCtrl as ctrl">' +
4684+
'<child ' +
4685+
'from-parent-1="{{ ctrl.value1 }}" ' +
4686+
'from-parent-2="ctrl.value2" ' +
4687+
'from-parent-3="ctrl.value3">' +
4688+
'</child>' +
4689+
'</div>')($rootScope);
4690+
$rootScope.$digest();
4691+
4692+
var parentCtrl = element.controller('ngController');
4693+
var childCtrl = element.find('child').controller('child');
4694+
4695+
expect(childCtrl.fromParent1).toBe(parentCtrl.value1);
4696+
expect(childCtrl.fromParent1).not.toBe(childCtrl.value1);
4697+
expect(childCtrl.fromParent2).toBe(parentCtrl.value2);
4698+
expect(childCtrl.fromParent2).not.toBe(childCtrl.value2);
4699+
expect(childCtrl.fromParent3()()).toBe(parentCtrl.value3());
4700+
expect(childCtrl.fromParent3()()).not.toBe(childCtrl.value3());
4701+
4702+
childCtrl.fromParent2 = 'modified';
4703+
$rootScope.$digest();
4704+
4705+
expect(parentCtrl.value2).toBe('modified');
4706+
expect(childCtrl.value2).toBe('child2');
4707+
});
4708+
}
4709+
);
4710+
4711+
45944712
it('should put controller in scope when controller identifier present but not using controllerAs', function() {
45954713
var controllerCalled = false;
45964714
var myCtrl;

0 commit comments

Comments
 (0)