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

hasOwnProperty #23

Closed
thatjessicakelly opened this issue Mar 2, 2016 · 11 comments
Closed

hasOwnProperty #23

thatjessicakelly opened this issue Mar 2, 2016 · 11 comments

Comments

@thatjessicakelly
Copy link
Contributor

Right now dot-prop will return any property that isn't undefined. But as a potentially unintended consequence stuff like m.get({}, 'hasOwnProperty') will return the non-enumerable method.

Instead of checking just for undefined, maybe it should test obj.hasOwnProperty(prop).

@sindresorhus
Copy link
Owner

I think that's intentional. If we used hasOwnProperty it would not work on inherited properties.

m.get({}, 'hasOwnProperty') will return the non-enumerable method.

But so will ({}).hasOwnProperty. This module is meant to imitate normal object handling.

@thatjessicakelly
Copy link
Contributor Author

This module is meant to imitate normal object handling.

It doesn't really do that though, undefined is special cased. If you run ({ foo: undefined }).foo.bar you will get a TypeError, where this module will return undefined. But at the same time ({ foo: null }).foo.bar will throw a TypeError in both cases.

I'm not saying that get({ foo: undefined }, 'foo.bar') should throw– it's clearly more useful that it doesn't.

I just think that it's more useful if get({ foo: null }, 'foo.bar') does not throw and get({}, 'hasOwnProperty') returns undefined.

@sindresorhus
Copy link
Owner

I just think that it's more useful if get({ foo: null }, 'foo.bar') does not throw and get({}, 'hasOwnProperty') returns undefined.

Yes. I definitely agree with that.

@stevemao
Copy link
Collaborator

stevemao commented Mar 2, 2016

duplicate of #11?

@stevemao
Copy link
Collaborator

stevemao commented Mar 2, 2016

@sindresorhus Do you want to ignore inherited properties?

@sindresorhus
Copy link
Owner

No

@stevemao
Copy link
Collaborator

stevemao commented Mar 2, 2016

get({}, 'hasOwnProperty') returns undefined.

isn't hasOwnProperty a property of Object.prototype?

@sindresorhus
Copy link
Owner

Yes, but it's enumerable: false:

Object.getOwnPropertyDescriptor(Object.prototype, 'hasOwnProperty');
/*
{
  "writable": true,
  "enumerable": false,
  "configurable": true
}
*/

@stevemao
Copy link
Collaborator

stevemao commented Mar 2, 2016

Hmm.. I'm just trying to understand this. I'm not enumerating the properties of an object.. I'm just trying to access it.

@SamVerschueren
Copy link
Contributor

@stevemao I think what Sindre is trying to say here is that if the descriptor tells us that it is not enumerable, it shouldn't be accessed and thus should be ignored. So inherited properties shouldn't be ignored, only non-enumerable properties. Will do a PR.

@stevemao
Copy link
Collaborator

if the descriptor tells us that it is not enumerable, it shouldn't be accessed and thus should be ignored.

@SamVerschueren, I just think that enumerability should have nothing to do with accessibility...

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

No branches or pull requests

4 participants