fix($compile): check clashing directives before compilation

Closes angular#3893
vojtajina · Oct 11, 2013 · e5917b4 · e5917b4
1 parent b1e604e
commit e5917b4
Show file tree

Hide file tree

Showing 2 changed files with 74 additions and 24 deletions.
diff --git a/src/ng/compile.js b/src/ng/compile.js
@@ -773,6 +773,55 @@ function $CompileProvider($provide) {
           linkFn,
           directiveValue;
 
+      for(var i = 0, ii = directives.length; i < ii; i++) {
+        directive = directives[i];
+        if (terminalPriority > directive.priority) {
+          break; // prevent further processing of directives
+        }
+
+        directiveName = directive.name;
+
+        if (directiveValue = directive.scope) {
+          assertNoDuplicate('new/isolated scope', newIsolateScopeDirective, directive, $compileNode);
+
+          // skip the check for directives with async templates, we'll check the derived sync directive when
+          // the template arrives
+          if (!directive.templateUrl && isObject(directiveValue)) {
+            newIsolateScopeDirective = directive;
+          }
+        }
+
+        if (!directive.templateUrl && directive.controller) {
+          directiveValue = directive.controller;
+          controllerDirectives = controllerDirectives || {};
+          assertNoDuplicate("'" + directiveName + "' controller",
+              controllerDirectives[directiveName], directive, $compileNode);
+          controllerDirectives[directiveName] = directive;
+        }
+
+        if (directiveValue = directive.transclude) {
+          terminalPriority = directive.priority;
+          assertNoDuplicate('transclusion', transcludeDirective, directive, $compileNode);
+          transcludeDirective = directive;
+        }
+
+        if (directive.template) {
+          assertNoDuplicate('template', templateDirective, directive, $compileNode);
+          templateDirective = directive;
+        }
+
+        if (directive.templateUrl) {
+          assertNoDuplicate('template', templateDirective, directive, $compileNode);
+          templateDirective = directive;
+        }
+
+        if (directive.terminal) {
+          terminalPriority = Math.max(terminalPriority, directive.priority);
+        }
+      }
+
+      terminalPriority = -Number.MAX_VALUE;
+
       // executes all directives on the current element
       for(var i = 0, ii = directives.length; i < ii; i++) {
         directive = directives[i];
@@ -792,31 +841,17 @@ function $CompileProvider($provide) {
         if (directiveValue = directive.scope) {
           newScopeDirective = newScopeDirective || directive;
 
-          // skip the check for directives with async templates, we'll check the derived sync directive when
-          // the template arrives
           if (!directive.templateUrl) {
-            assertNoDuplicate('new/isolated scope', newIsolateScopeDirective, directive, $compileNode);
             if (isObject(directiveValue)) {
               safeAddClass($compileNode, 'ng-isolate-scope');
-              newIsolateScopeDirective = directive;
             }
             safeAddClass($compileNode, 'ng-scope');
           }
         }
 
         directiveName = directive.name;
 
-        if (!directive.templateUrl && directive.controller) {
-          directiveValue = directive.controller;
-          controllerDirectives = controllerDirectives || {};
-          assertNoDuplicate("'" + directiveName + "' controller",
-              controllerDirectives[directiveName], directive, $compileNode);
-          controllerDirectives[directiveName] = directive;
-        }
-
         if (directiveValue = directive.transclude) {
-          assertNoDuplicate('transclusion', transcludeDirective, directive, $compileNode);
-          transcludeDirective = directive;
 
           if (directiveValue == 'element') {
             terminalPriority = directive.priority;
@@ -836,9 +871,6 @@ function $CompileProvider($provide) {
         }
 
         if (directive.template) {
-          assertNoDuplicate('template', templateDirective, directive, $compileNode);
-          templateDirective = directive;
-
           directiveValue = (isFunction(directive.template))
               ? directive.template($compileNode, templateAttrs)
               : directive.template;
@@ -847,6 +879,7 @@ function $CompileProvider($provide) {
 
           if (directive.replace) {
             replaceDirective = directive;
+
             $template = jqLite('<div>' +
                                  trim(directiveValue) +
                                '</div>').contents();
@@ -881,9 +914,6 @@ function $CompileProvider($provide) {
         }
 
         if (directive.templateUrl) {
-          assertNoDuplicate('template', templateDirective, directive, $compileNode);
-          templateDirective = directive;
-
           if (directive.replace) {
             replaceDirective = directive;
           }
@@ -908,7 +938,6 @@ function $CompileProvider($provide) {
           nodeLinkFn.terminal = true;
           terminalPriority = Math.max(terminalPriority, directive.priority);
         }
-
       }
 
       nodeLinkFn.scope = newScopeDirective && newScopeDirective.scope;

diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js
@@ -1533,7 +1533,7 @@ describe('$compile', function() {
             expect(function(){
               $compile('<div class="iscope-a; scope-b"></div>');
             }).toThrowMinErr('$compile', 'multidir', 'Multiple directives [iscopeA, scopeB] asking for new/isolated scope on: ' +
-                '<div class="iscope-a; scope-b ng-isolate-scope ng-scope">');
+                '<div class="iscope-a; scope-b">');
           })
         );
 
@@ -2745,7 +2745,7 @@ describe('$compile', function() {
     });
 
 
-    it('should only allow one transclude per element', function() {
+    it('should only allow one content transclusion per element', function() {
       module(function() {
         directive('first', valueFn({
           scope: {},
@@ -2761,7 +2761,28 @@ describe('$compile', function() {
         expect(function() {
           $compile('<div class="first second"></div>');
         }).toThrowMinErr('$compile', 'multidir', 'Multiple directives [first, second] asking for transclusion on: ' +
-            '<div class="first second ng-isolate-scope ng-scope">');
+            '<div class="first second">');
+      });
+    });
+
+
+    it('should only allow one element transclusion per element', function() {
+      module(function() {
+        directive('first', valueFn({
+          scope: {},
+          restrict: 'CA',
+          transclude: 'element'
+        }));
+        directive('second', valueFn({
+          restrict: 'CA',
+          transclude: 'element'
+        }));
+      });
+      inject(function($compile) {
+        expect(function() {
+          $compile('<div class="first second"></div>');
+        }).toThrowMinErr('$compile', 'multidir', 'Multiple directives [first, second] asking for transclusion on: ' +
+            '<div class="first second">');
       });
     });