Extending javascript array potentially breaks other code #1392

Closed
elgerm opened this Issue Jan 23, 2013 · 3 comments

Comments

Projects
None yet
5 participants

elgerm commented Jan 23, 2013

This one has been asked before but I saw this code in signalr.js:

// Array.prototype.map
if (!Array.prototype.hasOwnProperty("map")) {
    Array.prototype.map = function (fun, thisp) {
        var arr = this,
            i,
            length = arr.length,
            result = [];
        for (i = 0; i < length; i += 1) {
            if (arr.hasOwnProperty(i)) {
                result[i] = fun.call(thisp, arr[i], i, arr);
            }
        }
        return result;
    };
}

This results in that in each iteration/enumeration through arrays requires an extra line of code: array.hasOwnProperty() as well as third party code could break if they didn't.

It took me a while before I found out and gave me hard to track errors.

Any intentions of changing this in the future or do I have to live with it? :)

Owner

davidfowl commented Jan 23, 2013

What broke in your code?

elgerm commented Jan 23, 2013

For instance, I had the following legacy code imitating an observer
pattern, it uses a Hash wrapper that encapsulates an array:

this.Notify = function (conv) {
    for (var o in _this.observers.items) {
            var del = _this.observers.items[o];
            del(conv);
    }
}

Without checking with hasOwnProperty(), this breaks in IE8 because this
piece of code calls array.prototype.map in the signalr code.

The code snippet is what is wrong, I know that. But I can imagine that more
people will get strange behaviour when including the signalr.js file. I
also had to update jquery.address and some watermark plugin that had the
same kind of (arguably wrong) code which gave me 'this object doesn't
support this property or method' errors. (also IE ofcourse)

On Wed, Jan 23, 2013 at 1:48 PM, David Fowler notifications@github.comwrote:

What broke in your code?


Reply to this email directly or view it on GitHubhttps://github.com/SignalR/SignalR/issues/1392#issuecomment-12583697.

@DamianEdwards DamianEdwards added a commit that referenced this issue Mar 20, 2013

@DamianEdwards DamianEdwards Don't extend Array.prototype with map function
- Can cause issue with some site's script
- Just have a private map function instead
- #1392
c1ba3f4

Xiaohongt was assigned Mar 20, 2013

Contributor

Xiaohongt commented Mar 21, 2013

verified

Xiaohongt closed this Mar 21, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment