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

Commit

Permalink
fix($compile): accessing controllers of transcluded directives from c…
Browse files Browse the repository at this point in the history
…hildren

Additional API (backwards compatible)
- Injects `$transclude` (see directive controllers) as 5th argument to directive link functions.
- `$transclude` takes an optional scope as first parameter that overrides the
  bound scope.

Deprecations:
- `transclude` parameter of directive compile functions (use the new parameter for link functions instead).

Refactorings:
- Don't use comment node to temporarily store controllers
- `ngIf`, `ngRepeat`, ... now all use `$transclude`

Closes #4935.
  • Loading branch information
tbosch authored and vojtajina committed Nov 15, 2013
1 parent c785918 commit 90f8707
Show file tree
Hide file tree
Showing 11 changed files with 434 additions and 73 deletions.
141 changes: 94 additions & 47 deletions src/ng/compile.js

Large diffs are not rendered by default.

6 changes: 2 additions & 4 deletions src/ng/directive/ngIf.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,15 +86,14 @@ var ngIfDirective = ['$animate', function($animate) {
terminal: true,
restrict: 'A',
$$tlb: true,
compile: function (element, attr, transclude) {
return function ($scope, $element, $attr) {
link: function ($scope, $element, $attr, ctrl, $transclude) {
var block, childScope;
$scope.$watch($attr.ngIf, function ngIfWatchAction(value) {

if (toBoolean(value)) {
if (!childScope) {
childScope = $scope.$new();
transclude(childScope, function (clone) {
$transclude(childScope, function (clone) {
block = {
startNode: clone[0],
endNode: clone[clone.length++] = document.createComment(' end ngIf: ' + $attr.ngIf + ' ')
Expand All @@ -115,7 +114,6 @@ var ngIfDirective = ['$animate', function($animate) {
}
}
});
};
}
};
}];
6 changes: 3 additions & 3 deletions src/ng/directive/ngInclude.js
Original file line number Diff line number Diff line change
Expand Up @@ -154,12 +154,12 @@ var ngIncludeDirective = ['$http', '$templateCache', '$anchorScroll', '$compile'
priority: 400,
terminal: true,
transclude: 'element',
compile: function(element, attr, transclusion) {
compile: function(element, attr) {
var srcExp = attr.ngInclude || attr.src,
onloadExp = attr.onload || '',
autoScrollExp = attr.autoscroll;

return function(scope, $element) {
return function(scope, $element, $attr, ctrl, $transclude) {
var changeCounter = 0,
currentScope,
currentElement;
Expand Down Expand Up @@ -188,7 +188,7 @@ var ngIncludeDirective = ['$http', '$templateCache', '$anchorScroll', '$compile'
if (thisChangeId !== changeCounter) return;
var newScope = scope.$new();

transclusion(newScope, function(clone) {
$transclude(newScope, function(clone) {
cleanupLastIncludeContent();

currentScope = newScope;
Expand Down
6 changes: 2 additions & 4 deletions src/ng/directive/ngRepeat.js
Original file line number Diff line number Diff line change
Expand Up @@ -201,8 +201,7 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {
priority: 1000,
terminal: true,
$$tlb: true,
compile: function(element, attr, linker) {
return function($scope, $element, $attr){
link: function($scope, $element, $attr, ctrl, $transclude){
var expression = $attr.ngRepeat;
var match = expression.match(/^\s*(.+)\s+in\s+(.*?)\s*(\s+track\s+by\s+(.+)\s*)?$/),
trackByExp, trackByExpGetter, trackByIdExpFn, trackByIdArrayFn, trackByIdObjFn,
Expand Down Expand Up @@ -364,7 +363,7 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {
// jshint bitwise: true

if (!block.startNode) {
linker(childScope, function(clone) {
$transclude(childScope, function(clone) {
clone[clone.length++] = document.createComment(' end ngRepeat: ' + expression + ' ');
$animate.enter(clone, null, jqLite(previousNode));
previousNode = clone;
Expand All @@ -377,7 +376,6 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {
}
lastBlockMap = nextBlockMap;
});
};
}
};
}];
Expand Down
16 changes: 7 additions & 9 deletions src/ng/directive/ngSwitch.js
Original file line number Diff line number Diff line change
Expand Up @@ -160,10 +160,10 @@ var ngSwitchWhenDirective = ngDirective({
transclude: 'element',
priority: 800,
require: '^ngSwitch',
compile: function(element, attrs, transclude) {
return function(scope, element, attr, ctrl) {
compile: function(element, attrs) {
return function(scope, element, attr, ctrl, $transclude) {
ctrl.cases['!' + attrs.ngSwitchWhen] = (ctrl.cases['!' + attrs.ngSwitchWhen] || []);
ctrl.cases['!' + attrs.ngSwitchWhen].push({ transclude: transclude, element: element });
ctrl.cases['!' + attrs.ngSwitchWhen].push({ transclude: $transclude, element: element });
};
}
});
Expand All @@ -172,10 +172,8 @@ var ngSwitchDefaultDirective = ngDirective({
transclude: 'element',
priority: 800,
require: '^ngSwitch',
compile: function(element, attrs, transclude) {
return function(scope, element, attr, ctrl) {
ctrl.cases['?'] = (ctrl.cases['?'] || []);
ctrl.cases['?'].push({ transclude: transclude, element: element });
};
}
link: function(scope, element, attr, ctrl, $transclude) {
ctrl.cases['?'] = (ctrl.cases['?'] || []);
ctrl.cases['?'].push({ transclude: $transclude, element: element });
}
});
6 changes: 2 additions & 4 deletions src/ngRoute/directive/ngView.js
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,7 @@ function ngViewFactory( $route, $anchorScroll, $compile, $controller,
terminal: true,
priority: 400,
transclude: 'element',
compile: function(element, attr, linker) {
return function(scope, $element, attr) {
link: function(scope, $element, attr, ctrl, $transclude) {
var currentScope,
currentElement,
autoScrollExp = attr.autoscroll,
Expand All @@ -200,7 +199,7 @@ function ngViewFactory( $route, $anchorScroll, $compile, $controller,

if (template) {
var newScope = scope.$new();
linker(newScope, function(clone) {
$transclude(newScope, function(clone) {
clone.html(template);
$animate.enter(clone, null, currentElement || $element, function onNgViewEnter () {
if (angular.isDefined(autoScrollExp)
Expand Down Expand Up @@ -235,7 +234,6 @@ function ngViewFactory( $route, $anchorScroll, $compile, $controller,
cleanupLastView();
}
}
};
}
};
}
203 changes: 201 additions & 2 deletions test/ng/compileSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -3438,6 +3438,113 @@ describe('$compile', function() {
expect(log).toEqual('pre(); post(unicorn!)');
});
});

it('should copy the directive controller to all clones', function() {
var transcludeCtrl, cloneCount = 2;
module(function() {
directive('transclude', valueFn({
transclude: 'content',
controller: function($transclude) {
transcludeCtrl = this;
},
link: function(scope, el, attr, ctrl, $transclude) {
var i;
for (i=0; i<cloneCount; i++) {
$transclude(cloneAttach);
}

function cloneAttach(clone) {
el.append(clone);
}
}
}));
});
inject(function($compile) {
element = $compile('<div transclude><span></span></div>')($rootScope);
var children = element.children(), i;
expect(transcludeCtrl).toBeDefined();

expect(element.data('$transcludeController')).toBe(transcludeCtrl);
for (i=0; i<cloneCount; i++) {
expect(children.eq(i).data('$transcludeController')).toBeUndefined();
}
});
});

it('should provide the $transclude controller local as 5th argument to the pre and post-link function', function() {
var ctrlTransclude, preLinkTransclude, postLinkTransclude;
module(function() {
directive('transclude', valueFn({
transclude: 'content',
controller: function($transclude) {
ctrlTransclude = $transclude;
},
compile: function() {
return {
pre: function(scope, el, attr, ctrl, $transclude) {
preLinkTransclude = $transclude;
},
post: function(scope, el, attr, ctrl, $transclude) {
postLinkTransclude = $transclude;
}
};
}
}));
});
inject(function($compile) {
element = $compile('<div transclude></div>')($rootScope);
expect(ctrlTransclude).toBeDefined();
expect(ctrlTransclude).toBe(preLinkTransclude);
expect(ctrlTransclude).toBe(postLinkTransclude);
});
});

it('should allow an optional scope argument in $transclude', function() {
var capturedChildCtrl;
module(function() {
directive('transclude', valueFn({
transclude: 'content',
link: function(scope, element, attr, ctrl, $transclude) {
$transclude(scope, function(clone) {
element.append(clone);
});
}
}));
});
inject(function($compile) {
element = $compile('<div transclude>{{$id}}</div>')($rootScope);
$rootScope.$apply();
expect(element.text()).toBe($rootScope.$id);
});

});

it('should expose the directive controller to transcluded children', function() {
var capturedChildCtrl;
module(function() {
directive('transclude', valueFn({
transclude: 'content',
controller: function() {
},
link: function(scope, element, attr, ctrl, $transclude) {
$transclude(function(clone) {
element.append(clone);
});
}
}));
directive('child', valueFn({
require: '^transclude',
link: function(scope, element, attr, ctrl) {
capturedChildCtrl = ctrl;
}
}));
});
inject(function($compile) {
element = $compile('<div transclude><div child></div></div>')($rootScope);
expect(capturedChildCtrl).toBeTruthy();
});

});
});


Expand Down Expand Up @@ -3471,7 +3578,6 @@ describe('$compile', function() {
});
});


it('should only allow one element transclusion per element', function() {
module(function() {
directive('first', valueFn({
Expand Down Expand Up @@ -3620,8 +3726,101 @@ describe('$compile', function() {
]);
});
});
});

it('should allow to access $transclude in the same directive', function() {
var _$transclude;
module(function() {
directive('transclude', valueFn({
transclude: 'element',
controller: function($transclude) {
_$transclude = $transclude;
}
}));
});
inject(function($compile) {
element = $compile('<div transclude></div>')($rootScope);
expect(_$transclude).toBeDefined()
});
});

it('should copy the directive controller to all clones', function() {
var transcludeCtrl, cloneCount = 2;
module(function() {
directive('transclude', valueFn({
transclude: 'element',
controller: function() {
transcludeCtrl = this;
},
link: function(scope, el, attr, ctrl, $transclude) {
var i;
for (i=0; i<cloneCount; i++) {
$transclude(cloneAttach);
}

function cloneAttach(clone) {
el.after(clone);
}
}
}));
});
inject(function($compile) {
element = $compile('<div><div transclude></div></div>')($rootScope);
var children = element.children(), i;
for (i=0; i<cloneCount; i++) {
expect(children.eq(i).data('$transcludeController')).toBe(transcludeCtrl);
}
});
});

it('should expose the directive controller to transcluded children', function() {
var capturedTranscludeCtrl;
module(function() {
directive('transclude', valueFn({
transclude: 'element',
controller: function() {
},
link: function(scope, element, attr, ctrl, $transclude) {
$transclude(scope, function(clone) {
element.after(clone);
});
}
}));
directive('child', valueFn({
require: '^transclude',
link: function(scope, element, attr, ctrl) {
capturedTranscludeCtrl = ctrl;
}
}));
});
inject(function($compile) {
element = $compile('<div transclude><div child></div></div>')($rootScope);
expect(capturedTranscludeCtrl).toBeTruthy();
});
});

it('should allow access to $transclude in a templateUrl directive', function() {
var transclude;
module(function() {
directive('template', valueFn({
templateUrl: 'template.html',
replace: true
}));
directive('transclude', valueFn({
transclude: 'content',
controller: function($transclude) {
transclude = $transclude;
}
}));
});
inject(function($compile, $httpBackend) {
$httpBackend.expectGET('template.html').respond('<div transclude></div>');
element = $compile('<div template></div>')($rootScope);
$httpBackend.flush();
expect(transclude).toBeDefined();
});
});

});

it('should safely create transclude comment node and not break with "-->"',
inject(function($rootScope) {
Expand Down
28 changes: 28 additions & 0 deletions test/ng/directive/ngIfSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,34 @@ describe('ngIf', function () {

});

describe('ngIf and transcludes', function() {
it('should allow access to directive controller from children when used in a replace template', function() {
var controller;
module(function($compileProvider) {
var directive = $compileProvider.directive;
directive('template', valueFn({
template: '<div ng-if="true"><span test></span></div>',
replace: true,
controller: function() {
this.flag = true;
}
}));
directive('test', valueFn({
require: '^template',
link: function(scope, el, attr, ctrl) {
controller = ctrl;
}
}));
});
inject(function($compile, $rootScope) {
var element = $compile('<div><div template></div></div>')($rootScope);
$rootScope.$apply();
expect(controller.flag).toBe(true);
dealoc(element);
});
});
});

describe('ngIf animations', function () {
var body, element, $rootElement;

Expand Down
Loading

4 comments on commit 90f8707

@jeffbcross
Copy link
Contributor

Choose a reason for hiding this comment

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

If Mary Poppins were online, she'd be complaining about the length of the title and body lines of your commit message.

@tam4s
Copy link

@tam4s tam4s commented on 90f8707 Nov 20, 2013

Choose a reason for hiding this comment

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

Since upgrading to 1.2.1 I get the following error

TypeError: undefined is not a function
    at Object.ngIfWatchAction [as fn] (http://localhost:8080/ .... )
    at Scope.$digest (http://localhost:8080/ .... )
    at Scope.$apply (http://localhost:8080/ .... )
    at done (http://localhost:8080/ .... )
    at completeRequest (http://localhost:8080/ .... )
    at XMLHttpRequest.xhr.onreadystatechange (http://localhost:8080/ .... )

when the line $transclude(childScope, function (clone) { is executed from the snipped below:

var ngIfDirective = ['$animate', function($animate) {
  return {
    transclude: 'element',
    priority: 600,
    terminal: true,
    restrict: 'A',
    $$tlb: true,
    link: function ($scope, $element, $attr, ctrl, $transclude) {
        var block, childScope;
        $scope.$watch($attr.ngIf, function ngIfWatchAction(value) {

          if (toBoolean(value)) {
            if (!childScope) {
              childScope = $scope.$new();
              $transclude(childScope, function (clone) {
                block = {
                  startNode: clone[0],
                  endNode: clone[clone.length++] = document.createComment(' end ngIf: ' + $attr.ngIf + ' ')
                };
                $animate.enter(clone, $element.parent(), $element);
              });
            }
          } else {

I guess it's related, so I posted here.

(I have a the files 1.10.2/jquery.min.js, angular.min.js, angular-resource.min.js & angular-sanitize.min.js concatenated into a single js.)

@mntnoe
Copy link

@mntnoe mntnoe commented on 90f8707 Nov 25, 2013

Choose a reason for hiding this comment

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

I am getting the same error as tam4s:

Error: $transclude is not a function
    ngIfWatchAction@http://localhost:...

I didn't have the problem until updating from the last release candidate to 1.2.2. The error only appears with about 50 % propability when loading the page.

The error triggered by a ngIf on the top-level element (<form ng-if="state.showForm">) in a custom directive's template. Unfortunately, I haven't been able to isolate the problem yet.

Edit: It seems that wrapping the top-level element makes my particular problem disappear, e.g. <div><form ng-if="state.showForm">...</div>. Also, it's likely that the cause of my problem is that my custom directive has replace: true set. I guess it's bad practice to replace with nothing...

@tam4s
Copy link

@tam4s tam4s commented on 90f8707 Nov 26, 2013

Choose a reason for hiding this comment

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

I can confirm that it's related to ng-if on a top level elements of a directive.

Please sign in to comment.