CODEWITHSUNDEEP
javascripthtmlasynchronousdom
Shruggie
Shruggie

Reputation: 938

JS DOM manipulation

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

Upvotes: 3

Views: 71

Answers (3)

CertainPerformance
CertainPerformance

Reputation: 370709

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)

Upvotes: 4

Felix Kling
Felix Kling

Reputation: 816394

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.

Upvotes: 3

Shruggie
Shruggie

Reputation: 938

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

Upvotes: 1

Related Questions