Welcome to the Treehouse Community

Want to collaborate on code errors? Have bugs you need feedback on? Looking for an extra set of eyes on your latest project? Get support with fellow developers, designers, and programmers of all backgrounds and skill levels here with the Treehouse Community! While you're at it, check out some resources Treehouse students have shared here.

Looking to learn something new?

Treehouse offers a seven day free trial for new students. Get access to thousands of hours of content and join thousands of Treehouse students and alumni in the community today.

Start your free trial

JavaScript AJAX Basics Programming AJAX Processing JSON Data

Could anyone have a look and comment on my refactoring to avoid innerHTML

I was reading on mdn that using .innerHTML is frowned upon so I wanted to refactor this exercise to avoid using it. I would appreciate some feedback if I followed best practices. I also made sure to use const instead of var. Thanks!

const xhr = new XMLHttpRequest();

xhr.onreadystatechange = function () {
  if (xhr.readyState === 4) {
    const employees = JSON.parse(xhr.responseText);
    const list = document.createElement('ul');

    list.className = "bulleted";
    document.getElementById('employeeList').appendChild(list);

    for (let i = 0; i < employees.length; i++) {
      const listItem = document.createElement('li');
      if (employees[i].inoffice === true) {
        listItem.className = "in";
      } else {
         listItem.className = "out";
      }  
        listItem.innerText = employees[i].name
        list.appendChild(listItem);
    }
  }
};

xhr.open('GET', 'data/employees.json');
xhr.send();

I like this approach better!

2 Answers

Steven Parker
Steven Parker
229,670 Points

Perhaps "frowned upon" isn't entirely accurate. MDN says:

Setting the value of innerHTML lets you easily replace the existing contents of an element with new content.

But for security reasons, you might want to avoid it if your intention is to insert user-supplied text onto the page.

And since you're not intending to insert any markup anyway, using innerText instead is a good design choice. :+1:

Thank you for having a look and for your feedback!

I was referring to this warning from mdn

Warning: If your project is one that will undergo any form of security review, using innerHTML most likely will result in your code being rejected. For example, if you use innerHTML in a browser extension and submit the extension to addons.mozilla.org, it will not pass the automated review process.

Steven Parker
Steven Parker
229,670 Points

Of course, an automated review process isn't intelligent enough to discern a potential issue from a non-issue based on the data source. But if you're not submitting a mozilla add-on, or working where rigid coding standards prohibit it, using "innerHTML" appropriately should be fine.

But you're right that a "best practice" would be to use "innerText" instead anytime the content doesn't have to accommodate markup.

Piotr Manczak
seal-mask
.a{fill-rule:evenodd;}techdegree seal-36
Piotr Manczak
Front End Web Development Techdegree Graduate 28,940 Points

I accidentally created something similar:

const xhr = new XMLHttpRequest();
xhr.onreadystatechange = function() {
  if ( xhr.readyState === 4 ) {
    let employees = JSON.parse(xhr.responseText);
    let list = document.getElementById('employeeList');
    let ul = document.createElement('ul');
    ul.className = 'bulleted';
    list.appendChild(ul);
    for (let i=0; i<employees.length; i++) {
      let employee = employees[i];
      let li = document.createElement('li');
      li.textContent = employee.name;
      if ( employee.inoffice == true ) {
        li.className = 'in';
      } else {
        li.className = 'out';
      }
      ul.appendChild(li);
    }
  }
};
xhr.open('GET', 'data/employees.json');
xhr.send();