_.extend should work-around JScript's DontEnum bug

[timmolendijk]

See: https://developer.mozilla.org/en/ECMAScript_DontEnum_attribute#JScript_DontEnum_Bug

This should evaluate to true:
_.extend({}, {a: 1, toString: 'x'}).toString === 'x'

But in IE (I tested version 8) it evaluates to false.

I suggest a patch along these lines: https://prototype.lighthouseapp.com/projects/8886/tickets/23-object-keys-object-values-dontenum-ie-fix

[jashkenas]

I'm not sure which patch in that ticket you're proposing we adopt -- can you link directly to the patch itself?

I considered this when doing extend in the first place, but this IE bug is so nasty that I'm not sure it's worth doing a work around for -- even if extend is fixed, all of your other for ... in loops will be broken in IE. It's more of a dont-use-JS-objects-as-arbitrary-hashes-period, problem...

[timmolendijk]

Well, the idea is that internally you should never use (for ... in ...). Because it's broken. Instead you use a function that incorporates the fix. I'd say _.each nominates for that.

A work-around is not so complicated. I'm succesfully using the following code to patch the same problem in jQuery ($.al is my own namespace):

// [Source](http://msdn.microsoft.com/en-us/library/kb6te8d3(v=VS.85).aspx)
var objectPrototype = [
    'constructor',
    'propertyIsEnumerable',
    'isPrototypeOf',
    'hasOwnProperty',
    'toLocaleString',
    'toString',
    'valueOf'
];
// Like `jQuery.extend`, but attempts to work-around [JScript's DontEnum
// bug](https://developer.mozilla.org/en/ECMAScript_DontEnum_attribute#
// JScript_DontEnum_Bug). Does not support deep copy for now. Hopefully we can
// expect this utility function to become obsolete once jQuery decides to
// [incorporate a fix](http://bugs.jquery.com/ticket/7467).
$.al.extend = function(target) {
    var result = $.extend.apply(undefined, arguments),
        p, n, prop;

    // TODO: Ideally we would only run this if our current environment suffers
    // from the said bug, but we do not want to invest any more time in this
    // before jQuery has decided if they will be taking care of this problem
    // for us (see ticket as linked above).
    for (var i = typeof arguments[0] === 'boolean' ? 1 : 0, l = arguments.length - i; i < l && arguments[i]; i++) {
        for (p = 0, n = objectPrototype.length; p < n; p++) {
            prop = objectPrototype[p];
            if (arguments[i].hasOwnProperty(prop)) {
                result[prop] = arguments[i][prop];
            }
        }
    }

    return result;
};

[jashkenas]

I'm afraid this sort of fix won't work. _.extend copies all properties from the source object, not just the own-properties. If toString was a function on a prototypal parent, it would still be copied incorrectly in IE, because the hasOwnProperty check would fail.

As far as I'm aware ... there's no complete workaround for this bug in IE, given that you can't traverse the prototype chain upwards in IE either. Even if there were a workaround, it would probably be prohibitively expensive...

Fortunately, the problem isn't total disaster, because it only affects objects that are being used as hashes that contains one of the unenumerable keys. You need to be aware of when you're using a hash of this type (rare, unless you're writing a compiler or code generator), and use an array instead.

Closing the ticket, but I'd love to continue to hear your thoughts about this...

[timmolendijk]

Hmm good points. I guess I never realized that _.extend also copies non-instance-properties. Well I think I share your conclusion in that case.

Fortunately for my particular use case this is not a problem, but it means my work-around is use case specific and not generic. So I'll need to reorganize my code a bit. Thanks for improving my code! ;)