Skip to content

Commit e5dac10

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 e5dac10

File tree

2 files changed

+131
-3
lines changed

2 files changed

+131
-3
lines changed

src/ng/compile.js

+18-3
Original file line numberDiff line numberDiff line change
@@ -1153,8 +1153,17 @@ 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');
1158+
1159+
// When `parentBoundTranscludeFn` is passed, it is a
1160+
// `controllersBoundTransclude` function (it was previously passed
1161+
// as `transclude` to directive.link) so we must unwrap it to get
1162+
// its `boundTranscludeFn`
1163+
if (parentBoundTranscludeFn && parentBoundTranscludeFn.$$boundTransclude) {
1164+
parentBoundTranscludeFn = parentBoundTranscludeFn.$$boundTransclude;
1165+
}
1166+
11581167
if (!namespace) {
11591168
namespace = detectNamespaceForChildElements(futureParentElement);
11601169
}
@@ -1324,7 +1333,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
13241333
transcludedScope.$$transcluded = true;
13251334
}
13261335

1327-
return transcludeFn(transcludedScope, cloneFn, controllers, previousBoundTranscludeFn, futureParentElement);
1336+
return transcludeFn(transcludedScope, cloneFn, previousBoundTranscludeFn, controllers, futureParentElement);
13281337
};
13291338

13301339
return boundTranscludeFn;
@@ -1793,7 +1802,13 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
17931802
isolateScope = scope.$new(true);
17941803
}
17951804

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

test/ng/compileSpec.js

+113
Original file line numberDiff line numberDiff line change
@@ -5124,6 +5124,119 @@ 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+
compile: function(tElement, tAttrs) {
5152+
var content = tElement.contents();
5153+
tElement.empty();
5154+
return function(scope, element, attrs, ctrls, transcludeFn) {
5155+
element.append(content);
5156+
$compile(content)(scope, undefined, transcludeFn);
5157+
};
5158+
}
5159+
};
5160+
});
5161+
directive('toggle', function() {
5162+
return {
5163+
scope: {t: '=toggle'},
5164+
transclude: true,
5165+
template: '<div ng-if="t"><lazy-compile><div ng-transclude></div></lazy-compile></div>'
5166+
};
5167+
});
5168+
}));
5169+
5170+
it('should preserve the bound scope', function() {
5171+
5172+
inject(function($compile, $rootScope) {
5173+
element = $compile(
5174+
'<div>' +
5175+
'<div ng-init="outer=true"></div>' +
5176+
'<div toggle="t">' +
5177+
'<span ng-if="outer">Success</span><span ng-if="!outer">Error</span>' +
5178+
'</div>' +
5179+
'</div>')($rootScope);
5180+
5181+
$rootScope.$apply('t = false');
5182+
expect(countScopes($rootScope)).toBe(2);
5183+
expect(element.text()).toBe('');
5184+
5185+
$rootScope.$apply('t = true');
5186+
expect(countScopes($rootScope)).toBe(5);
5187+
expect(element.text()).toBe('Success');
5188+
5189+
$rootScope.$apply('t = false');
5190+
expect(countScopes($rootScope)).toBe(2);
5191+
expect(element.text()).toBe('');
5192+
5193+
$rootScope.$apply('t = true');
5194+
expect(countScopes($rootScope)).toBe(5);
5195+
expect(element.text()).toBe('Success');
5196+
});
5197+
});
5198+
5199+
5200+
it('should preserve the bound scope when using recursive transclusion', function() {
5201+
5202+
directive('recursiveTransclude', function() {
5203+
return {
5204+
transclude: true,
5205+
template: '<div><lazy-compile><div ng-transclude></div></lazy-compile></div>'
5206+
};
5207+
});
5208+
5209+
inject(function($compile, $rootScope) {
5210+
element = $compile(
5211+
'<div>' +
5212+
'<div ng-init="outer=true"></div>' +
5213+
'<div toggle="t">' +
5214+
'<div recursive-transclude>' +
5215+
'<span ng-if="outer">Success</span><span ng-if="!outer">Error</span>' +
5216+
'</div>' +
5217+
'</div>' +
5218+
'</div>')($rootScope);
5219+
5220+
$rootScope.$apply('t = false');
5221+
expect(countScopes($rootScope)).toBe(2);
5222+
expect(element.text()).toBe('');
5223+
5224+
$rootScope.$apply('t = true');
5225+
expect(countScopes($rootScope)).toBe(5);
5226+
expect(element.text()).toBe('Success');
5227+
5228+
$rootScope.$apply('t = false');
5229+
expect(countScopes($rootScope)).toBe(2);
5230+
expect(element.text()).toBe('');
5231+
5232+
$rootScope.$apply('t = true');
5233+
expect(countScopes($rootScope)).toBe(5);
5234+
expect(element.text()).toBe('Success');
5235+
});
5236+
});
5237+
});
5238+
5239+
51275240
// see issue https://github.com/angular/angular.js/issues/9095
51285241
describe('removing a transcluded element', function() {
51295242

0 commit comments

Comments
 (0)