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

Transclude issue #7530

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 15 additions & 8 deletions src/ng/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -937,7 +937,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
return linkFnFound ? compositeLinkFn : null;

function compositeLinkFn(scope, nodeList, $rootElement, boundTranscludeFn) {
var nodeLinkFn, childLinkFn, node, $node, childScope, childTranscludeFn, i, ii, n;
var nodeLinkFn, childLinkFn, node, $node, childScope, i, ii, n, childBoundTranscludeFn;

// copy nodeList so that linking doesn't break due to live list updates.
var nodeListLength = nodeList.length,
Expand All @@ -959,14 +959,19 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
} else {
childScope = scope;
}
childTranscludeFn = nodeLinkFn.transclude;
if (childTranscludeFn || (!boundTranscludeFn && transcludeFn)) {
nodeLinkFn(childLinkFn, childScope, node, $rootElement,
createBoundTranscludeFn(scope, childTranscludeFn || transcludeFn)
);

// We need to create a new boundTranscludeFn if
// - a directive on this element wants to transclude
// or
// - there is no boundTranscludeFn already and a transcludeFn was passed in
if ( nodeLinkFn.transcludeOnThisElement || (!boundTranscludeFn && transcludeFn) ) {
childBoundTranscludeFn = createBoundTranscludeFn(scope, nodeLinkFn.transclude || transcludeFn);
} else {
nodeLinkFn(childLinkFn, childScope, node, $rootElement, boundTranscludeFn);
childBoundTranscludeFn = boundTranscludeFn;
}

nodeLinkFn(childLinkFn, childScope, node, $rootElement, childBoundTranscludeFn);

} else if (childLinkFn) {
childLinkFn(scope, node.childNodes, undefined, boundTranscludeFn);
}
Expand Down Expand Up @@ -1342,7 +1347,9 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
}

nodeLinkFn.scope = newScopeDirective && newScopeDirective.scope === true;
nodeLinkFn.transclude = hasTranscludeDirective && childTranscludeFn;
nodeLinkFn.transcludeOnThisElement = hasTranscludeDirective;
nodeLinkFn.transclude = childTranscludeFn;

previousCompileContext.hasElementTranscludeDirective = hasElementTranscludeDirective;

// might be normal or delayed nodeLinkFn depending on if templateUrl is present
Expand Down
108 changes: 108 additions & 0 deletions test/ng/compileSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1497,6 +1497,114 @@ describe('$compile', function() {
));


describe('nested transcludes', function() {

beforeEach(module(function($compileProvider) {

$compileProvider.directive('noop', valueFn({}));

$compileProvider.directive('sync', valueFn({
template: '<div ng-transclude></div>',
transclude: true
}));

$compileProvider.directive('async', valueFn({
templateUrl: 'async',
transclude: true
}));

$compileProvider.directive('syncSync', valueFn({
template: '<div noop><div sync><div ng-transclude></div></div></div>',
transclude: true
}));

$compileProvider.directive('syncAsync', valueFn({
template: '<div noop><div async><div ng-transclude></div></div></div>',
transclude: true
}));

$compileProvider.directive('asyncSync', valueFn({
templateUrl: 'asyncSync',
transclude: true
}));

$compileProvider.directive('asyncAsync', valueFn({
templateUrl: 'asyncAsync',
transclude: true
}));

}));

beforeEach(inject(function($templateCache) {
$templateCache.put('async', '<div ng-transclude></div>');
$templateCache.put('asyncSync', '<div noop><div sync><div ng-transclude></div></div></div>');
$templateCache.put('asyncAsync', '<div noop><div async><div ng-transclude></div></div></div>');
}));


it('should allow nested transclude directives with sync template containing sync template', inject(function($compile, $rootScope) {
element = $compile('<div sync-sync>transcluded content</div>')($rootScope);
$rootScope.$digest();
expect(element.text()).toEqual('transcluded content');
}));

it('should allow nested transclude directives with sync template containing async template', inject(function($compile, $rootScope) {
element = $compile('<div sync-async>transcluded content</div>')($rootScope);
$rootScope.$digest();
expect(element.text()).toEqual('transcluded content');
}));

it('should allow nested transclude directives with async template containing sync template', inject(function($compile, $rootScope) {
element = $compile('<div async-sync>transcluded content</div>')($rootScope);
$rootScope.$digest();
expect(element.text()).toEqual('transcluded content');
}));

it('should allow nested transclude directives with async template containing asynch template', inject(function($compile, $rootScope) {
element = $compile('<div async-async>transcluded content</div>')($rootScope);
$rootScope.$digest();
expect(element.text()).toEqual('transcluded content');
}));
});

describe('transclude and children', function() {
beforeEach(module(function($compileProvider) {

$compileProvider.directive('myExample', valueFn({
scope: {},
link: function link(scope, element, attrs) {
var foo = element[0].querySelector('.foo');
scope.children = angular.element(foo).children().length;
},
template: '<div>' +
'<div>myExample {{children}}!</div>' +
'<div ng-if="children">has children</div>' +
'<div class="foo" ng-transclude></div>' +
'</div>',
transclude: true

}));

}));

it("should not pick up too many children when transcluding", inject(function($compile, $rootScope) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a small nit, because I had trouble coming up with a good name for this failing test case when I was looking at this on Tuesday, too, but I feel like this description makes it hard to understand what the actual expected behaviour is. "should not..." and "too many..." feel wrong here.

Again, I'm not sure what a good name for this would be, so maybe this is okay, it just doesn't seem to send the right signals, IMO.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll rename

var element = $compile('<div my-example></div>')($rootScope);
$rootScope.$digest();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need $digest twice?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vojtajina This is not needed now I've rewritten the ngif fix. I'll close this and send in a series of prs for all this today

$rootScope.$digest();
expect(element.text()).toEqual('myExample 0!');
dealoc(element);

element = $compile('<div my-example><p></p></div>')($rootScope);
$rootScope.$digest();
$rootScope.$digest();
expect(element.text()).toEqual('myExample 1!has children');
dealoc(element);
}));

});



it("should fail if replacing and template doesn't have a single root element", function() {
module(function($exceptionHandlerProvider) {
$exceptionHandlerProvider.mode('log');
Expand Down