Skip to content

Commit 660f916

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 74981c9 commit 660f916

File tree

2 files changed

+109
-2
lines changed

2 files changed

+109
-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

+92
Original file line numberDiff line numberDiff line change
@@ -5209,6 +5209,98 @@ describe('$compile', function() {
52095209
});
52105210

52115211

5212+
// see issue https://github.com/angular/angular.js/issues/9413
5213+
ddescribe('passing a parent bound transclude function to the link ' +
5214+
'function returned from `$compile`', function() {
5215+
5216+
beforeEach(module(function() {
5217+
directive('lazyCompile', function($compile) {
5218+
return {
5219+
compile: function(tElement, tAttrs) {
5220+
var content = tElement.contents();
5221+
tElement.empty();
5222+
return function(scope, element, attrs, ctrls, transcludeFn) {
5223+
element.append(content);
5224+
$compile(content)(scope, undefined, transcludeFn);
5225+
};
5226+
}
5227+
};
5228+
});
5229+
directive('toggle', valueFn({
5230+
scope: {t: '=toggle'},
5231+
transclude: true,
5232+
template: '<div ng-if="t"><lazy-compile><div ng-transclude></div></lazy-compile></div>'
5233+
}));
5234+
}));
5235+
5236+
it('should preserve the bound scope', function() {
5237+
5238+
inject(function($compile, $rootScope) {
5239+
element = $compile(
5240+
'<div>' +
5241+
'<div ng-init="outer=true"></div>' +
5242+
'<div toggle="t">' +
5243+
'<span ng-if="outer">Success</span><span ng-if="!outer">Error</span>' +
5244+
'</div>' +
5245+
'</div>')($rootScope);
5246+
5247+
$rootScope.$apply('t = false');
5248+
expect($rootScope.$countChildScopes()).toBe(1);
5249+
expect(element.text()).toBe('');
5250+
5251+
$rootScope.$apply('t = true');
5252+
expect($rootScope.$countChildScopes()).toBe(4);
5253+
expect(element.text()).toBe('Success');
5254+
5255+
$rootScope.$apply('t = false');
5256+
expect($rootScope.$countChildScopes()).toBe(1);
5257+
expect(element.text()).toBe('');
5258+
5259+
$rootScope.$apply('t = true');
5260+
expect($rootScope.$countChildScopes()).toBe(4);
5261+
expect(element.text()).toBe('Success');
5262+
});
5263+
});
5264+
5265+
5266+
it('should preserve the bound scope when using recursive transclusion', function() {
5267+
5268+
directive('recursiveTransclude', valueFn({
5269+
transclude: true,
5270+
template: '<div><lazy-compile><div ng-transclude></div></lazy-compile></div>'
5271+
}));
5272+
5273+
inject(function($compile, $rootScope) {
5274+
element = $compile(
5275+
'<div>' +
5276+
'<div ng-init="outer=true"></div>' +
5277+
'<div toggle="t">' +
5278+
'<div recursive-transclude>' +
5279+
'<span ng-if="outer">Success</span><span ng-if="!outer">Error</span>' +
5280+
'</div>' +
5281+
'</div>' +
5282+
'</div>')($rootScope);
5283+
5284+
$rootScope.$apply('t = false');
5285+
expect($rootScope.$countChildScopes()).toBe(1);
5286+
expect(element.text()).toBe('');
5287+
5288+
$rootScope.$apply('t = true');
5289+
expect($rootScope.$countChildScopes()).toBe(4);
5290+
expect(element.text()).toBe('Success');
5291+
5292+
$rootScope.$apply('t = false');
5293+
expect($rootScope.$countChildScopes()).toBe(1);
5294+
expect(element.text()).toBe('');
5295+
5296+
$rootScope.$apply('t = true');
5297+
expect($rootScope.$countChildScopes()).toBe(4);
5298+
expect(element.text()).toBe('Success');
5299+
});
5300+
});
5301+
});
5302+
5303+
52125304
// see issue https://github.com/angular/angular.js/issues/9095
52135305
describe('removing a transcluded element', function() {
52145306

0 commit comments

Comments
 (0)