-
-
Notifications
You must be signed in to change notification settings - Fork 4k
refactor: use Object.hasOwn instead of Object.prototype.hasOwnProperty(...) #15866
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
Conversation
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.
Pull request overview
This PR refactors the codebase to use Object.hasOwn() instead of the older .hasOwnProperty() and Object.prototype.hasOwnProperty.call() patterns. The motivation is to improve code safety (works with Object.create(null)), performance (static method with less overhead), and readability (no need for .call() to avoid prototype shadowing).
Key changes:
- Replaced all instances of
.hasOwnProperty()withObject.hasOwn()across 24 files - Replaced all instances of
Object.prototype.hasOwnProperty.call()withObject.hasOwn() - Minor refactoring in
lib/document.jsto use a for loop instead of.filter().forEach()for better performance
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/virtualType.js | Replace .hasOwnProperty() with Object.hasOwn() for populated virtuals check |
| lib/types/objectid.js | Replace .hasOwnProperty() with Object.hasOwn() for valueOf property check |
| lib/types/documentArray/index.js | Replace .hasOwnProperty() with Object.hasOwn() in proxy handlers (with bug in optional chaining) |
| lib/types/array/index.js | Replace .hasOwnProperty() with Object.hasOwn() in proxy handlers (with bug in optional chaining) |
| lib/schemaType.js | Replace .hasOwnProperty() and Object.prototype.hasOwnProperty.call() with Object.hasOwn() |
| lib/schema/array.js | Replace .hasOwnProperty() with Object.hasOwn() for type checking |
| lib/schema.js | Replace .hasOwnProperty() with Object.hasOwn() across multiple path and option checks |
| lib/query.js | Replace .hasOwnProperty() with Object.hasOwn() for session and discriminator checks |
| lib/mongoose.js | Replace .hasOwnProperty() with Object.hasOwn() for model cache checks, also simplify optional chaining |
| lib/model.js | Replace .hasOwnProperty() with Object.hasOwn() for session and hook context checks |
| lib/helpers/update/applyTimestampsToUpdate.js | Replace .hasOwnProperty() with Object.hasOwn() for timestamp field check |
| lib/helpers/timestamps/setupTimestamps.js | Replace .hasOwnProperty() with Object.hasOwn() for currentTime option check |
| lib/helpers/setDefaultsOnInsert.js | Replace .hasOwnProperty() with Object.hasOwn() for path existence checks |
| lib/helpers/query/getEmbeddedDiscriminatorPath.js | Replace .hasOwnProperty() with Object.hasOwn() in array filter find |
| lib/helpers/projection/applyProjection.js | Replace .hasOwnProperty() with Object.hasOwn() for projection checks |
| lib/helpers/populate/removeDeselectedForeignField.js | Replace .hasOwnProperty() with Object.hasOwn() for projection check |
| lib/helpers/populate/modelNamesFromRefPath.js | Replace .hasOwnProperty() with Object.hasOwn() for virtuals check |
| lib/helpers/populate/getModelsMapForPopulate.js | Replace .hasOwnProperty() with Object.hasOwn() for virtual options checks |
| lib/helpers/model/castBulkWrite.js | Replace .hasOwnProperty() with Object.hasOwn() for discriminator check |
| lib/helpers/model/applyMethods.js | Replace .hasOwnProperty() with Object.hasOwn() for schema tree check |
| lib/helpers/indexes/isDefaultIdIndex.js | Replace .hasOwnProperty() with Object.hasOwn() for _id key check |
| lib/helpers/indexes/applySchemaCollation.js | Replace both instances of .hasOwnProperty() with Object.hasOwn() |
| lib/helpers/common.js | Replace .hasOwnProperty() with Object.hasOwn() for result property check |
| lib/drivers/node-mongodb-native/connection.js | Replace utils.object.hasOwnProperty() with Object.hasOwn() |
| lib/document.js | Replace .hasOwnProperty() with Object.hasOwn() across multiple state and path checks, refactor filter+forEach to for loop |
| lib/connection.js | Replace .hasOwnProperty() with Object.hasOwn() and utils.object.hasOwnProperty() |
| lib/cast.js | Replace .hasOwnProperty() with Object.hasOwn() for $comment path check |
| lib/aggregate.js | Replace .hasOwnProperty() with Object.hasOwn() for session option check |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@AbdelrahmanHafez I've opened a new pull request, #15867, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@AbdelrahmanHafez I've opened a new pull request, #15868, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.
Pull request overview
Copilot reviewed 30 out of 30 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
vkarpov15
left a comment
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.
Nice! Would you be able to make a similar PR for 8.x branch?
|
Sure, will do 👍 |
Summary
Replace
.hasOwnProperty()andObject.prototype.hasOwnProperty.call()withObject.hasOwn()Why
Object.hasOwn()is:Object.create(null)objects.call()to avoid prototype shadowing