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

Commit

Permalink
fix($compile): only pass isolate scope to children that belong to the…
Browse files Browse the repository at this point in the history
… isolate directive

I had to fix one unit test, as it assumed the broken behavior, where application template gets the
isolate scope of other (isolate) directive, rather than the regular scope.

BREAKING CHANGE: Child elements that are defined either in the application template or in some other
directives template do not get the isolate scope. In theory, nobody should rely on this behavior, as
it is very rare - in most cases the isolate directive has a template.
  • Loading branch information
vojtajina authored and IgorMinar committed Nov 8, 2013
1 parent 909cabd commit d0efd5e
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 4 deletions.
9 changes: 7 additions & 2 deletions src/ng/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -1480,8 +1480,13 @@ function $CompileProvider($provide) {
}

// RECURSION
// TODO(vojta): only pass isolate if the isolate directive has template
childLinkFn && childLinkFn(isolateScope || scope, linkNode.childNodes, undefined, boundTranscludeFn);
// We only pass the isolate scope, if the isolate directive has a template,
// otherwise the child elements do not belong to the isolate directive.
var scopeToChild = scope;
if (newIsolateScopeDirective && (newIsolateScopeDirective.template || newIsolateScopeDirective.templateUrl === null)) {
scopeToChild = isolateScope;
}
childLinkFn && childLinkFn(scopeToChild, linkNode.childNodes, undefined, boundTranscludeFn);

// POSTLINKING
for(i = postLinkFns.length - 1; i >= 0; i--) {
Expand Down
60 changes: 58 additions & 2 deletions test/ng/compileSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1426,7 +1426,7 @@ describe('$compile', function() {
return {
restrict: 'CA',
link: {pre: function(scope) {
log('log-' + scope.$id + '-' + scope.$parent.$id);
log('log-' + scope.$id + '-' + (scope.$parent && scope.$parent.$id || 'no-parent'));
}}
};
});
Expand All @@ -1443,7 +1443,7 @@ describe('$compile', function() {
it('should allow creation of new isolated scopes for directives', inject(
function($rootScope, $compile, log) {
element = $compile('<div><span iscope><a log></a></span></div>')($rootScope);
expect(log).toEqual('log-002-001; LOG; 002');
expect(log).toEqual('log-001-no-parent; LOG; 002');
$rootScope.name = 'abc';
expect(iscope.$parent).toBe($rootScope);
expect(iscope.name).toBeUndefined();
Expand Down Expand Up @@ -2131,6 +2131,62 @@ describe('$compile', function() {
expect(componentScope.$parent).toBe(regularScope)
}));

it('should not give the isolate scope to other directive template', function() {
module(function() {
directive('otherTplDir', function() {
return {
template: 'value: {{value}}'
};
});
});

inject(function($rootScope) {
compile('<div my-component other-tpl-dir>');

$rootScope.$apply(function() {
$rootScope.value = 'from-parent';
});

expect(element.html()).toBe('value: from-parent');
});
});


it('should not give the isolate scope to other directive template (with templateUrl)', function() {
module(function() {
directive('otherTplDir', function() {
return {
templateUrl: 'other.html'
};
});
});

inject(function($rootScope, $templateCache) {
$templateCache.put('other.html', 'value: {{value}}')
compile('<div my-component other-tpl-dir>');

$rootScope.$apply(function() {
$rootScope.value = 'from-parent';
});

expect(element.html()).toBe('value: from-parent');
});
});


it('should not give the isolate scope to regular child elements', function() {
inject(function($rootScope) {
compile('<div my-component>value: {{value}}</div>');

$rootScope.$apply(function() {
$rootScope.value = 'from-parent';
});

expect(element.html()).toBe('value: from-parent');
});
});


describe('attribute', function() {
it('should copy simple attribute', inject(function() {
compile('<div><span my-component attr="some text">');
Expand Down

5 comments on commit d0efd5e

@acheshkov
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there an option to prevent this behavior?

@acheshkov
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if i setup templateUrl to null should it help?

@euge
Copy link

@euge euge commented on d0efd5e Dec 3, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@euge
Copy link

@euge euge commented on d0efd5e Dec 3, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like you should be able to receive the isolate scope if you are constructing the template programatically?

@btm1
Copy link

@btm1 btm1 commented on d0efd5e Apr 2, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this is FAIL and messed up my app badly now i can't do something like this:

lrApp.directive('foo', function () {
return {
restrict: 'E',
scope : {
yup : '@check'
},
link: function (scope, iElement, iAttrs) {
scope.blah = 'yes ye syes';
}
};
});

{{blah}} {{yup}}

and i should be able to ....

Please sign in to comment.