JS DOM manipulation

user_3724563 0
Viewed 58

I'm experiencing an interesting bug where elt is undefined for some calls to switchToReady. It seems like the function inside setTimeout is passing the same DOM node twice. 

function switchToReady(elt) {
    elt.setAttribute('transform', 'translate(17, 0)');
    elt.classList.remove('compiling');
}

const compilingElts = document.getElementsByClassName('compiling');
for (let i = 0; i < compilingElts.length; i++) {
    const randTime = Math.round(Math.random() * (2000 - 500)) + 500;
    setTimeout(() => {
        switchToReady(compilingElts[i]);
    }, randTime);
}
asynchronous dom html javascript
3 Answers

getElementsByClassName returns a live collection, which means that if the class of an element in the collection changes while you're iterating over it, or if you add another element with that class to the DOM, then the index you're on (eg, if i is 2) may no longer refer to the old element there - it may refer to the next item in the collection, or the previous item, or it may even be undefined. The behavior is pretty unintuitive, so I would suggest using querySelectorAll instead, which returns a static NodeList, which won't change while you're iterating over it.

const compilingElts = document.querySelectorAll('.compiling');

Other benefits of querySelectorAll:

  • The selector string it accepts as an argument can be very flexible - it's not limited to just classes

  • In newer browsers, you can call forEach directly on a NodeList, thus removing the need for manual iteration and keeping track of indicies:

    compilingElts.forEach((elm) => {
  const randTime = Math.round(Math.random() * (2000 - 500)) + 500;
  setTimeout(() => {
      switchToReady(elm);
  }, randTime);
});

Array methods are a lot nicer to work with than for loops in many cases. One way to achieve similar functionality on older browsers, using an HTMLCollection generated from getElementsByClassName is to use Array.prototype.forEach:

Array.prototype.forEach.call(
  compilingElts,
  (elm) => {
    // do stuff with elm
  }
);

(a shortcut is to use [].forEach.call instead, which will accomplish the same thing with less code, but referencing Array.prototype is a bit clearer IMO)

user_95152070

getElementsByClassName returns a live list, which, as you have noticed, means that if you remove the class from an element, the list will change (its size).

You could use document.querySelectorAll('.compiling') instead, which returns a list that is not live.

user_2181960

Ah dumb mistake. Realized that by removing the class, it is changing the contents and therefore the index of the referenced compilingElts object.

user_37245630
Related