Why is "element.innerHTML+=" bad code?
Asked Answered
C

9

64

I have been told not to append stuff using element.innerHTML += ... like this:

var str = "<div>hello world</div>";
var elm = document.getElementById("targetID");

elm.innerHTML += str; //not a good idea?

What is wrong with it?, what other alternatives do I have?

Cherrylchersonese answered 17/7, 2012 at 2:50 Comment(6)
Did they told you what to use instead of element.innerHTML +=?Friedafriedberg
I doubt whoever warned you against it has a good reason for doing so in 2012. Browsers are fast and support for this property is nearly universal. Other than sticking to a particular library's facilities (if you're using jQuery, use the $() constructor to create elements) I don't see any problem any more. As always though, test and find out.Incapacious
you should read this article : taligarsiel.com/Projects/howbrowserswork1.htmMccants
@Triptych: There are considerations other than performance. For example, since the DOM subtree of the main element is completely reconstructed, event handlers on the existing elements will be destroyed. Also, any <script> within the element will be rerun in some browsers. Finally, there is no guarantee that elm.innerHTML = elm.innerHTML will always reproduce an identical copy of the element's DOM.Abie
You have nothing that shows what innerHTML references initially, with the += you will just be appending content onto existing content. Also, its better to add content to the DOM using the functions appendChild.Singlecross
Related: Is it possible to append to innerHTML without destroying descendants' event listeners?Earmark
B
63

Every time innerHTML is set, the HTML has to be parsed, a DOM constructed, and inserted into the document. This takes time.

For example, if elm.innerHTML has thousands of divs, tables, lists, images, etc, then calling .innerHTML += ... is going to cause the parser to re-parse all that stuff over again. This could also break references to already constructed DOM elements and cause other chaos. In reality, all you want to do is append a single new element to the end.

It's better to just call appendChild:

var newElement = document.createElement('div');
newElement.innerHTML = '<div>Hello World!</div>';
elm.appendChild(newElement);​​​​​​​​​​​​​​​​

This way, the existing contents of elm are not parsed again.

NOTE: It's possible that [some] browsers are smart enough to optimize the += operator and not re-parse the existing contents. I have not researched this.

Blackett answered 17/7, 2012 at 2:52 Comment(10)
In this example, he only sets innerHTML once though, so I'm not sure this answer the question.Friedafriedberg
but what if the target have already some important stuff that we want to keep (e.g appending to the body)?, this would erase itCherrylchersonese
Just do var s = elem.innerHTMLCalculous
The issue is less the parsing of what you are adding and more the serialization of existing DOM nodes, destruction of them, reparsing and rebuilding the DOM tree when appending HTML.Worldbeater
Okay say that innerHTML had 200,000 bytes of stuff in it. Then, you append a single <div>Hello</div>. Now, you're re-parsing 200,000 bytes plus the single DIV and re-constructing that entire DOM over again. It would be far better to just call appendChild() once and insert into the existing DOM directly.Blackett
+1 very useful edit, I will sooner or later be working with really big tables.Cherrylchersonese
@Cherrylchersonese - No problem, my original answer was perfectly accurate but didn't address your specific scenario.Blackett
Don't forget that it also demolishes any stateful data on the nodes that were destroyed. This includes event handlers.Strobotron
@amnotiam - Yea that's what I was alluding to with "cause other chaos"Blackett
@MikeChristensen How would you go about saving that dynamic content for use next time the page is loaded?Anabolism
T
37

Yes, elm.innerHTML += str; is a very bad idea.
Use elm.insertAdjacentHTML( 'beforeend', str ) as the perfect alternative.

The typical "browser has to rebuild DOM" answer really doesn't do the question justice:

  1. First the browser need to go through each elements under elm, each of their properties, and all their texts & comments & process nodes, and escape them to build you a string.

  2. Then you have a long string, which you append to. This step is ok.

  3. Third, when you set innerHTML, browser has to remove all the elements, properties, and nodes it just went through.

  4. Then it parse the string, build from all the elements, properties, and nodes it just destroyed, to create a new DOM fragment that is mostly identical.

  5. Finally it attach the new nodes, and the browser has to layout the whole thing. This may be avoidable (see the alternative below), but even if the appended node(s) requires a layout, old nodes would have their layout properties cached instead of re-calculated from fresh.

  6. But it's not done yet! The browser also have to recycle old nodes by scanning all javascript variables.

