Why not just use Array.prototype.forEach?

[chris-morgan]

As far as I can tell, Array.prototype.forEach will function identically to NodeList.prototype.forEach and would thus be fine on NodeList.prototype:

if (typeof NodeList != "undefined" && !NodeList.prototype.forEach) {
    NodeList.prototype.forEach = Array.prototype.forEach;
}

This simplifies and shortens the code, but introduces a dependency on Array.prototype.forEach (IE9+). But it seems implausible that one would polyfill one and not the other if caring about IE<9 (and who does, these days?), so I think it’d be reasonable to do that.

[AlexWayfer]

If you’ve ever done this, then it probably wasn’t a good idea (and please do not use it). Extending existing DOM functionality through prototypes is often considered bad practice as this can lead to masses of issues.

Reference.

This is pretty intense (probably dangerous and not recommended)

Another reference.

It's often not a good idea to extend the functionality of DOM through prototypes, especially in older versions of IE (article).

One more reference with the link to article.

[chris-morgan]

@AlexWayfer I am perplexed by your comment. This polyfill is already assigning to NodeList.prototype.forEach—as a polyfill must. My suggestion is simply to use an already-existing forEach method instead of writing a new one.

The articles you link to are uniformly out of date (spanning 2010 to 2014), and not applicable to polyfills.

Some comments specifically on the Todd Motto article you linked: it’s a terrible article. Even back in early 2014 when it was written it was by and large a bad article. Certainly, the specific thing of not defining NodeList.prototype.forEach was reasonable back then, because it wasn’t a standard yet; now, however, it’s perfectly reasonable, because it’s a defined standard—and exactly what this polyfill is for. And using [].forEach instead of Array.prototype.forEach was always silly, so I agree with his fourth problem (and the part of the fifth problem which is just repeating the fourth problem). But problems one, two three, six, seven and eight and eleven were fairly ridiculous when the article was written, and now in the context of NodeList.prototype.forEach are completely irrelevant. And as for the recommendation to write your own forEach method—that is the height of the craziness in the article: that’s exactly what Array.prototype.forEach is; as with most of Array’s methods, it’s deliberately designed to be generic, and perfectly safe for application on other types with a length property and properties matching the indexes. So writing another forEach method is simply slowing things down further.

[chris-morgan]

Ooh, one other thing I just noticed: in both Chrome and Firefox, NodeList.prototype.forEach is actually the same function as Array.prototype.forEach. That is, NodeList.prototype.forEach === Array.prototype.forEach.

[AlexWayfer]

@chris-morgan, OK, you're right, thank you for the answer.

Then, we can use polyfill-nodelist-foreach package with suggested by you realization.

[FrankConijn]

Even simpler:

if (Array.prototype.forEach) NodeList.prototype.forEach = Array.prototype.forEach;?

[chris-morgan]

@FrankConijn There are two problems with your suggestion:

1. Clobbering NodeList.prototype.forEach unnecessarily is bad form. If already present, it’s not necessarily the same function as Array.prototype.forEach; it’s possible that in future it may be specialised for operating more efficiently on a node list. Also, writing to such an object has a habit of slowing down JavaScript engines as they fall back to a slow path, though the difference will normally not be measurable.

2. There is no guarantee that NodeList is defined; in browsers, it will be, but in other environments such as Node, it may not be. A polyfill like this is generally designed to just be a noop in other environments that don’t have a prerequisite for what is being polyfilled. (On that note, in writing window.NodeList in my suggestion I did the wrong thing: I should indeed have written typeof NodeList != "undefined", because the global object is not necessarily named window.)

[FrankConijn]

@chris-morgan -- According to TJ Crowder on https://stackoverflow.com/a/46929259/2056165, after simplifying and debugging it per my suggestions, it should be: if (typeof NodeList !== "undefined" && NodeList.prototype && !NodeList.prototype.forEach) NodeList.prototype.forEach = Array.prototype.forEach;

So, he is adding && NodeList.prototype. Would you agree that that is indeed necessary or at least useful?

[chris-morgan]

Checking NodeList.prototype is just rigour on TJ’s part—if someone had defined a global NodeList which wasn’t a type (outrageous, but theoretically possible), that would prevent it from causing an exception. I could take it or leave it.

(It’s similar rigour that leads to using !== instead of != which is sufficient in this case, given that both sides are known to be strings.)

[mitikov]

@FrankConijn @chris-morgan what's the summary?

I recall stepping into similar trouble a while ago (server-side rendering), but we managed to walk around by hand-written polyfill.

Considering the package already gives similar functionality it seem we could include the suggestion.

Agree?

Thanks.