I am writing a function for a library that takes an object of elements and removes it from the DOM. I have this working fine but was wondering if there is a way to do a single for loop? I found that NodeLists and HTMLCollections can't use the same for loop so I build an array then remove the elements with the same loop.

   _remove = function(elem) {
      if(!elem) { return false; }
      // getElementById
      if(!elem.length) {
        if(elem.parentNode) {
          elem.parentNode.removeChild(elem);
        }
      // querySelectorAll, jquery, getElementsByClassName, getElementsByTagName
      } else {
        var elems = [];
        for(var j = 0; j<elem.length; j++) {
          if(elem[j]) {
            elems.push(elem[j]);
          }
        }
        for(var i=0; i<elems.length; i++) {
          if(elems[i].parentNode) {
            elems[i].parentNode.removeChild(elems[i]);
          }
        }
        return false;
      }
    }

called by:

_remove(document.getElementById('someId'));
_remove(document.getElementsByClassName('someClass'));
_remove(document.getElementsByTagName('tag'));
_remove($('#someId));
_remove($('.someClass));
_remove(document.querySelectorAll('someID or Class or Tag));

I am writing a function for a library that takes an object of elements and removes it from the DOM. I have this working fine but was wondering if there is a way to do a single for loop? I found that NodeLists and HTMLCollections can't use the same for loop so I build an array then remove the elements with the same loop.

   _remove = function(elem) {
      if(!elem) { return false; }
      // getElementById
      if(!elem.length) {
        if(elem.parentNode) {
          elem.parentNode.removeChild(elem);
        }
      // querySelectorAll, jquery, getElementsByClassName, getElementsByTagName
      } else {
        var elems = [];
        for(var j = 0; j<elem.length; j++) {
          if(elem[j]) {
            elems.push(elem[j]);
          }
        }
        for(var i=0; i<elems.length; i++) {
          if(elems[i].parentNode) {
            elems[i].parentNode.removeChild(elems[i]);
          }
        }
        return false;
      }
    }

called by:

_remove(document.getElementById('someId'));
_remove(document.getElementsByClassName('someClass'));
_remove(document.getElementsByTagName('tag'));
_remove($('#someId));
_remove($('.someClass));
_remove(document.querySelectorAll('someID or Class or Tag));

• javascript

Share Improve this question Follow edited Oct 31, 2014 at 1:49 lapin 2,14833 gold badges2222 silver badges3232 bronze badges asked Oct 31, 2014 at 0:55 user1572796user1572796 1,05722 gold badges2121 silver badges4848 bronze badges 3

• Iterate in reverse, and you'll be covered for both types of lists. – user1106925 Commented Oct 31, 2014 at 1:01
• can you give an example? I used both types of loops because while NodeLists were working with the 2nd loop by itself HTMLCollections were removing items reducing i as the loop was running...so half the items in the collection were not getting removed – user1572796 Commented Oct 31, 2014 at 1:02
• Start with i as the last index instead of the first, and decrement instead of incrementing. – user1106925 Commented Oct 31, 2014 at 1:03

Add a ment  | 

2 Answers 2

Sorted by: Reset to default 5

Some of these functions return a live NodeList, which means that when you change the DOM, the collection changes immediately to reflect this. That's why a normal for loop doesn't work: when you remove elem[0], all the other entries in the collection move down. Then when you increment the index, you skip over the new elem[0].

The easiest way to work around this is to loop from the high end down, instead of from 0 up.

for (var j = elem.length-1; j >= 0; j--) {
    if (elem[j].parentNode) {
        elem[j].parentNode.removeChild(elem[j]);
    }
}

I would remend a bit more robust implementation for these reasons:

1. If it's a jQuery object passed in, then you should call the jQuery .remove() because jQuery may leak memory for data and event listeners if you don't let jQuery remove DOM elements that jQuery operations such as .data() or .on() have been used with.

2. It's not enough to just do if (elem.length) because it could still be a nodeList or an Array with a .length of 0. I changed it to test to see if that property actually exists. FYI, if you want a more robust test to see if it's an actual single DOM node, there's a robust function to do that here, though I'm not sure that's needed here.

3. To protect against a dynamic nodeList changing out from under you or children and parents both being in the list (you don't want to remove parent before child), it is best to iterate the nodeList backwards from back to front (you don't have to make a copy).

4. If this is just a normal array of DOM elements that are not necessarily in document order and there are parent/child elements both in the list and out of order or anyone passes you an array with any old DOM elements in it, you could end up getting an exception on a .removeChild() operation so it is best to just catch any exceptions that might happen there so the operation can always plete through the whole list, even if there is a bad element in the list.

5. Your code did not have a consistent return value. It had one path that wasn't returning anything. I changed it to return false if nothing elem was falsey and true if it was processed. You can obviously modify this as desired, but if you're going to have a meaningful return value, you should have a return value for all code paths (you didn't have one if it was a single DOM node).

Remended code:

// _remove(elem) - remove one or more DOM nodes from their DOM hierarchy
// elem can be an Array of DOM nodes or a pseudo-array like a nodeList 
// or elem can be a single DOM element

function _remove(elem) {
    if (!elem) {
        return false;
    } else if (typeof elem.jquery === "string" && typeof elem.remove === "function") {
        // if a jQuery object, it should be removed with jQuery 
        // so jQuery data and event listener stuff will get cleaned up
        // there are more prehensive ways to check for a jQuery object,
        // but those are probably not needed here
        elem.remove();
    } else if (elem.nodeType) {
        // single DOM node
        // could also do more prehensive test to see if it's really a DOM node, 
        // but probably not needed
        if (elem.parentNode) {
            elem.parentNode.removeChild(elem);
        }
    } else if (typeof elem.length === "number") {
        // array or pseudo-array here
        // querySelectorAll, getElementsByClassName, getElementsByTagName or 
        // an array of DOM elements assembled by any other code
        // iterate backwards so if it is a dynamic nodeList, then children will be
        // be removed before parents and any nodeList changes will happen after our
        // iteration point
        for (var i = elem.length - 1; i >= 0; i--) {
            // catch errors in case anything has already been deleted
            // from a prior parent delete so we don't abort early
            try {
                elem[i].parentNode.removeChild(elem[i]);
            } catch(e) { }
        }
    } else {
        // don't know what was passed here - not something we were expecting
        return false;
    }
    return true;
}