Skip to content

Commit e5917b4

Browse files
jankucavojtajina
authored andcommitted
fix($compile): check clashing directives before compilation
Closes angular#3893
1 parent b1e604e commit e5917b4

File tree

2 files changed

+74
-24
lines changed

2 files changed

+74
-24
lines changed

src/ng/compile.js

Lines changed: 50 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -773,6 +773,55 @@ function $CompileProvider($provide) {
773773
linkFn,
774774
directiveValue;
775775

776+
for(var i = 0, ii = directives.length; i < ii; i++) {
777+
directive = directives[i];
778+
if (terminalPriority > directive.priority) {
779+
break; // prevent further processing of directives
780+
}
781+
782+
directiveName = directive.name;
783+
784+
if (directiveValue = directive.scope) {
785+
assertNoDuplicate('new/isolated scope', newIsolateScopeDirective, directive, $compileNode);
786+
787+
// skip the check for directives with async templates, we'll check the derived sync directive when
788+
// the template arrives
789+
if (!directive.templateUrl && isObject(directiveValue)) {
790+
newIsolateScopeDirective = directive;
791+
}
792+
}
793+
794+
if (!directive.templateUrl && directive.controller) {
795+
directiveValue = directive.controller;
796+
controllerDirectives = controllerDirectives || {};
797+
assertNoDuplicate("'" + directiveName + "' controller",
798+
controllerDirectives[directiveName], directive, $compileNode);
799+
controllerDirectives[directiveName] = directive;
800+
}
801+
802+
if (directiveValue = directive.transclude) {
803+
terminalPriority = directive.priority;
804+
assertNoDuplicate('transclusion', transcludeDirective, directive, $compileNode);
805+
transcludeDirective = directive;
806+
}
807+
808+
if (directive.template) {
809+
assertNoDuplicate('template', templateDirective, directive, $compileNode);
810+
templateDirective = directive;
811+
}
812+
813+
if (directive.templateUrl) {
814+
assertNoDuplicate('template', templateDirective, directive, $compileNode);
815+
templateDirective = directive;
816+
}
817+
818+
if (directive.terminal) {
819+
terminalPriority = Math.max(terminalPriority, directive.priority);
820+
}
821+
}
822+
823+
terminalPriority = -Number.MAX_VALUE;
824+
776825
// executes all directives on the current element
777826
for(var i = 0, ii = directives.length; i < ii; i++) {
778827
directive = directives[i];
@@ -792,31 +841,17 @@ function $CompileProvider($provide) {
792841
if (directiveValue = directive.scope) {
793842
newScopeDirective = newScopeDirective || directive;
794843

795-
// skip the check for directives with async templates, we'll check the derived sync directive when
796-
// the template arrives
797844
if (!directive.templateUrl) {
798-
assertNoDuplicate('new/isolated scope', newIsolateScopeDirective, directive, $compileNode);
799845
if (isObject(directiveValue)) {
800846
safeAddClass($compileNode, 'ng-isolate-scope');
801-
newIsolateScopeDirective = directive;
802847
}
803848
safeAddClass($compileNode, 'ng-scope');
804849
}
805850
}
806851

807852
directiveName = directive.name;
808853

809-
if (!directive.templateUrl && directive.controller) {
810-
directiveValue = directive.controller;
811-
controllerDirectives = controllerDirectives || {};
812-
assertNoDuplicate("'" + directiveName + "' controller",
813-
controllerDirectives[directiveName], directive, $compileNode);
814-
controllerDirectives[directiveName] = directive;
815-
}
816-
817854
if (directiveValue = directive.transclude) {
818-
assertNoDuplicate('transclusion', transcludeDirective, directive, $compileNode);
819-
transcludeDirective = directive;
820855

821856
if (directiveValue == 'element') {
822857
terminalPriority = directive.priority;
@@ -836,9 +871,6 @@ function $CompileProvider($provide) {
836871
}
837872

838873
if (directive.template) {
839-
assertNoDuplicate('template', templateDirective, directive, $compileNode);
840-
templateDirective = directive;
841-
842874
directiveValue = (isFunction(directive.template))
843875
? directive.template($compileNode, templateAttrs)
844876
: directive.template;
@@ -847,6 +879,7 @@ function $CompileProvider($provide) {
847879

848880
if (directive.replace) {
849881
replaceDirective = directive;
882+
850883
$template = jqLite('<div>' +
851884
trim(directiveValue) +
852885
'</div>').contents();
@@ -881,9 +914,6 @@ function $CompileProvider($provide) {
881914
}
882915

883916
if (directive.templateUrl) {
884-
assertNoDuplicate('template', templateDirective, directive, $compileNode);
885-
templateDirective = directive;
886-
887917
if (directive.replace) {
888918
replaceDirective = directive;
889919
}
@@ -908,7 +938,6 @@ function $CompileProvider($provide) {
908938
nodeLinkFn.terminal = true;
909939
terminalPriority = Math.max(terminalPriority, directive.priority);
910940
}
911-
912941
}
913942

914943
nodeLinkFn.scope = newScopeDirective && newScopeDirective.scope;

test/ng/compileSpec.js

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1533,7 +1533,7 @@ describe('$compile', function() {
15331533
expect(function(){
15341534
$compile('<div class="iscope-a; scope-b"></div>');
15351535
}).toThrowMinErr('$compile', 'multidir', 'Multiple directives [iscopeA, scopeB] asking for new/isolated scope on: ' +
1536-
'<div class="iscope-a; scope-b ng-isolate-scope ng-scope">');
1536+
'<div class="iscope-a; scope-b">');
15371537
})
15381538
);
15391539

@@ -2745,7 +2745,7 @@ describe('$compile', function() {
27452745
});
27462746

27472747

2748-
it('should only allow one transclude per element', function() {
2748+
it('should only allow one content transclusion per element', function() {
27492749
module(function() {
27502750
directive('first', valueFn({
27512751
scope: {},
@@ -2761,7 +2761,28 @@ describe('$compile', function() {
27612761
expect(function() {
27622762
$compile('<div class="first second"></div>');
27632763
}).toThrowMinErr('$compile', 'multidir', 'Multiple directives [first, second] asking for transclusion on: ' +
2764-
'<div class="first second ng-isolate-scope ng-scope">');
2764+
'<div class="first second">');
2765+
});
2766+
});
2767+
2768+
2769+
it('should only allow one element transclusion per element', function() {
2770+
module(function() {
2771+
directive('first', valueFn({
2772+
scope: {},
2773+
restrict: 'CA',
2774+
transclude: 'element'
2775+
}));
2776+
directive('second', valueFn({
2777+
restrict: 'CA',
2778+
transclude: 'element'
2779+
}));
2780+
});
2781+
inject(function($compile) {
2782+
expect(function() {
2783+
$compile('<div class="first second"></div>');
2784+
}).toThrowMinErr('$compile', 'multidir', 'Multiple directives [first, second] asking for transclusion on: ' +
2785+
'<div class="first second">');
27652786
});
27662787
});
27672788

0 commit comments

Comments
 (0)