Problems:

  • Some properties may not be reflected by HTML, for example the current value of <input> will be lost and reset to the initial value in the HTML.

  • If you have any event handlers on the old nodes, they will be destroyed and you have to reattach all of them.

  • If your js code is referencing any old nodes, they will not be destroyed but will instead be orphaned. They belong to the document but is no longer in the DOM tree. When your code access them, nothing may happen or it may throw error.

  • Both problems mean it is unfriendly with js plugins - the plugins may attach handlers, or keep reference to old nodes and cause memory leak.

  • If you get into the habit of doing DOM manipulation with innerHTML, you may accidentally change properties or do other things you didn't want to.

  • The more nodes you have, the more inefficient this is, the more battery juice for nothing.

In short, it is inefficient, it is error prone, it is simply lazy and uninformed.


The best alternative is Element.insertAdjacentHTML, that I haven't seen other answers mention:

elm.insertAdjacentHTML( 'beforeend', str )

Almost same code, without innerHTML's problems. No rebuild, no handler lost, no input reset, less memory fragmentation, no bad habit, no manual element creations and assignments.

It allows you to inject html string into elements in one line, including properties, and even allows yow to inject composite and multiple elements. Its speed is optimised - in Mozilla's test it is 150 times faster.

In case someone tell you it is not cross browser, it is so useful that it is HTML5 standard and available on all browsers.

Don't ever write elm.innerHTML+= again.

Teplica answered 30/11, 2015 at 9:40 Comment(11)
+1 for highlighting the issues with .innerHTML. I am not much convinced with Element.insertAdjacentHTML as an alternative though. .innerHTML helps in replacing everything inside an element, but Element.insertAdjacentHTML is about insertion, not replacement.Gaunt
@RahulDesai This question is specifically about insertion, true. For total replacement sometimes I removeChild & appendChild / insertAdjacement, sometimes replaceNode with createElement / createContextualFragment.Teplica
On publishing extension to Mozilla store I get the warning: Unsafe call to insertAdjacentHTML Warning: Due to both security and performance concerns, this may not be set using dynamic values which have not been adequately sanitized. This can lead to security issues or fairly serious performance degradation.Mammilla
@VitalyZdanevich If a similiar issue I found in google is any indication, the warning should be on your input and you'll get the same message even if you set innerHTML instead of calling insertAdjacentHTML.Teplica
Nowadays both innerHTML and insertAdjacentHTML are considered to be a bad idea. DOMParser object is recommended instead.Hearst
@Hearst I think DOMParser serves a different purpose, while being heavier than insertAdjacentHTML. Any references to back your claim, such as a browser or js blog article?Teplica
It is certainly heavier and I do not like it at all but Mozilla lately forces me to use it to avoid validation warnings due to XSS security issues. discourse.mozilla.org/t/…Hearst
@Hearst Well, insertAdjacentHTML does not even execute script, at least in the user land. Considering the scope of the question, I'd prefer to leave the topic of XSS and addon validation for another Q&A.Teplica
The documentation for insertAdjacentHTML has security precautions about "escaping" things correctly as otherwise, things can go really bad. Also, why do you think DOMParser serves a different purpose? It might have multiple purposes but it also works here, even though it is slower.Hearst
@Hearst Does the same precautions not apply to DOMParser? Webview and browser do same things, do they serve same purpose? Depends on the purpose, perhaps, and here the question is not related with addons, much less Mozilla addons. If you want to discuss more you can ping me in the javascript chatroom.Teplica
To be correct, the security validations I mentioned are not limited to addons or Mozilla, they are part of general programming guidelines which are updated oftern, so matters related to alternative methods of achieving same result are very much valid.Hearst
H
9

