Polyfill-service: Add NodeList polyfills

[gustavpursche]

At the guardian we realized yesterday, that there is currently no polyfill for NodeList.forEach(). According to MDN there is no support for the feature in the following browsers:

|Browser|Version|
|---|---|
|Chrome|< 51|
|Firefox| <50
|IE|*|
|Opera|<38|
|Safari|<10|

The possible polyfill would be quite small:

if (window.NodeList && !NodeList.prototype.forEach) {
    NodeList.prototype.forEach = function (callback, argument) {
        argument = argument || window;
        for (var i = 0; i < this.length; i++) {
            callback.call(argument, this[i], i, this);
        }
    };
}

Do you think it's worth adding this as a single polyfill, or would you rather like to polyfill all potential missing methods on NodeList, which are .entries(), forEach(), item(), keys(), values()?

I'm happy to work on a PR for this.

[JakeChampion]

A single polyfill for NodeList.prototype.foreach would be great. If you want to add all the other methods as other individual polyfills that would be very much appreciated :-)

[gustavpursche]

@JakeChampion When I wanted to fill out the Spec field, I discovered this actually isn't part of any Spec. FF, Chrome and Webkit just tries to help helpful. I guess that's a reason why .forEach() shouldn't go in here, right? The other ones are in the Spec.

[JakeChampion]

@gustavpursche Everything except forEach is inside the DOM specification: https://dom.spec.whatwg.org/#interface-nodelist

[JakeChampion]

I think we should avoid adding forEach since it is not in a specification. Thanks for taking the time to find all this out.

[gustavpursche]

I agree. I'll ask the flow people aswell why they put it in. Thanks!

[jakearchibald]

@JakeChampion

I think we should avoid adding forEach since it is not in a specification

https://heycam.github.io/webidl/#idl-iterable says any iterable interface should have forEach. However, I believe there were some libraries that used thing.map etc as a way of assuming "This is an array, not a nodelist", so browsers haven't added it.

[JakeChampion]

@jakearchibald Thanks for the information and the link! If it is in IDL definition of Iterable then we should look to include a polyfill for it.

@gustavpursche Are you okay to submit a PR which adds this feature?

[gustavpursche]

@jakearchibald @JakeChampion Thanks for looking into this. I realized the last couple of days how little knowledge I have about these low-level Interfaces. I'll have to read a bit more about it, but I hope next week I can start working on it.

[jakearchibald]

FWIW, it also caught me out that iterable also adds forEach etc.

[amannn]

May I ask why this issue was closed? According to the linked spec above, a NodeList instance should have a forEach method – no? Also the flow PR was closed because of this reason.

[gustavpursche]

@amannn I closed it, because I had the feeling I was convinced that the specs differ. Also there is the difference between the specs and the browser realities. I'm not deep enough into this, to say what's the way to go. Feel free to open the issue(s) again, if you disagree.

[amannn]

Actually I also don't know much about specs. I just thought the consensus in this thread was that it should rather be added, therefore I wanted to ask.

[nlenkowski]

Just ran into this as well. I wrongly assumed that NodeList.forEach() would be polyfilled for IE10/11 and be safe to use. +1 for adding it, in the meantime the mdn polyfill works just fine.

[Sora2455]

Just to summarise the thread so far:

1. NodeList is defined as iterable in the DOM Living Standard
2. Iterable is defined as having a foreach method in the Web IDL Candidate Recomendation
3. According to MDN, the current versions of Edge, Chrome, Firefox, Opera and Safari all support foreach on Nodelist (at the time of writing).

As this is such an easy thing to polyfill, I say yes, definitely add it.

[cferdinandi]

So.... it's been almost a year. I see the polyfill for this in the source code, but I'm able to get it to load in any way: https://github.com/Financial-Times/polyfill-service/tree/master/packages/polyfill-library/polyfills/NodeList/prototype/forEach

Am I missing something?

[michaelpumo]

Any news on getting NodeList.prototype.forEach working? I don't see any mention of it in the API docs.

Seems kind of crazy that modern browsers all support this, yet we're arguing whether to add it in? As was previously discussed NodeList is iterable, so it should have a forEach...?

[loilo]

I don't think that anyone's arguing whether the feature should be added. It has in fact already been merged into master over six months ago.

However, no new version has been published to polyfill.io since January — not sure why.

[loilo]

For anyone subscribed to this: This feature is available in polyfill.io v3 which has been out for a couple days.

It's not in the default feature set though, you have to include it manually:

https://polyfill.io/v3/polyfill.min.js?features=default,NodeList.prototype.forEach

@JakeChampion and others, looks like this issue can be closed now.

[JakeChampion]

Thank you @Loilo, it is difficult to remember which issues have been fixed in v3 :-)

[loilo]

I'm sure it is. Thanks for putting all the work into open sourcing these things. :)

[gustavpursche]

@Loilo Thanks for clarification! ✨

//cc @SiAdcock