Skip to content

refactor: use prototype#2165

Merged
bcoe merged 5 commits intoyargs:mainfrom
jly36963:use-prototype
May 16, 2022
Merged

refactor: use prototype#2165
bcoe merged 5 commits intoyargs:mainfrom
jly36963:use-prototype

Conversation

@jly36963
Copy link
Copy Markdown
Contributor

Relates to: #2161 (comment)

@jly36963 jly36963 requested a review from bcoe April 11, 2022 22:43
@bcoe bcoe changed the title fix: use prototype refactor: use prototype Apr 21, 2022
Copy link
Copy Markdown
Member

@bcoe bcoe left a comment

Choose a reason for hiding this comment

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

MDN suggests using Object.hasOwn, vs., Object.hasOwnProperty, or Object.prototype.hasOwnProperty:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/hasOwnProperty#using_hasownproperty_as_a_property_name

Should we go this route?

@bcoe
Copy link
Copy Markdown
Member

bcoe commented Apr 21, 2022

CC: @dhensby ☝️

@dhensby
Copy link
Copy Markdown

dhensby commented Apr 21, 2022

Object.hasOwn() is only in node >= 16.9, so that's a call for this library to make. As the MDN docs say, this implementation (using the Object prototype) is acceptable

@jly36963
Copy link
Copy Markdown
Contributor Author

@bcoe The second code block in the MDN fragment you linked recommends Object.prototype.hasOwnProperty.call as an acceptable alternative, and as @dhensby said, Object.hasOwn is a relatively new feature in Node.

@bcoe
Copy link
Copy Markdown
Member

bcoe commented Apr 25, 2022

👏 works for me, I should have read when hasOwn was introduced.

@bcoe bcoe merged commit 8912078 into yargs:main May 16, 2022
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.

3 participants