Skip to content

Commit 1011393

Browse files
committed
fix($compile): do not rebind parent bound transclude functions
Do not rebind parent bound transclude functions passed to the link function returned from `$compile`. Change parameter order of ```publicLinkFn``` to allow parent bound transclude functions to be passed (do not make public yet). The current `$compile` public API documentation indicates that a transclude function may be passed as a second parameter, but it is not clear that this is not the same function that is given to directive link functions as the `transcludeFn` parameter. This parameter must be passed in order to implement, for example, lazy compilation. In order to pass the `transcludeFn` parameter given to a directive's link function, it must be passed to the result of a call to `$compile` (i.e. `publicLinkFn`). Doing so, however, would cause two bugs. First, the passed transclude function would get rebound, improperly changing its scope from its original binding. Second, the `containingScope` would not be updated and the wrong `$parent` would be assigned to the `transcludedScope` resulting in a memory leak. This patch fixes both of these issues. This patch does not expose a new public API for `publicLinkFn`, rather, it leaves this to a later change that better considers how that public API should look. Thanks to @lgalfaso, @tbosch, and @petebacondarwin. Related to angular#9413
1 parent 7f4d24c commit 1011393

File tree

2 files changed

+130
-4
lines changed

2 files changed

+130
-4
lines changed

src/ng/compile.js

+14-4
Original file line numberDiff line numberDiff line change
@@ -1153,7 +1153,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
11531153
maxPriority, ignoreDirective, previousCompileContext);
11541154
compile.$$addScopeClass($compileNodes);
11551155
var namespace = null;
1156-
return function publicLinkFn(scope, cloneConnectFn, transcludeControllers, parentBoundTranscludeFn, futureParentElement){
1156+
return function publicLinkFn(scope, cloneConnectFn, parentBoundTranscludeFn, transcludeControllers, futureParentElement){
11571157
assertArg(scope, 'scope');
11581158
if (!namespace) {
11591159
namespace = detectNamespaceForChildElements(futureParentElement);
@@ -1324,7 +1324,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
13241324
transcludedScope.$$transcluded = true;
13251325
}
13261326

1327-
return transcludeFn(transcludedScope, cloneFn, controllers, previousBoundTranscludeFn, futureParentElement);
1327+
return transcludeFn(transcludedScope, cloneFn, previousBoundTranscludeFn, controllers, futureParentElement);
13281328
};
13291329

13301330
return boundTranscludeFn;
@@ -1793,7 +1793,11 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
17931793
isolateScope = scope.$new(true);
17941794
}
17951795

1796-
transcludeFn = boundTranscludeFn && controllersBoundTransclude;
1796+
if (boundTranscludeFn) {
1797+
transcludeFn = controllersBoundTransclude;
1798+
controllersBoundTransclude.$$boundTransclude = boundTranscludeFn;
1799+
}
1800+
17971801
if (controllerDirectives) {
17981802
// TODO: merge `controllers` and `elementControllers` into single object.
17991803
controllers = {};
@@ -1965,7 +1969,13 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
19651969
if (!futureParentElement) {
19661970
futureParentElement = hasElementTranscludeDirective ? $element.parent() : $element;
19671971
}
1968-
return boundTranscludeFn(scope, cloneAttachFn, transcludeControllers, futureParentElement, scopeToChild);
1972+
1973+
// When a transcludeFn is passed to publicLinkFn, it is already
1974+
// a `controllersBoundTransclude` function, so prevent double-wrapping
1975+
// by using the previously-wrapped boundTranscludeFn.
1976+
var unwrappedFn = boundTranscludeFn.$$boundTransclude || boundTranscludeFn;
1977+
controllersBoundTransclude.$$boundTransclude = unwrappedFn;
1978+
return unwrappedFn(scope, cloneAttachFn, transcludeControllers, futureParentElement, scopeToChild);
19691979
}
19701980
}
19711981
}

test/ng/compileSpec.js

+116
Original file line numberDiff line numberDiff line change
@@ -5124,6 +5124,122 @@ describe('$compile', function() {
51245124
});
51255125

51265126