Short

If you change innerHTML += ... (update content) to innerHTML = ... (regenerate content) then you will get very fast code. It looks like the slowest part of += is READING DOM content as string (not transforming string to DOM)

Drawback of using innerHTML is that you loose old content event handlers - however you can use tag arguments to omit this e.g. <div onclick="yourfunc(event)"> which is acceptable in small projects

Long

I made performance tests HERE on Chrome, Firefox and Safari (2019 May) (you can run them in your machine but be patient - it takes ~5 min)

enter image description here

function up() {
  var container = document.createElement('div');
  container.id = 'container';
  container.innerHTML = "<p>Init <span>!!!</span></p>"
  document.body.appendChild(container);
}

function down() {
  container.remove()
}

up();

// innerHTML+=
container.innerHTML += "<p>Just first <span>text</span> here</p>";
container.innerHTML += "<p>Just second <span>text</span> here</p>";
container.innerHTML += "<p>Just third <span>text</span> here</p>";

down();up();

// innerHTML += str
var s='';
s += "<p>Just first <span>text</span> here</p>";
s += "<p>Just second <span>text</span> here</p>";
s += "<p>Just third <span>text</span> here</p>";
container.innerHTML += s;

down();up();

// innerHTML = innerHTML+str
var s=container.innerHTML+'';
s += "<p>Just first <span>text</span> here</p>";
s += "<p>Just second <span>text</span> here</p>";
s += "<p>Just third <span>text</span> here</p>";
container.innerHTML = s;

down();up();

// innerHTML = str
var s="<p>Init <span>!!!</span></p>";
s += "<p>Just first <span>text</span> here</p>";
s += "<p>Just second <span>text</span> here</p>";
s += "<p>Just third <span>text</span> here</p>";
container.innerHTML = s;

down();up();

// insertAdjacentHTML str
var s='';
s += "<p>Just first <span>text</span> here</p>";
s += "<p>Just second <span>text</span> here</p>";
s += "<p>Just third <span>text</span> here</p>";
container.insertAdjacentHTML("beforeend",s);

down();up();

// appendChild
var p1 = document.createElement("p");
var s1 = document.createElement("span"); 
s1.appendChild( document.createTextNode("text ") );
p1.appendChild( document.createTextNode("Just first ") );
p1.appendChild( s1 );
p1.appendChild( document.createTextNode(" here") );
container.appendChild(p1);

var p2 = document.createElement("p");
var s2 = document.createElement("span"); 
s2.appendChild( document.createTextNode("text ") );
p2.appendChild( document.createTextNode("Just second ") );
p2.appendChild( s2 );
p2.appendChild( document.createTextNode(" here") );
container.appendChild(p2);

var p3 = document.createElement("p");
var s3 = document.createElement("span"); 
s3.appendChild( document.createTextNode("text ") );
p3.appendChild( document.createTextNode("Just third ") );
p3.appendChild( s3 );
p3.appendChild( document.createTextNode(" here") );
container.appendChild(p3);

down();up();

// insertAdjacentHTML
container.insertAdjacentHTML("beforeend","<p>Just first <span>text</span> here</p>");
container.insertAdjacentHTML("beforeend","<p>Just second <span>text</span> here</p>");
container.insertAdjacentHTML("beforeend","<p>Just third <span>text</span> here</p>");

down();up();

// appendChild and innerHTML
var p1 = document.createElement('p');
p1.innerHTML = 'Just first <span>text</span> here';
var p2 = document.createElement('p');
p2.innerHTML = 'Just second <span>text</span> here';
var p3 = document.createElement('p');
p3.innerHTML = 'Just third <span>text</span> here';
container.appendChild(p1);
container.appendChild(p2);
container.appendChild(p3);
b {color: red}
<b>This snippet NOT test anythig - only presents code used in tests</b>

  • For all browsers the innerHTML += was the slowest solutions.
  • The fastest solution for chrome appendChild - it is ~38% faster than second fast solutions but it is very unhandy. Surprisingly on Firefox appendChild was slower than innerHTML =.
  • The second fast solutions and similar performance we get for insertAdjacentHTML str and innerHTML = str
  • If we look closer on case innerHTML = innerHTML +str and compare with innerHTML = str it looks like the slowest part of innerHTML += is READING DOM content as string (not transforming string to DOM)
  • If you want change DOM tree generate first whole string(with html) and update/regenerate DOM only ONCE
  • mixing appendChild with innerHTML= is actually slower than pure innerHTML=
