skip to Main Content

I have an empty ul in html and everytime I add or remove a color I call ShowColors which adds li items via JavaScript using .innerHTML. How can I optimize this code without using .innerHTML?

const colorList = document.querySelector(".all-colors");

function ShowColors() {
    colorList.innerHTML = savedColorsArray.map(color => `
        <li class="color">
            <span class="rect" data-color="${color}" style="background: ${color};"></span>
        </li>`).join("");
};
<ul class="all-colors"></ul>

I tried .insertAdjacentHTML but I would have to change how the code works to save each element so I can remove it later and also probably have to save it to localStorage so it’s persistent.

savedColorsArray.forEach(color => {
    colorList.insertAdjacentHTML('beforeend', `<li class="color">
    <span class="rect" data-color="${color}" style="background: ${color};"></span>
</li>`);

2

Answers


  1. .innerHTML should be avoided whenever possible because calling it invokes the HTML parser, which rebuilds the DOM with the new content, so there are performance implications to using it.

    While the DOM is being rebuilt with new content, it’s very easy to wipe out previously added event handlers, so there is an opportunity to introduce bugs.

    It is well known that using .innerHTML can potentially open up a security vulnerability in code as well, when the string that is passed to it is not 100% within the control of the developer.

    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 may be rejected in the
    review process. Please see Safely inserting external content into a
    page for alternative methods.

    So, the moral of the story is, only use .innerHTML when you have no other choice. Fortunately, there often are other choices, the first among them being to create new DOM elements, configure them, and then attach them to the DOM.

    Here’s how that is done:

    const colorList = document.querySelector(".all-colors");
    
    function ShowColors() {
      // Create a new DOM elements
      const li = document.createElement("li");
      const span = document.createElement("span");  
      
      // Configure the elements via DOM properties
      li.classList.add("color");
      span.classList.add("rect");
      span.style.backgroundColor = color;
      span.dataset.dataColor = color:
      
      // Append the new DOM element to the appropriate parent elements
      li.appendChild(span);
      colorList.appendChild(li);
    };
    Login or Signup to reply.
  2. A modern approach could be a reactive solution with Proxy:
    you just modify an array of colors and the list is updated automatically.
    You just iterate the color array and try to reuse existing <li>. If there’s a <li> with the same index as a color, update it with dataset.color = color and style.background = color. Otherwise create an <li> with innerHTML like you did. The reusing of DOM elements seems like optimization like you requested.

    enter image description here

    const colorList = document.querySelector(".all-colors");
    
    const deleteItem = item => 
    savedColorsArray.splice([...colorList.children].indexOf(item.closest('.color')), 1);
    
    let updatePromise;
    
    let updateCount = 0;
    
    const savedColorsArray = new Proxy([], {
      set(target, prop, val){
        // postpone in a microtask  
        updatePromise ??= Promise.resolve().then(() => {
          updatePromise = null;
          const log = document.createElement('div');
          log.textContent = `updated ${++updateCount}`;
          document.querySelector('.log').appendChild(log);
          let idx = 0;
          const $children = [...colorList.children];
          for(const color of target){
            let $item = $children[idx++];
            if(!$item){
              const div = document.createElement('div');
              div.innerHTML = `<li class="color">
                <span  class="rect" data-color="${color}" style="background: ${color};"><b onclick="deleteItem(this)">✕</b></span>
            </li>`;
              colorList.appendChild(div.children[0]);
            }else{
              const $span = $item.querySelector('span');
              $span.dataset.color = color;
              $span.style.background = color;
            }
    
          }
          // clear unused items
          while(idx < $children.length){
            $children[idx++].remove();
          }
        });
      
        return Reflect.set(target, prop, val);
      }
    });
    
    savedColorsArray.push(...'red orange yellow lime green blue'.split(' '));
    .color span{
      display:block;
      height: 30px;
      width: 150px;
      border-radius:4px;
      transition: background .5s;
      position:relative;
    }
    .all-colors{
      padding:0;
      margin:0;
    }
    b{
      position:absolute;
      right:5px;
      top:3px;
      cursor:pointer;
    }
    .color{
      padding: 5px;
      display:block;
      list-style:none;
    }
    body{
      display:flex;
      gap:30px;
    }
    <ul class="all-colors"></ul>
    <div>
      <input>
      <button onclick="document.querySelector('input').value.match(/w+/g)?.forEach(color => savedColorsArray.unshift(color))">Add color</button>
    </div>
    <div class="log"></div>
    Login or Signup to reply.
Please signup or login to give your own answer.
Back To Top
Search