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

Commit 2adaff0

Browse files
dchermanpetebacondarwin
authored andcommitted
fix(ngTransclude): only compile fallback content if necessary
If the instance of the directive does provide transcluded content, then the fallback content should not be compiled and linked as it will never be used. If the instance of the directive does not provide transcluded content, then the transcluded scope that was created for this non-existent content is never used, so it should be destroy in order to clean up unwanted memory use and digests. Fixes #14768 Fixes #14765 Closes #14775
1 parent 857d78d commit 2adaff0

File tree

2 files changed

+98
-30
lines changed

2 files changed

+98
-30
lines changed

src/ng/directive/ngTransclude.js

Lines changed: 37 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -159,35 +159,45 @@
159159
* </example>
160160
*/
161161
var ngTranscludeMinErr = minErr('ngTransclude');
162-
var ngTranscludeDirective = ngDirective({
163-
restrict: 'EAC',
164-
link: function($scope, $element, $attrs, controller, $transclude) {
162+
var ngTranscludeDirective = ['$compile', function($compile) {
163+
return {
164+
restrict: 'EAC',
165+
terminal: true,
166+
link: function($scope, $element, $attrs, controller, $transclude) {
167+
if ($attrs.ngTransclude === $attrs.$attr.ngTransclude) {
168+
// If the attribute is of the form: `ng-transclude="ng-transclude"`
169+
// then treat it like the default
170+
$attrs.ngTransclude = '';
171+
}
165172

166-
if ($attrs.ngTransclude === $attrs.$attr.ngTransclude) {
167-
// If the attribute is of the form: `ng-transclude="ng-transclude"`
168-
// then treat it like the default
169-
$attrs.ngTransclude = '';
170-
}
173+
function ngTranscludeCloneAttachFn(clone, transcludedScope) {
174+
if (clone.length) {
175+
$element.empty();
176+
$element.append(clone);
177+
} else {
178+
// Since this is the fallback content rather than the transcluded content,
179+
// we compile against the scope we were linked against rather than the transcluded
180+
// scope since this is the directive's own content
181+
$compile($element.contents())($scope);
171182

172-
function ngTranscludeCloneAttachFn(clone) {
173-
if (clone.length) {
174-
$element.empty();
175-
$element.append(clone);
183+
// There is nothing linked against the transcluded scope since no content was available,
184+
// so it should be safe to clean up the generated scope.
185+
transcludedScope.$destroy();
186+
}
176187
}
177-
}
178188

179-
if (!$transclude) {
180-
throw ngTranscludeMinErr('orphan',
181-
'Illegal use of ngTransclude directive in the template! ' +
182-
'No parent directive that requires a transclusion found. ' +
183-
'Element: {0}',
184-
startingTag($element));
185-
}
186-
187-
// If there is no slot name defined or the slot name is not optional
188-
// then transclude the slot
189-
var slotName = $attrs.ngTransclude || $attrs.ngTranscludeSlot;
190-
$transclude(ngTranscludeCloneAttachFn, null, slotName);
191-
}
192-
});
189+
if (!$transclude) {
190+
throw ngTranscludeMinErr('orphan',
191+
'Illegal use of ngTransclude directive in the template! ' +
192+
'No parent directive that requires a transclusion found. ' +
193+
'Element: {0}',
194+
startingTag($element));
195+
}
193196

197+
// If there is no slot name defined or the slot name is not optional
198+
// then transclude the slot
199+
var slotName = $attrs.ngTransclude || $attrs.ngTranscludeSlot;
200+
$transclude(ngTranscludeCloneAttachFn, null, slotName);
201+
}
202+
};
203+
}];

test/ng/compileSpec.js

Lines changed: 61 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7941,7 +7941,7 @@ describe('$compile', function() {
79417941
directive('trans', function() {
79427942
return {
79437943
transclude: true,
7944-
template: '<div ng-transclude>old stuff! </div>'
7944+
template: '<div ng-transclude>old stuff!</div>'
79457945
};
79467946
});
79477947
});
@@ -7958,18 +7958,76 @@ describe('$compile', function() {
79587958
directive('trans', function() {
79597959
return {
79607960
transclude: true,
7961-
template: '<div ng-transclude>old stuff! </div>'
7961+
template: '<div ng-transclude>old stuff!</div>'
79627962
};
79637963
});
79647964
});
79657965
inject(function(log, $rootScope, $compile) {
79667966
element = $compile('<div trans></div>')($rootScope);
79677967
$rootScope.$apply();
7968-
expect(sortedHtml(element.html())).toEqual('<div ng-transclude="">old stuff! </div>');
7968+
expect(sortedHtml(element.html())).toEqual('<div ng-transclude="">old stuff!</div>');
79697969
});
79707970
});
79717971

79727972

7973+
it('should not compile the fallback content if transcluded content is provided', function() {
7974+
var contentsDidLink = false;
7975+
7976+
module(function() {
7977+
directive('inner', function() {
7978+
return {
7979+
restrict: 'E',
7980+
template: 'old stuff! ',
7981+
link: function() {
7982+
contentsDidLink = true;
7983+
}
7984+
};
7985+
});
7986+
7987+
directive('trans', function() {
7988+
return {
7989+
transclude: true,
7990+
template: '<div ng-transclude><inner></inner></div>'
7991+
};
7992+
});
7993+
});
7994+
inject(function($rootScope, $compile) {
7995+
element = $compile('<div trans>unicorn!</div>')($rootScope);
7996+
$rootScope.$apply();
7997+
expect(sortedHtml(element.html())).toEqual('<div ng-transclude="">unicorn!</div>');
7998+
expect(contentsDidLink).toBe(false);
7999+
});
8000+
});
8001+
8002+
it('should compile and link the fallback content if no transcluded content is provided', function() {
8003+
var contentsDidLink = false;
8004+
8005+
module(function() {
8006+
directive('inner', function() {
8007+
return {
8008+
restrict: 'E',
8009+
template: 'old stuff! ',
8010+
link: function() {
8011+
contentsDidLink = true;
8012+
}
8013+
};
8014+
});
8015+
8016+
directive('trans', function() {
8017+
return {
8018+
transclude: true,
8019+
template: '<div ng-transclude><inner></inner></div>'
8020+
};
8021+
});
8022+
});
8023+
inject(function(log, $rootScope, $compile) {
8024+
element = $compile('<div trans></div>')($rootScope);
8025+
$rootScope.$apply();
8026+
expect(sortedHtml(element.html())).toEqual('<div ng-transclude=""><inner>old stuff! </inner></div>');
8027+
expect(contentsDidLink).toBe(true);
8028+
});
8029+
});
8030+
79738031
it('should throw on an ng-transclude element inside no transclusion directive', function() {
79748032
inject(function($rootScope, $compile) {
79758033
// we need to do this because different browsers print empty attributes differently

0 commit comments

Comments
 (0)