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

Commit 72bbc33

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 #9413
1 parent 875f8f6 commit 72bbc33

File tree

2 files changed

+130
-2
lines changed

2 files changed

+130
-2
lines changed

src/ng/compile.js

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1157,6 +1157,15 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
11571157
var namespace = null;
11581158
return function publicLinkFn(scope, cloneConnectFn, transcludeControllers, parentBoundTranscludeFn, futureParentElement) {
11591159
assertArg(scope, 'scope');
1160+
1161+
// When `parentBoundTranscludeFn` is passed, it is a
1162+
// `controllersBoundTransclude` function (it was previously passed
1163+
// as `transclude` to directive.link) so we must unwrap it to get
1164+
// its `boundTranscludeFn`
1165+
if (parentBoundTranscludeFn && parentBoundTranscludeFn.$$boundTransclude) {
1166+
parentBoundTranscludeFn = parentBoundTranscludeFn.$$boundTransclude;
1167+
}
1168+
11601169
if (!namespace) {
11611170
namespace = detectNamespaceForChildElements(futureParentElement);
11621171
}
@@ -1326,7 +1335,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
13261335
transcludedScope.$$transcluded = true;
13271336
}
13281337

1329-
return transcludeFn(transcludedScope, cloneFn, controllers, previousBoundTranscludeFn, futureParentElement);
1338+
return transcludeFn(transcludedScope, cloneFn, previousBoundTranscludeFn, controllers, futureParentElement);
13301339
};
13311340

13321341
return boundTranscludeFn;
@@ -1794,7 +1803,13 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
17941803
isolateScope = scope.$new(true);
17951804
}
17961805

1797-
transcludeFn = boundTranscludeFn && controllersBoundTransclude;
1806+
if (boundTranscludeFn) {
1807+
// track `boundTranscludeFn` so it can be unwrapped if `transcludeFn`
1808+
// is later passed as `parentBoundTranscludeFn` to `publicLinkFn`
1809+
transcludeFn = controllersBoundTransclude;
1810+
transcludeFn.$$boundTransclude = boundTranscludeFn;
1811+
}
1812+
17981813
if (controllerDirectives) {
17991814
// TODO: merge `controllers` and `elementControllers` into single object.
18001815
controllers = {};

test/ng/compileSpec.js

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5209,6 +5209,119 @@ describe('$compile', function() {
52095209
});
52105210

52115211

5212+
// see issue https://github.com/angular/angular.js/issues/9413
5213+
describe('passing a parent bound transclude function to the link ' +
5214+
'function returned from `$compile`', function() {
5215+
5216+
function countScopes($rootScope) {
5217+
return [$rootScope].concat(
5218+
getChildScopes($rootScope)
5219+
).length;
5220+
}
5221+
5222+
function getChildScopes(scope) {
5223+
var children = [];
5224+
if (!scope.$$childHead) { return children; }
5225+
var childScope = scope.$$childHead;
5226+
do {
5227+
children.push(childScope);
5228+
children = children.concat(getChildScopes(childScope));
5229+
} while ((childScope = childScope.$$nextSibling));
5230+
return children;
5231+
}
5232+
5233+
beforeEach(module(function() {
5234+
directive('lazyCompile', function($compile) {
5235+
return {
5236+
compile: function(tElement, tAttrs) {
5237+
var content = tElement.contents();
5238+
tElement.empty();
5239+
return function(scope, element, attrs, ctrls, transcludeFn) {
5240+
element.append(content);
5241+
$compile(content)(scope, undefined, transcludeFn);
5242+
};
5243+
}
5244+
};
5245+
});
5246+
directive('toggle', function() {
5247+
return {
5248+
scope: {t: '=toggle'},
5249+
transclude: true,
5250+
template: '<div ng-if="t"><lazy-compile><div ng-transclude></div></lazy-compile></div>'
5251+
};
5252+
});
5253+
}));
5254+
5255+
it('should preserve the bound scope', function() {
5256+
5257+
inject(function($compile, $rootScope) {
5258+
element = $compile(
5259+
'<div>' +
5260+
'<div ng-init="outer=true"></div>' +
5261+
'<div toggle="t">' +
5262+
'<span ng-if="outer">Success</span><span ng-if="!outer">Error</span>' +
5263+
'</div>' +
5264+
'</div>')($rootScope);
5265+
5266+
$rootScope.$apply('t = false');
5267+
expect(countScopes($rootScope)).toBe(2);
5268+
expect(element.text()).toBe('');
5269+
5270+
$rootScope.$apply('t = true');
5271+
expect(countScopes($rootScope)).toBe(5);
5272+
expect(element.text()).toBe('Success');
5273+
5274+
$rootScope.$apply('t = false');
5275+
expect(countScopes($rootScope)).toBe(2);
5276+
expect(element.text()).toBe('');
5277+
5278+
$rootScope.$apply('t = true');
5279+
expect(countScopes($rootScope)).toBe(5);
5280+
expect(element.text()).toBe('Success');
5281+
});
5282+
});
5283+
5284+
5285+
it('should preserve the bound scope when using recursive transclusion', function() {
5286+
5287+
directive('recursiveTransclude', function() {
5288+
return {
5289+
transclude: true,
5290+
template: '<div><lazy-compile><div ng-transclude></div></lazy-compile></div>'
5291+
};
5292+
});
5293+
5294+
inject(function($compile, $rootScope) {
5295+
element = $compile(
5296+
'<div>' +
5297+
'<div ng-init="outer=true"></div>' +
5298+
'<div toggle="t">' +
5299+
'<div recursive-transclude>' +
5300+
'<span ng-if="outer">Success</span><span ng-if="!outer">Error</span>' +
5301+
'</div>' +
5302+
'</div>' +
5303+
'</div>')($rootScope);
5304+
5305+
$rootScope.$apply('t = false');
5306+
expect(countScopes($rootScope)).toBe(2);
5307+
expect(element.text()).toBe('');
5308+
5309+
$rootScope.$apply('t = true');
5310+
expect(countScopes($rootScope)).toBe(5);
5311+
expect(element.text()).toBe('Success');
5312+
5313+
$rootScope.$apply('t = false');
5314+
expect(countScopes($rootScope)).toBe(2);
5315+
expect(element.text()).toBe('');
5316+
5317+
$rootScope.$apply('t = true');
5318+
expect(countScopes($rootScope)).toBe(5);
5319+
expect(element.text()).toBe('Success');
5320+
});
5321+
});
5322+
});
5323+
5324+
52125325
// see issue https://github.com/angular/angular.js/issues/9095
52135326
describe('removing a transcluded element', function() {
52145327

0 commit comments

Comments
 (0)