Clone input nodes when inserting over a set

This patch increases parity with jQuery, specifically in cases where
manipulation methods are called with existing nodes on sets containing
more than one element. In such cases, the provided node should be cloned
prior to insertion (excepting the final element in the set, where the
original node should be inserted).

lib/api/manipulation.js

@@ -5,40 +5,44 @@ var _ = require('lodash'),
     evaluate = parse.evaluate,
     utils = require('../utils'),
     domEach = utils.domEach,
+    cloneDom = utils.cloneDom,
     slice = Array.prototype.slice;
 
 // Create an array of nodes, recursing into arrays and parsing strings if
 // necessary
-exports._makeDomArray = function makeDomArray(elem) {
+exports._makeDomArray = function makeDomArray(elem, clone) {
   if (elem == null) {
     return [];
   } else if (elem.cheerio) {
-    return elem.get();
+    return clone ? cloneDom(elem.get(), elem.options) : elem.get();
   } else if (Array.isArray(elem)) {
-    return _.flatten(elem.map(makeDomArray, this));
+    return _.flatten(elem.map(function(el) {
+      return this._makeDomArray(el, clone);
+    }, this));
   } else if (typeof elem === 'string') {
     return evaluate(elem, this.options);
   } else {
-    return [elem];
+    return clone ? cloneDom([elem]) : [elem];
   }
 };
 
 var _insert = function(concatenator) {
   return function() {
-    var self = this,
-        elems = slice.call(arguments),
-        dom = this._makeDomArray(elems);
+    var elems = slice.call(arguments),
+        lastIdx = this.length - 1;
 
-    if (typeof elems[0] === 'function') {
-      return domEach(this, function(i, el) {
-        dom = self._makeDomArray(elems[0].call(el, i, $.html(el.children)));
-        concatenator(dom, el.children, el);
-      });
-    } else {
-      return domEach(this, function(i, el) {
-        concatenator(dom, el.children, el);
-      });
-    }
+    return domEach(this, function(i, el) {
+      var dom, domSrc;
+
+      if (typeof elems[0] === 'function') {
+        domSrc = elems[0].call(el, i, $.html(el.children));
+      } else {
+        domSrc = elems;
+      }
+
+      dom = this._makeDomArray(domSrc, i < lastIdx);
+      concatenator(dom, el.children, el);
+    });
   };
 };
 
@@ -108,8 +112,7 @@ exports.prepend = _insert(function(dom, children, parent) {
 
 exports.after = function() {
   var elems = slice.call(arguments),
-      dom = this._makeDomArray(elems),
-      self = this;
+      lastIdx = this.length - 1;
 
   domEach(this, function(i, el) {
     var parent = el.parent || el.root;
@@ -118,14 +121,18 @@ exports.after = function() {
     }
 
     var siblings = parent.children,
-        index = siblings.indexOf(el);
+        index = siblings.indexOf(el),
+        domSrc, dom;
 
     // If not found, move on
     if (index < 0) return;
 
     if (typeof elems[0] === 'function') {
-      dom = self._makeDomArray(elems[0].call(el, i, $.html(el.children)));
+      domSrc = elems[0].call(el, i, $.html(el.children));
+    } else {
+      domSrc = elems;
     }
+    dom = this._makeDomArray(domSrc, i < lastIdx);
 
     // Add element after `this` element
     uniqueSplice(siblings, index + 1, 0, dom, parent);
@@ -136,8 +143,7 @@ exports.after = function() {
 
 exports.before = function() {
   var elems = slice.call(arguments),
-      dom = this._makeDomArray(elems),
-      self = this;
+      lastIdx = this.length - 1;
 
   domEach(this, function(i, el) {
     var parent = el.parent || el.root;
@@ -146,15 +152,20 @@ exports.before = function() {
     }
 
     var siblings = parent.children,
-        index = siblings.indexOf(el);
+        index = siblings.indexOf(el),
+        domSrc, dom;
 
     // If not found, move on
     if (index < 0) return;
 
     if (typeof elems[0] === 'function') {
-      dom = self._makeDomArray(elems[0].call(el, i, $.html(el.children)));
+      domSrc = elems[0].call(el, i, $.html(el.children));
+    } else {
+      domSrc = elems;
     }
 
+    dom = this._makeDomArray(domSrc, i < lastIdx);
+
     // Add element before `el` element
     uniqueSplice(siblings, index, 0, dom, parent);
   });
@@ -296,7 +307,5 @@ exports.text = function(str) {
 };
 
 exports.clone = function() {
-  // Turn it into HTML, then recreate it,
-  // Seems to be the easiest way to reconnect everything correctly
-  return this._make($.html(this, this.options));
+  return this._make(cloneDom(this.get(), this.options));
 };

lib/utils.js

@@ -1,3 +1,6 @@
+var parse = require('./parse'),
+    render = require('dom-serializer');
+
 /**
  * HTML Tags
  */
@@ -46,6 +49,17 @@ exports.cssCase = function(str) {
 
 exports.domEach = function(cheerio, fn) {
   var i = 0, len = cheerio.length;
-  while (i < len && fn(i, cheerio[i]) !== false) ++i;
+  while (i < len && fn.call(cheerio, i, cheerio[i]) !== false) ++i;
   return cheerio;
 };
+
+/**
+ * Create a deep copy of the given DOM structure by first rendering it to a
+ * string and then parsing the resultant markup.
+ *
+ * @argument {Object} dom - The htmlparser2-compliant DOM structure
+ * @argument {Object} options - The parsing/rendering options
+ */
+exports.cloneDom = function(dom, options) {
+  return parse(render(dom, options), options).children;
+};

test/api/manipulation.js

@@ -70,6 +70,19 @@ describe('$(...)', function() {
       expect($('.pear').prev()[0]).to.be($('.apple')[0]);
     });
 
+    it('(existing Node) : should clone all but the last occurrence', function() {
+      var $originalApple = $('.apple');
+      var $apples;
+
+      $('.orange, .pear').append($originalApple);
+
+      $apples = $('.apple');
+      expect($apples).to.have.length(2);
+      expect($apples.eq(0).parent()[0]).to.be($('.orange')[0]);
+      expect($apples.eq(1).parent()[0]).to.be($('.pear')[0]);
+      expect($apples[1]).to.be($originalApple[0]);
+    });
+
     it('(elem) : should NOP if removed', function() {
       var $apple = $('.apple');
 
@@ -229,6 +242,19 @@ describe('$(...)', function() {
       expect($('.pear').prev()[0]).to.be($('.apple')[0]);
     });
 
+    it('(existing Node) : should clone all but the last occurrence', function() {
+      var $originalApple = $('.apple');
+      var $apples;
+
+      $('.orange, .pear').prepend($originalApple);
+
+      $apples = $('.apple');
+      expect($apples).to.have.length(2);
+      expect($apples.eq(0).parent()[0]).to.be($('.orange')[0]);
+      expect($apples.eq(1).parent()[0]).to.be($('.pear')[0]);
+      expect($apples[1]).to.be($originalApple[0]);
+    });
+
     it('(elem) : should handle if removed', function() {
       var $apple = $('.apple');
 
@@ -387,6 +413,19 @@ describe('$(...)', function() {
       expect($('.pear').prev()[0]).to.be($('.apple')[0]);
     });
 
+    it('(existing Node) : should clone all but the last occurrence', function() {
+      var $originalApple = $('.apple');
+      $('.orange, .pear').after($originalApple);
+
+      expect($('.apple')).to.have.length(2);
+      expect($('.apple').eq(0).prev()[0]).to.be($('.orange')[0]);
+      expect($('.apple').eq(0).next()[0]).to.be($('.pear')[0]);
+      expect($('.apple').eq(1).prev()[0]).to.be($('.pear')[0]);
+      expect($('.apple').eq(1).next()).to.have.length(0);
+      expect($('.apple')[0]).to.not.eql($originalApple[0]);
+      expect($('.apple')[1]).to.eql($originalApple[0]);
+    });
+
     it('(elem) : should handle if removed', function() {
       var $apple = $('.apple');
       var $plum = $('<li class="plum">Plum</li>');
@@ -510,6 +549,19 @@ describe('$(...)', function() {
       expect($('.pear').prev()[0]).to.be($('.apple')[0]);
     });
 
+    it('(existing Node) : should clone all but the last occurrence', function() {
+      var $originalPear = $('.pear');
+      $('.apple, .orange').before($originalPear);
+
+      expect($('.pear')).to.have.length(2);
+      expect($('.pear').eq(0).prev()).to.have.length(0);
+      expect($('.pear').eq(0).next()[0]).to.be($('.apple')[0]);
+      expect($('.pear').eq(1).prev()[0]).to.be($('.apple')[0]);
+      expect($('.pear').eq(1).next()[0]).to.be($('.orange')[0]);
+      expect($('.pear')[0]).to.not.eql($originalPear[0]);
+      expect($('.pear')[1]).to.eql($originalPear[0]);
+    });
+
     it('(elem) : should handle if removed', function() {
       var $apple = $('.apple');
       var $plum = $('<li class="plum">Plum</li>');