Fix DontEnum bugs w/ _.isEmpty() and _.keys()
jdalton opened this issue · 8 comments
IE and other browsers have bugs with for...in statements.
A simple test case can be found here (try in IE < 9).
// in IE < 9
_.isEmpty({ "toString": 1 }); // expected: false; got: true;
_.keys({ "toString": 1 }); // expected: ["toString"]; got: [];For a reference on how to handle these issues check out spotlight.js.
(the comments in the code also give insight into other for..in issues)
I'm afraid that Underscore isn't supposed to attempt to work around IE's terrible DontEnum bugs.
Why not? Other libs like Prototype and MooTools attempt to?
This seems to be our usual debate ;) ... but let's spell it out clearly.
Underscore was never intended to be a JavaScript library that strived for perfect browser compatibility or spec compliance ... there are plenty of polyfill-ish libraries that already fill that role quite nicely.
The original idea with Underscore was to take the common set of useful data-manipulation primitives that you often need when working on a large JS application, and implement them in the fastest, smallest, and least invasive way possible -- so that if you need to just throw in Underscore into an existing application or web page to use a helper or two, it's never a problem to do so. (As opposed to all of the problems it can cause if you throw in Prototype.js on an existing webpage.)
To that extent, we want to go the extra mile to implement cross-browser fixes to the extent that they don't slow things down too much, they don't bloat the code too much, and most importantly, they actually solve a bug that people run into in real-world code, not just in test cases.
IE's DontEnum bug is horrible, and it makes JS objects unusable as arbitrary hashes -- you often have to use arrays instead. (https://github.com/jashkenas/coffee-script/blob/master/src/scope.coffee#L21) Even if we did come up with a foolproof way to have all Underscore iteration functions work around the DontEnum bug, you still wouldn't want to use JS objects as arbitrary hashes because regular for..in iteration would fail, and an array would probably be simpler and more efficient, even if less conceptually clean.
So, Underscore is happy to cede that ground to Spotlight.js and other similar libraries. For the record, MooTools and Prototype don't seem to attempt for fix DontEnum either:
https://github.com/sstephenson/prototype/blob/master/src/prototype/lang/object.js#L308-317
https://github.com/mootools/mootools-core/blob/master/Source/Core/Core.js#L298-303
Underscore was never intended to be a JavaScript library that strived for perfect browser compatibility or spec compliance ... there are plenty of polyfill-ish libraries that already fill that role quite nicely.
Devs expect browser compatibility and as you've seen being spec compliant helps Underscore.js leverage native performance gains while maintaining a consistent fallback for older browsers.
To that extent, we want to go the extra mile to implement cross-browser fixes to the extent that they don't slow things down too much, they don't bloat the code too much,
Lazy defining / code forking can solve slow-down concerns and be kept small enough that size increase is minimal.
and most importantly, they actually solve a bug that people run into in real-world code, not just in test cases..
I and others, as seen by MooTools, Prototype, and other lib's support, have been bit by this issue in various use cases. I've run into for..in gotchas far more often than issues with switch statement fall throughs or == and some felt the need for a compile step to address those.
IE's DontEnum bug is horrible, and it makes JS objects unusable as arbitrary hashes -- you often have to use arrays instead.
I've seen you write this before, but you can use objects as hashes, in a cross-browser way, by simply prefixing the keys or adding hasOwnProperty checks. Though I think you are getting derailed a bit. Being able to consistently iterate an object's properties seems so basic and assumed it's almost a no brainer.
Even if we did come up with a foolproof way to have all Underscore iteration functions work around the DontEnum bug,
It's not a matter of if a way exists, it does as other libs have shown.
So, Underscore is happy to cede that ground to Spotlight.js and other similar libraries.
Spotlight.js is an environment exploration utility not smth to hook into libs like Underscore.js.
For the record, MooTools and Prototype don't seem to attempt for fix DontEnum either:
You're looking in the wrong place:
MooTools covers it here+here+here+here+here (and plans to cover the oversight you linked to in v2) and Prototype attempts to cover it here (and recently patched their Object.keys implementation). In addition Dojo (here) and YUI (here+here+here) also address this bug in some way.
+1 for doing the right thing and handling these scenarios as expected.
+1
@jashkenas the code for Prototype keys() is slightly out of sync with the intentions of the authors. The most recent discussion was in Oct 2010: https://groups.google.com/forum/#!topic/prototype-core/SEANvtTB3WI it was agreed that handling DontEnum issues would be added - unfortunately, this patch was never filed by the OP, but it certainly garnered support from the lib authors. The last time keys was actually worked on was Feb 2010 and Dec 2008 before that. https://github.com/sstephenson/prototype/blame/master/src/prototype/lang/object.js (Blames don't allow linking to lines, so check out line 308-317 )
+1 for doing the right thing and handling these scenarios as expected.
+1, I'm all for doing things properly.
"Any job worth doing is worth doing right."