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

Commit 4cdb797

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 7f4d24c commit 4cdb797

File tree

2 files changed

+87
-4
lines changed

2 files changed

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

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

51265126

5127+
// see issue https://github.com/angular/angular.js/issues/9413
5128+
it('should preserve the bound scope when passing a parent bound ' +
5129+
'transclude function to the link 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+
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+
inject(function($compile) {
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+
51275200
// see issue https://github.com/angular/angular.js/issues/9095
51285201
describe('removing a transcluded element', function() {
51295202

0 commit comments

Comments
 (0)