refactor(compile): Optimizations for the directive 'multi-element'

Avoid cloning elements during a multi-element transclude
Put only one comment that will be sent to the linking function and
remove all the placeholde elements
Cleanup the containsAttr method with an implementation from @petebacondarwin
Fixes on the unit tests to follow angular#2492
Document the transclude type 'multi-element' at the docs
Fix a typo on the method compileNodes
Move the important unit tests from ngRepeatStartSpec to ngRepeatSpec
Add more tests at compile for the new multi-element transclude

Author: lgalfaso

docs/content/guide/directive.ngdoc

@@ -444,7 +444,11 @@ compiler}. The attributes are:
 
     * `true` - transclude the content of the directive.
     * `'element'` - transclude the whole element including any directives defined at lower priority.
-
+    * `'multi-element'` - if the node where this directive is present contains an attribute
+      named `${directiveName}-start` (were `${directiveName}` is the name of the directive) then
+      the entire block of elements is transcluded until a sibling node is found with and attribute
+      named `${directiveName}-end`. If and attribute name `${directiveName}-start` is not found
+      then it behaves like the `'element'` transclude.
 
   * `compile`: This is the compile function described in the section below.
 

src/ng/compile.js

@@ -419,9 +419,10 @@ function $CompileProvider($provide) {
      *        rootElement must be set the jqLite collection of the compile root. This is
      *        needed so that the jqLite collection items can be replaced with widgets.
      * @param {number=} max directive priority
-     * @param {boolean=} if the max priority should only apply to the first element in the list.
-     *        A true value here will make the maxPriority only apply to the first element on the
-     *        list while the other elements on the list will not have a maxPriority set
+     * @param {boolean=} limitMaxPriorityToFirstElement if the max priority should only apply to
+     *        the first element in the list. A true value here will make the maxPriority only apply
+     *        to the first element on the list while the other elements on the list will not have
+     *        a maxPriority set
      * @returns {?function} A composite linking function of all of the matched directives or null.
      */
     function compileNodes(nodeList, transcludeFn, $rootElement, maxPriority, limitMaxPriorityToFirstElement) {
@@ -654,20 +655,20 @@ function $CompileProvider($provide) {
             // We need to compile a clone of the elements, but at the same time we have to be sure that these elements are siblings
             var nestedContent = extractMultiElementTransclude(compileNode, directiveName);
             var cloneContent = jqLite('<div></div>');
-            forEach(nestedContent, function(nestedElement) {
-              cloneContent.append(JQLiteClone(nestedElement));
-            });
-            childTranscludeFn = compile(cloneContent.contents(), transcludeFn, terminalPriority, true);
+            
             $template = jqLite(compileNode);
             $compileNode = templateAttrs.$$element =
                 jqLite(document.createComment(' ' + directiveName + ': ' + templateAttrs[directiveName] + ' '));
             compileNode = $compileNode[0];
-            replaceWith($rootElement, $template, compileNode);
+            replaceWith($rootElement, jqLite($template[0]), compileNode);
+            cloneContent.append($template);
             forEach(nestedContent.splice(1),
               function(toRemove) {
-                replaceWith($rootElement, jqLite(toRemove), document.createComment(' placeholder '));
+                removeElement($rootElement, toRemove);
+                cloneContent.append(toRemove);
               }
             );
+            childTranscludeFn = compile(cloneContent.contents(), transcludeFn, terminalPriority, true);
           } else {
             $template = jqLite(JQLiteClone(compileNode)).contents();
             $compileNode.html(''); // clear contents
@@ -774,32 +775,16 @@ function $CompileProvider($provide) {
 
 
       function containsAttr(node, attributeName) {
-        var nodeType = node.nodeType,
-          result = false;
-
-        switch(nodeType) {
-          case 1:
-          // iterate over the attributes
-          for (var attr, name, nName, ngAttrName, nAttrs = node.attributes,
-                   j = 0, jj = nAttrs && nAttrs.length; j < jj; j++) {
-            attr = nAttrs[j];
-            if (attr.specified) {
-              name = attr.name;
-              // support ngAttr attribute binding
-              ngAttrName = directiveNormalize(name);
-              if (NG_ATTR_BINDING.test(ngAttrName)) {
-                name = ngAttrName.substr(6).toLowerCase();
-              }
-              nName = directiveNormalize(name.toLowerCase());
-              if (nName == attributeName) {
-                result = true;
-                break;
-              }
+        var attr, attrs = node.attributes, length = attrs && attrs.length;
+        if ( length ) {
+          for (var j = 0; j < length; j++) {
+            attr = attrs[j];
+            if (attr.specified && directiveNormalize(attr.name) == attributeName) {
+              return true;
             }
           }
-          break;
         }
-        return result;
+        return false;
       }
 
 
@@ -1224,6 +1209,20 @@ function $CompileProvider($provide) {
       newNode[jqLite.expando] = oldNode[jqLite.expando];
       $element[0] = newNode;
     }
+
+
+    function removeElement($rootElement, element) {
+      var i, ii;
+
+      if ($rootElement) {
+        for(i = 0, ii = $rootElement.length; i < ii; i++) {
+          if ($rootElement[i] == element) {
+            $rootElement.splice(i, 1);
+            break;
+          }
+        }
+      }
+    }
   }];
 }
 

test/ng/compileSpec.js

@@ -381,6 +381,47 @@ describe('$compile', function() {
       })
     });
 
+    it("should complain when the multi-element end tag can't be found among one of the following siblings", inject(function($compile) {
+      forEach([
+        // no content, no end tag
+        '<div ng-repeat="item in items" ng-repeat-start></div>',
+
+        // content, no end tag
+        '<div ng-repeat="item in items" ng-repeat-start></div>' +
+        '<span>{{item.text}}></span>' +
+        '<span>{{item.done}}</span>',
+
+        // content, end tag too deep
+        '<div ng-repeat="item in items" ng-repeat-start></div>' +
+        '<div>' +
+          '<span>{{item.text}}></span>' +
+          '<span>{{item.done}}</span>' +
+          '<span ng-repeat-end></span>' +
+        '</div>',
+
+        // content, end tag too high
+        '<div>' +
+          '<div ng-repeat="item in items" ng-repeat-start></div>' +
+          '<span>{{item.text}}></span>' +
+          '<span>{{item.done}}</span>' +
+        '</div>' +
+        '<div ng-repeat-end></div>',
+
+        ], function(template) {
+          expect(function() {
+            $compile('<div>' + template + '</div>');
+          }).toThrow("Unmatched ngRepeatStart.");
+        });
+    }));
+
+
+    it("should complain when there is an end attribute at the start of a multi-element directive", inject(function($compile) {
+      expect(function() {
+        $compile('<div><li ng-repeat-end ng-repeat="i in items"></li><li></li></div>');
+      }).toThrow("Unexpected ngRepeatEnd.");
+    }));
+
+
     describe('compiler control', function() {
       describe('priority', function() {
         it('should honor priority', inject(function($compile, $rootScope, log){
@@ -2345,6 +2386,35 @@ describe('$compile', function() {
     });
 
 
+    it('should compile get templateFn on multi-element', function() {
+      module(function() {
+        directive('trans', function(log) {
+          return {
+            transclude: 'multi-element',
+            priority: 2,
+            controller: function($transclude) { this.$transclude = $transclude; },
+            compile: function(element, attrs, template) {
+              log('compile: ' + angular.mock.dump(element));
+              return function(scope, element, attrs, ctrl) {
+                log('link');
+                var cursor = element;
+                template(scope.$new(), function(clone) {cursor.after(clone); cursor = clone.eq(-1);});
+                ctrl.$transclude(function(clone) {cursor.after(clone)});
+              };
+            }
+          }
+        });
+      });
+      inject(function(log, $rootScope, $compile) {
+        element = $compile('<div><div high-log trans="text" trans-start log>{{$parent.$id}}-{{$id}};</div><div trans-end>{{$parent.$id}}-{{$id}};</div></div>')
+            ($rootScope);
+        $rootScope.$apply();
+        expect(log).toEqual('compile: <!-- trans: text -->; HIGH; link; LOG; LOG');
+        expect(element.text()).toEqual('001-002;001-002;001-003;001-003;');
+      });
+    });
+
+
     it('should support transclude directive', function() {
       module(function() {
         directive('trans', function() {
@@ -2485,6 +2555,30 @@ describe('$compile', function() {
     });
 
 
+    it('should support transcluded multi-element on root content', function() {
+      var comment;
+      module(function() {
+        directive('transclude', valueFn({
+          transclude: 'multi-element',
+          compile: function(element, attr, linker) {
+            return function(scope, element, attr) {
+              comment = element;
+            };
+          }
+        }));
+      });
+      inject(function($compile, $rootScope) {
+        var element = jqLite('<div>before<div transclude transclude-begin></div><div transclude-end></div>after</div>').contents();
+        expect(element.length).toEqual(4);
+        expect(nodeName_(element[1])).toBe('DIV');
+        $compile(element)($rootScope);
+        expect(nodeName_(element[1])).toBe('#comment');
+        expect(nodeName_(comment)).toBe('#comment');
+        expect(nodeName_(element[2])).toBe('DIV');
+      });
+    });
+
+
     it('should safely create transclude comment node and not break with "-->"',
         inject(function($rootScope) {
       // see: https://github.com/angular/angular.js/issues/1740