Removing elements from dom using queryselectorall, jquery, getelementsbyid, getelementsbyclassname

Question

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));

Answer 1

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]);
    }
}

Answer 2

I would recommend 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 complete 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).

Recommended 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 comprehensive 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 comprehensive 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;
}