5127+
// see issue https://github.com/angular/angular.js/issues/9413
5128+
describe('passing a parent bound transclude function to the link ' +
5129+
'function returned from `$compile`', function() {
5130+
5131+
function countScopes($rootScope) {
5132+
return [$rootScope].concat(
5133+
getChildScopes($rootScope)
5134+
).length;
5135+
}
5136+
5137+
function getChildScopes(scope) {
5138+
var children = [];
5139+
if (!scope.$$childHead) { return children; }
5140+
var childScope = scope.$$childHead;
5141+
do {
5142+
children.push(childScope);
5143+
children = children.concat(getChildScopes(childScope));
5144+
} while ((childScope = childScope.$$nextSibling));
5145+
return children;
5146+
}
5147+
5148+
beforeEach(module(function() {
5149+
directive('lazyCompile', function($compile) {
5150+
return {
5151+
terminal: true,
5152+
priority: 1,
5153+
compile: function(tElement, tAttrs) {
5154+
var content = tElement.contents();
5155+
tElement.empty();
5156+
return function(scope, element, attrs, ctrls, transcludeFn) {
5157+
element.append(content);
5158+
$compile(content)(scope, undefined, transcludeFn);
5159+
};
5160+
}
5161+
};
5162+
});
5163+
directive('toggle', function() {
5164+
return {
5165+
scope: {t: '=toggle'},
5166+
transclude: true,
5167+
template: '<div ng-if="t"><lazy-compile><div ng-transclude></div></lazy-compile></div>'
5168+
};
5169+
});
5170+
}));
5171+
5172+
it('should preserve the bound scope', function() {
5173+
5174+
inject(function($compile, $rootScope) {
5175+
element = $compile(
5176+
'<div>' +
5177+
'<div ng-init="outer=true"></div>' +
5178+
'<div toggle="t">' +
5179+
'<span ng-if="outer">Success</span><span ng-if="!outer">Error</span>' +
5180+
'</div>' +
5181+
'</div>')($rootScope);
5182+
5183+
$rootScope.$apply('t = false');
5184+
expect(countScopes($rootScope)).toBe(2);
5185+
expect(element.text()).toBe('');
5186+
5187+
$rootScope.$apply('t = true');
5188+
expect(countScopes($rootScope)).toBe(5);
5189+
expect(element.text()).toBe('Success');
5190+
5191+
$rootScope.$apply('t = false');
5192+
expect(countScopes($rootScope)).toBe(2);
5193+
expect(element.text()).toBe('');
5194+
5195+
$rootScope.$apply('t = true');
5196+
expect(countScopes($rootScope)).toBe(5);
5197+
expect(element.text()).toBe('Success');
5198+
});
5199+
});
5200+
5201+
5202+
it('should preserve the bound scope when using recursive transclusion', function() {
5203+
5204+
directive('recursiveTransclude', function() {
5205+
return {
5206+
scope: {},
5207+
transclude: true,
5208+
template: '<div><lazy-compile><div ng-transclude></div></lazy-compile></div>'
5209+
};
5210+
});
5211+
5212+
inject(function($compile, $rootScope) {
5213+
element = $compile(
5214+
'<div>' +
5215+
'<div ng-init="outer=true"></div>' +
5216+
'<div toggle="t">' +
5217+
'<div recursive-transclude>' +
5218+
'<span ng-if="outer">Success</span><span ng-if="!outer">Error</span>' +
5219+
'</div>' +
5220+
'</div>' +
5221+
'</div>')($rootScope);
5222+
5223+
$rootScope.$apply('t = false');
5224+
expect(countScopes($rootScope)).toBe(2);
5225+
expect(element.text()).toBe('');
5226+
5227+
$rootScope.$apply('t = true');
5228+
expect(countScopes($rootScope)).toBe(5);
5229+
expect(element.text()).toBe('Success');
5230+
5231+
$rootScope.$apply('t = false');
5232+
expect(countScopes($rootScope)).toBe(2);
5233+
expect(element.text()).toBe('');
5234+
5235+
$rootScope.$apply('t = true');
5236+
expect(countScopes($rootScope)).toBe(5);
5237+
expect(element.text()).toBe('Success');
5238+
});
5239+
});
5240+
});
5241+
5242+
51275243
// see issue https://github.com/angular/angular.js/issues/9095
51285244
describe('removing a transcluded element', function() {
51295245

0 commit comments

Comments
 (0)