intercellular / cell

A self-driving web app framework
https://www.celljs.org
MIT License
1.5k stars 94 forks source link

forEach polyfil #29

Closed fprijate closed 7 years ago

fprijate commented 7 years ago

NodeList behaves as array so:


if (window.NodeList && !NodeList.prototype.forEach) { 
  NodeList.prototype.forEach = Array.prototype.forEach;
}

It's shorter and throws some exceptions. Array.prototype.forEach

gliechtenstein commented 7 years ago

Thanks for pointing out. The Array.prototype.forEach option was actually what I've been using throughout 99% of the development process before I released the library to public.

The reason I switched it at the last moment was because so many people online keep saying this is a bad pattern (although I don't agree entirely) and I didn't want to worry too much about this so just decided to go with the polyfill I found on MDN (I mean it's MDN, so gotta be alright, right? :) )

BUT, I honestly don't think they will make a big difference and prefer your Array.prototype.forEach option over the MDN one since I want to keep the code as short as possible.

If anyone has thoughts on this or think I'm missing something, please jump in and share. I'm almost leaning towards using this Array.prototype.forEach solution.

fprijate commented 7 years ago

Hi

I am aware of MDN polyfil IMO it's intended for older browsers which don't implement functional Array methods (map, filter, forEach, ...). We use map method in few places. We should polyfill map and other Array methods to in this case. All modern browsers implement them natively (except Edge misses Nodelist.forEach). So your solution is sane. Anyone who is fan of ancient browsers can anyhow use some polyfill library.

gliechtenstein commented 7 years ago

Thanks for the suggestion, we finally incorporated this with https://github.com/intercellular/cell/commit/b4751fc15f3442707b3b442b9d0ce0fc16aed7f3

Now it's just a one-liner! https://github.com/intercellular/cell/blob/develop/cell.js#L376