CODEWITHSUNDEEP
javascript
playedandmissed
playedandmissed

Reputation: 35

Vanilla JS - Looping through a list and inserting an element causes infinite loop error

I have a nav menu and want to add a decorative element between each item. Outside of the loop my code seems to work when I target a specific list item with [0], but when used inside the loop using [i] the page never finishes loading.

When i move to JSbin to test, I am getting infinite loop errors.

HTML: 

<ul id="menu-mobile-navigation">
  <li>One</li>
  <li>Two</li>
  <li>Three</li>
  <li>Four</li>
  <li>Etc</li>
</ul>

JS:

var mobileNav = document.getElementById('menu-mobile-navigation');
var mobileNavChildren = mobileNav.children;

// var decoration = document.createElement('div');
// decoration.classList.add( 'decoration' );
// mobileNav.insertBefore( decoration, mobileNavChildren[0].nextSibling ); // works outside loop


for (var i = 0; i < mobileNavChildren.length; i++) {

  var decoration = document.createElement('div');
  decoration.classList.add( 'decoration' );
  mobileNav.insertBefore( decoration, mobileNavChildren[i].nextSibling ); // page never loads, infinite loop error in JSbin

}

Upvotes: 0

Views: 1219

Answers (1)

CertainPerformance
CertainPerformance

Reputation: 370979

The .children of an element returns a live HTMLCollectio, which will mutate even as you iterate over it if elements get added to it in the middle of looping:

var mobileNav = document.getElementById('menu-mobile-navigation');
var mobileNavChildren = mobileNav.children;
console.log(mobileNavChildren.length);
mobileNav.appendChild(document.createElement('div'));
console.log(mobileNavChildren.length);
<ul id="menu-mobile-navigation">
  <li>One</li>
  <li>Two</li>
  <li>Three</li>
  <li>Four</li>
  <li>Etc</li>
</ul>

You want a static collection instead - consider using querySelectorAll, or spreading into an array:

const mobileNav = document.getElementById('menu-mobile-navigation');
const mobileNavChildren = mobileNav.querySelectorAll('li');

for (var i = 0; i < mobileNavChildren.length; i++) {
  var decoration = document.createElement('div');
  decoration.classList.add( 'decoration' );
  mobileNav.insertBefore( decoration, mobileNavChildren[i].nextSibling );

}
<ul id="menu-mobile-navigation">
  <li>One</li>
  <li>Two</li>
  <li>Three</li>
  <li>Four</li>
  <li>Etc</li>
</ul>

Though, also note that a <div> should not be a child of a <ul> - only list elements should be children of a <ul>.

Upvotes: 4

Related Questions