Skip to content
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

Keys shim misses non enumerable properties #1755

Merged
merged 2 commits into from
Sep 2, 2014

Conversation

megawac
Copy link
Collaborator

@megawac megawac commented Jul 21, 2014

Currently the _.matches tests fail in IE<9 because of this bug.

Tested in ie7 and ie8

// Retrieve the names of an object's properties.
// Delegates to **ECMAScript 5**'s native `Object.keys`
_.keys = function(obj) {
if (!_.isObject(obj)) return [];
if (nativeKeys) return nativeKeys(obj);
var keys = [];
var keys = [], nonemerableIndex = 5;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo? Looks like you meant nonEnumerableIndex.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah thanks, made 2 of those

@jdalton
Copy link
Contributor

jdalton commented Jul 21, 2014

The _.matches test that causes the fail is this one.

jashkenas added a commit that referenced this pull request Sep 2, 2014
Keys shim misses non enumerable properties
@jashkenas jashkenas merged commit b247bab into jashkenas:master Sep 2, 2014
@megawac megawac deleted the keys-fix branch September 2, 2014 22:01
@jdalton
Copy link
Contributor

jdalton commented Sep 2, 2014

It would rock to have a version of this that worked for for-in iteration as there are methods which will, as of the 1.7.1 revert, no longer use _.keys. Which means for example _.extend and _.clone will miss properties like constructor in IE < 9.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants