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

get() method's switch to object.prototype.propertyIsEnumerable broke my code #46

Closed
joshua-golub opened this issue Mar 8, 2018 · 3 comments · Fixed by #54
Closed

get() method's switch to object.prototype.propertyIsEnumerable broke my code #46

joshua-golub opened this issue Mar 8, 2018 · 3 comments · Fixed by #54
Labels
bug 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt help wanted

Comments

@joshua-golub
Copy link

joshua-golub commented Mar 8, 2018

Issuehunt badges

This change was made in July 2016:

use `object.prototype.propertyIsEnumerable instead of `Object.getOwnPropertyDescriptor`

Unfortunately, this change broke my code. Specifically, I was using it like this in the Express framework:

dot = require( "dot-prop" );
req.app.locals.foo = 1;
console.log( "foo = " + dot.get( req, "app.locals.foo" ) );

where the req object is defined here.

Before this change, foo = 1 was logged.
After this change, foo = undefined was logged.

The problem appears to be that req.app isn't an enumerable property because it is an inherited property.

So, here's my question: why are we discriminating against inherited properties inside the get() method? That doesn't seem right to me.


IssueHunt Summary

stroncium stroncium has been rewarded.

Backers (Total: $40.00)

Submitted pull Requests


Tips

joshua-golub referenced this issue Mar 8, 2018
* add null check for `has` on the path that's not the last bit

fixes #28

* early return for `delete` when value is not object

fixes #28

* use `object.prototype.propertyIsEnumerable instead of `Object.getOwnPropertyDescriptor`

fixes #28

* `has({foo: undefined}, foo) should be `true`

fixes #27

* fix linting

* use `in` for property detection
@OmgImAlexis
Copy link

This also affects getters. My current work around is to use dot-prop-immutable.

@IssueHuntBot
Copy link

@IssueHunt has funded $40.00 to this issue.


@issuehunt-oss issuehunt-oss bot added the 💵 Funded on Issuehunt This issue has been funded on Issuehunt label May 10, 2019
@issuehunt-oss
Copy link

issuehunt-oss bot commented Oct 7, 2020

@sindresorhus has rewarded $36.00 to @stroncium. See it on IssueHunt

  • 💰 Total deposit: $40.00
  • 🎉 Repository reward(0%): $0.00
  • 🔧 Service fee(10%): $4.00

@issuehunt-oss issuehunt-oss bot added 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt and removed 💵 Funded on Issuehunt This issue has been funded on Issuehunt labels Oct 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt help wanted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants