Skip to content

Commit

Permalink
fix($compile): do not rebind parent bound transclude functions
Browse files Browse the repository at this point in the history
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
  • Loading branch information
dlongley committed Oct 14, 2014
1 parent 7f4d24c commit 3859ea0
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 4 deletions.
14 changes: 10 additions & 4 deletions src/ng/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -1153,7 +1153,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
maxPriority, ignoreDirective, previousCompileContext);
compile.$$addScopeClass($compileNodes);
var namespace = null;
return function publicLinkFn(scope, cloneConnectFn, transcludeControllers, parentBoundTranscludeFn, futureParentElement){
return function publicLinkFn(scope, cloneConnectFn, parentBoundTranscludeFn, transcludeControllers, futureParentElement){
assertArg(scope, 'scope');
if (!namespace) {
namespace = detectNamespaceForChildElements(futureParentElement);
Expand Down Expand Up @@ -1324,7 +1324,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
transcludedScope.$$transcluded = true;
}

return transcludeFn(transcludedScope, cloneFn, controllers, previousBoundTranscludeFn, futureParentElement);
return transcludeFn(transcludedScope, cloneFn, previousBoundTranscludeFn, controllers, futureParentElement);
};

return boundTranscludeFn;
Expand Down Expand Up @@ -1953,7 +1953,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
var transcludeControllers;

// No scope passed in:
if (!isScope(scope)) {
if (!isScope(scope) && !isUndefined(scope)) {
futureParentElement = cloneAttachFn;
cloneAttachFn = scope;
scope = undefined;
Expand All @@ -1965,7 +1965,13 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
if (!futureParentElement) {
futureParentElement = hasElementTranscludeDirective ? $element.parent() : $element;
}
return boundTranscludeFn(scope, cloneAttachFn, transcludeControllers, futureParentElement, scopeToChild);

// When a transcludeFn is passed to publicLinkFn, it is already
// a `controllersBoundTransclude` function, so prevent double-wrapping
// by using the previously-wrapped boundTranscludeFn.
var unwrappedFn = boundTranscludeFn.$$boundTransclude || boundTranscludeFn;
controllersBoundTransclude.$$boundTransclude = unwrappedFn;
return unwrappedFn(scope, cloneAttachFn, transcludeControllers, futureParentElement, scopeToChild);
}
}
}
Expand Down
73 changes: 73 additions & 0 deletions test/ng/compileSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -5124,6 +5124,79 @@ describe('$compile', function() {
});


// see issue https://github.com/angular/angular.js/issues/9413
it('should preserve the bound scope when passing a parent bound ' +
'transclude function to the link function returned from `$compile`', function() {

function countScopes($rootScope) {
return [$rootScope].concat(
getChildScopes($rootScope)
).length;
}

function getChildScopes(scope) {
var children = [];
if (!scope.$$childHead) { return children; }
var childScope = scope.$$childHead;
do {
children.push(childScope);
children = children.concat(getChildScopes(childScope));
} while ((childScope = childScope.$$nextSibling));
return children;
}

module(function() {
directive('lazyCompile', function($compile) {
return {
terminal: true,
priority: 1,
compile: function(tElement, tAttrs) {
var content = tElement.contents();
tElement.empty();
return function(scope, element, attrs, ctrls, transcludeFn) {
element.append(content);
$compile(content)(scope, undefined, transcludeFn);
};
}
};
});
directive('toggle', function() {
return {
scope: {t: '=toggle'},
transclude: true,
template: '<div ng-if="t"><lazy-compile><div ng-transclude></div></lazy-compile></div>'
};
});
});

inject(function($compile) {
element = $compile(
'<div>' +
'<div ng-init="outer=true"></div>' +
'<div toggle="t">' +
'<span ng-if="outer">Success</span><span ng-if="!outer">Error</span>' +
'</div>' +
'</div>')($rootScope);

$rootScope.$apply('t = false');
expect(countScopes($rootScope)).toBe(2);
expect(element.text()).toBe('');

$rootScope.$apply('t = true');
expect(countScopes($rootScope)).toBe(5);
expect(element.text()).toBe('Success');

$rootScope.$apply('t = false');
expect(countScopes($rootScope)).toBe(2);
expect(element.text()).toBe('');

$rootScope.$apply('t = true');
expect(countScopes($rootScope)).toBe(5);
expect(element.text()).toBe('Success');
});
});


// see issue https://github.com/angular/angular.js/issues/9095
describe('removing a transcluded element', function() {

Expand Down

0 comments on commit 3859ea0

Please sign in to comment.