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

Commit b27c4f5

Browse files
committed
fix($compile): check clashing directives before compilation
Closes angular#3893
1 parent e9a2224 commit b27c4f5

File tree

2 files changed

+74
-24
lines changed

2 files changed

+74
-24
lines changed

Diff for: src/ng/compile.js

+50-21
Original file line numberDiff line numberDiff line change
@@ -770,6 +770,55 @@ function $CompileProvider($provide) {
770770
linkFn,
771771
directiveValue;
772772

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

792-
// skip the check for directives with async templates, we'll check the derived sync directive when
793-
// the template arrives
794841
if (!directive.templateUrl) {
795-
assertNoDuplicate('new/isolated scope', newIsolateScopeDirective, directive, $compileNode);
796842
if (isObject(directiveValue)) {
797843
safeAddClass($compileNode, 'ng-isolate-scope');
798-
newIsolateScopeDirective = directive;
799844
}
800845
safeAddClass($compileNode, 'ng-scope');
801846
}
802847
}
803848

804849
directiveName = directive.name;
805850

806-
if (!directive.templateUrl && directive.controller) {
807-
directiveValue = directive.controller;
808-
controllerDirectives = controllerDirectives || {};
809-
assertNoDuplicate("'" + directiveName + "' controller",
810-
controllerDirectives[directiveName], directive, $compileNode);
811-
controllerDirectives[directiveName] = directive;
812-
}
813-
814851
if (directiveValue = directive.transclude) {
815-
assertNoDuplicate('transclusion', transcludeDirective, directive, $compileNode);
816-
transcludeDirective = directive;
817852
terminalPriority = directive.priority;
818853
if (directiveValue == 'element') {
819854
$template = groupScan(compileNode, attrStart, attrEnd)
@@ -832,9 +867,6 @@ function $CompileProvider($provide) {
832867
}
833868

834869
if (directive.template) {
835-
assertNoDuplicate('template', templateDirective, directive, $compileNode);
836-
templateDirective = directive;
837-
838870
directiveValue = (isFunction(directive.template))
839871
? directive.template($compileNode, templateAttrs)
840872
: directive.template;
@@ -843,6 +875,7 @@ function $CompileProvider($provide) {
843875

844876
if (directive.replace) {
845877
replaceDirective = directive;
878+
846879
$template = jqLite('<div>' +
847880
trim(directiveValue) +
848881
'</div>').contents();
@@ -877,9 +910,6 @@ function $CompileProvider($provide) {
877910
}
878911

879912
if (directive.templateUrl) {
880-
assertNoDuplicate('template', templateDirective, directive, $compileNode);
881-
templateDirective = directive;
882-
883913
if (directive.replace) {
884914
replaceDirective = directive;
885915
}
@@ -904,7 +934,6 @@ function $CompileProvider($provide) {
904934
nodeLinkFn.terminal = true;
905935
terminalPriority = Math.max(terminalPriority, directive.priority);
906936
}
907-
908937
}
909938

910939
nodeLinkFn.scope = newScopeDirective && newScopeDirective.scope;

Diff for: test/ng/compileSpec.js

+24-3
Original file line numberDiff line numberDiff line change
@@ -1519,7 +1519,7 @@ describe('$compile', function() {
15191519
expect(function(){
15201520
$compile('<div class="iscope-a; scope-b"></div>');
15211521
}).toThrowMinErr('$compile', 'multidir', 'Multiple directives [iscopeA, scopeB] asking for new/isolated scope on: ' +
1522-
'<div class="iscope-a; scope-b ng-isolate-scope ng-scope">');
1522+
'<div class="iscope-a; scope-b">');
15231523
})
15241524
);
15251525

@@ -2700,7 +2700,7 @@ describe('$compile', function() {
27002700
});
27012701

27022702

2703-
it('should only allow one transclude per element', function() {
2703+
it('should only allow one content transclusion per element', function() {
27042704
module(function() {
27052705
directive('first', valueFn({
27062706
scope: {},
@@ -2716,7 +2716,28 @@ describe('$compile', function() {
27162716
expect(function() {
27172717
$compile('<div class="first second"></div>');
27182718
}).toThrowMinErr('$compile', 'multidir', 'Multiple directives [first, second] asking for transclusion on: ' +
2719-
'<div class="first second ng-isolate-scope ng-scope">');
2719+
'<div class="first second">');
2720+
});
2721+
});
2722+
2723+
2724+
it('should only allow one element transclusion per element', function() {
2725+
module(function() {
2726+
directive('first', valueFn({
2727+
scope: {},
2728+
restrict: 'CA',
2729+
transclude: 'element'
2730+
}));
2731+
directive('second', valueFn({
2732+
restrict: 'CA',
2733+
transclude: 'element'
2734+
}));
2735+
});
2736+
inject(function($compile) {
2737+
expect(function() {
2738+
$compile('<div class="first second"></div>');
2739+
}).toThrowMinErr('$compile', 'multidir', 'Multiple directives [first, second] asking for transclusion on: ' +
2740+
'<div class="first second">');
27202741
});
27212742
});
27222743

0 commit comments

Comments
 (0)