-
Notifications
You must be signed in to change notification settings - Fork 505
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
iterate non-enumerables in old IE also #2696
Conversation
👍 but why can't you guys implement all these methods by way of |
@megawac this was the best I came up with. Input is really welcome, to make it even better :) In this case since its a IE8- fix i think |
I mean Looks like this fix isn't being applied to |
@megawac oh, I see. Actually |
@SergioCrisostomo I would say it should be the other way around (thats how most other frameworks otu there are doing it). Having I would argue all the object methods ( |
I think that'd be better indeed. We have to move Object.keys to Core.js from Types/Object.js. After that we could rewrite all |
ca342a3
to
0ee7a00
Compare
var keys = []; | ||
eachKey(object, function(key, value){ | ||
if (hasOwnProperty.call(object, key)) keys.push(key); | ||
}, this); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need this this
here..
@arian updated again. |
var enumerables = true; | ||
for (var i in {toString: 1}) enumerables = null; | ||
if (enumerables) enumerables = ['hasOwnProperty', 'valueOf', 'isPrototypeOf', 'propertyIsEnumerable', 'toLocaleString', 'toString', 'constructor']; | ||
/*</ltIE8>*/ | ||
var hasOwnProperty = Object.prototype.hasOwnProperty; | ||
function eachKey(object, fn, thisArg){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think eachKey
isn't a very nice name. maybe objectEach
, or just each
?
Also I'm thinking if this shouldn't be:
var objectKeys = Object.keys || function(object){
// ...
return keys;
};
And use that in the overloadSetter, with a for-loop there.
And below, just have:
Object.extend({
keys: objectKeys,
forEach: // ...
});
9d41b14
to
7270530
Compare
var keys = []; | ||
for (var k in object){ | ||
if (alsoPrototypeChain) keys.push(k); | ||
else hasOwnProperty.call(object, k) && keys.push(k); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seriously, use else if
, not the x && y
thing!
7270530
to
144e445
Compare
overloadSetter needs to loop over non own properties too.
144e445
to
9f9128a
Compare
added commit and just rebased. |
iterate non-enumerables in old IE also
fixes #2613