Don't rely on Array.prototype.map #31
Conversation
|
What browsers are of concern here? |
|
IE8 and below. Still relevant in some settings, especially since your lib is used by test runners and assertion libraries. |
| var other = new Array(arr.length); | ||
|
|
||
| for (var i = 0, n = arr.length; i < n; i++) { | ||
| if (i in arr) { |
There was a problem hiding this comment.
What is this in operation doing? Seems like it's just overhead for an iterator that is walking length.
There was a problem hiding this comment.
Hmm, I didn't think about that, I just copied the function from somewhere else. I think it originates from https://github.com/LearnBoost/expect.js/blame/master/index.js#L812-L824
It seems like the reason for that check is sparse arrays. If I comment out the check and the use of Array.prototype.map I get:
console.log(map(new Array(5), function (item) {return 10;})); // [10, 10, 10, 10, 10]as opposed to: [ , , , , ] when the check is there.
There was a problem hiding this comment.
I'd remove it as none of the uses here are spare arrays.
|
Looks like this has a merge conflict with some other refactoring work that went in. If you can rebase this I'll merge. Otherwise I'll try to rebase but it might take a little bit of time. |
|
Rebased. |
|
Awesome! Thank you. |
Don't rely on Array.prototype.map
|
Thanks! How about a new npm release soon? |
|
Released in 1.1.0 |
|
Sorry for the delay, I was hoping to try to resolve some of the other issues in a larger release but I opted to get this out and get it out of my TODO list (as I was ignoring the item, which didn't help). |
Seems like this is the only thing that makes https://github.com/visionmedia/mocha not work in older browsers.