Skip to content

Commit 026407e

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 875f8f6 commit 026407e

File tree

2 files changed

+126
-2
lines changed

2 files changed

+126
-2
lines changed

src/ng/compile.js

+17-2
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

+109
Original file line numberDiff line numberDiff line change
@@ -5209,6 +5209,115 @@ 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', valueFn({
5247+
scope: {t: '=toggle'},
5248+
transclude: true,
5249+
template: '<div ng-if="t"><lazy-compile><div ng-transclude></div></lazy-compile></div>'
5250+
}));
5251+
}));
5252+
5253+
it('should preserve the bound scope', function() {
5254+
5255+
inject(function($compile, $rootScope) {
5256+
element = $compile(
5257+
'<div>' +
5258+
'<div ng-init="outer=true"></div>' +
5259+
'<div toggle="t">' +
5260+
'<span ng-if="outer">Success</span><span ng-if="!outer">Error</span>' +
5261+
'</div>' +
5262+
'</div>')($rootScope);
5263+
5264+
$rootScope.$apply('t = false');
5265+
expect(countScopes($rootScope)).toBe(2);
5266+
expect(element.text()).toBe('');
5267+
5268+
$rootScope.$apply('t = true');
5269+
expect(countScopes($rootScope)).toBe(5);
5270+
expect(element.text()).toBe('Success');
5271+
5272+
$rootScope.$apply('t = false');
5273+
expect(countScopes($rootScope)).toBe(2);
5274+
expect(element.text()).toBe('');
5275+
5276+
$rootScope.$apply('t = true');
5277+
expect(countScopes($rootScope)).toBe(5);
5278+
expect(element.text()).toBe('Success');
5279+
});
5280+
});
5281+
5282+
5283+
it('should preserve the bound scope when using recursive transclusion', function() {
5284+
5285+
directive('recursiveTransclude', valueFn({
5286+
transclude: true,
5287+
template: '<div><lazy-compile><div ng-transclude></div></lazy-compile></div>'
5288+
}));
5289+
5290+
inject(function($compile, $rootScope) {
5291+
element = $compile(
5292+
'<div>' +
5293+
'<div ng-init="outer=true"></div>' +
5294+
'<div toggle="t">' +
5295+
'<div recursive-transclude>' +
5296+
'<span ng-if="outer">Success</span><span ng-if="!outer">Error</span>' +
5297+
'</div>' +
5298+
'</div>' +
5299+
'</div>')($rootScope);
5300+
5301+
$rootScope.$apply('t = false');
5302+
expect(countScopes($rootScope)).toBe(2);
5303+
expect(element.text()).toBe('');
5304+
5305+
$rootScope.$apply('t = true');
5306+
expect(countScopes($rootScope)).toBe(5);
5307+
expect(element.text()).toBe('Success');
5308+
5309+
$rootScope.$apply('t = false');
5310+
expect(countScopes($rootScope)).toBe(2);
5311+
expect(element.text()).toBe('');
5312+
5313+
$rootScope.$apply('t = true');
5314+
expect(countScopes($rootScope)).toBe(5);
5315+
expect(element.text()).toBe('Success');
5316+
});
5317+
});
5318+
});
5319+
5320+
52125321
// see issue https://github.com/angular/angular.js/issues/9095
52135322
describe('removing a transcluded element', function() {
52145323

0 commit comments

Comments
 (0)