Headspring answered 3/5, 2019 at 13:35 Comment(0)
O
8

The alternative is .createElement(), .textContent, and .appendChild(). Appending with += is only an issue if you're dealing with a lot of data.

Demo: http://jsfiddle.net/ThinkingStiff/v6WgG/

Script

var elm = document.getElementById( 'targetID' ),
    div = document.createElement( 'div' );
div.textContent = 'goodbye world';
elm.appendChild( div );

HTML

<div id="targetID">hello world</div>
Orifice answered 17/7, 2012 at 2:58 Comment(0)
A
2

If the user has older versions of IE (or maybe newer ones too, haven't tried), innerHTML on a td will cause issues. Table elements in IE are read-only, tsk tsk tsk.

Assiut answered 17/7, 2012 at 2:54 Comment(0)
F
2

I just learned the hard way why innerHTML is bad, in this code below when you set innerHTML chrome loses the onclick event jsFiddle

var blah = document.getElementById('blah');
var div = document.createElement('button');
div.style['background-color'] = 'black';
div.style.padding = '20px;';
div.style.innerHTML = 'a';
div.onclick = () => { alert('wtf');};

blah.appendChild(div);

// Uncomment this to make onclick stop working
blah.innerHTML += ' this is the culprit';

<div id="blah">
</div>
Fully answered 3/4, 2017 at 22:51 Comment(0)
G
1

Mike's answer is probably the better one, but another consideration is that you are dealing with strings. And string concatenation in JavaScript can be really slow especially in some older browsers. If you are just concatenating little fragments from HTML then it probably isn't noticeable, but if you have a major part of the page that you are appending something to repeatedly you very well could see a noticeable pause in the browser.

Genisia answered 17/7, 2012 at 2:56 Comment(0)
J
1

One way to do it (but not performance tested) : (inspired by DDRRSS response)

    const parser = new DOMParser();
    const parsedBody = parser.parseFromString(str, 'text/html').body;
    for(let i = 0; i <= parsedBody.childNodes.length; i++){
        const tag = parsedBody.childNodes[i];
        if(!tag) continue;

        if(tag instanceof Text){
            codeElement.append(document.createTextNode(tag.textContent));
        } else if(tag instanceof HTMLElement){
            codeElement.appendChild(tag.cloneNode(true));
        }}

    codeElement.appendChild(document.createTextNode(parsedBody.innerText));
Jollenta answered 24/3, 2020 at 19:40 Comment(0)
H
0

Another reason why "element.innerHTML+=" is bad code is because changing innerHTML property directly has been recently found to be insecure in principle, which is now reflected e.g. in Mozilla's warning about it.

Here is a Javascript security/XSS validation-safe/proof practical example of working without using innerHTML property or insertAdjacentHTML method. The content of some htmlElement gets updated-replaced with a new HTML content:

const parser = new DOMParser(),
      tags = parser.parseFromString('[some HTML code]'), `text/html`).body.children, 
      range = document.createRange();
range.selectNodeContents(htmlElement);
range.deleteContents();
for (let i = tags.length; i > 0; i--)
{
     htmlElement.appendChild(tags[0]); 
     // latter elements in HTMLCollection get automatically emptied out with each use of
     // appendChild method, moving later elements to the first position (0), so 'tags' 
     // can not be walked through normally via usual 'for of' loop
}

However, the parser-generated DOM might be too safe for cases when you need to insert script nodes which might end up appearing in DOM but not executed. In such cases, one might want to use this method:

const fragment = document.createRange().createContextualFragment('[some HTML code]');

Hearst answered 26/2, 2020 at 10:34 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.