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

BREAKING CHANGE: apply minimize by default when updating document #13843

Merged
merged 3 commits into from Sep 26, 2023

Conversation

vkarpov15
Copy link
Collaborator

Fix #13782

Summary

This change is small code-wise, but significant and worth thinking about carefully. The docs on minimize say that Mongoose will, by default, "minimize" schemas by removing empty objects. but don't specify exactly when minimize is applied.

Right now, Mongoose only applies minimize() when $toObject() is called. So that means when you save() a new doc, Mongoose applies minimize() unless you set minimize: false option in your schema. However, if you try to update an existing doc's nested or subdocument property, Mongoose will currently not apply minimize: Mongoose will store an empty object in MongoDB. However, if you update a document array on an existing document, Mongoose will minimize the document array.

This behavior is inconsistent. This PR makes it so that Mongoose applies minimize() when updating an existing document with save(). However, I'm a bit worried about changing this behavior; I'm worried of unforeseen consequences. Any thoughts @hasezoey @AbdelrahmanHafez ?

Examples

@vkarpov15 vkarpov15 added this to the 8.0 milestone Sep 8, 2023
Copy link
Collaborator

@hasezoey hasezoey left a comment

Choose a reason for hiding this comment

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

Looks and sounds good to me, as long as all the tests still pass and this change is mentioned in the changelog, hopefully this wont turn into another strictQuery thing

lib/helpers/minimize.js Outdated Show resolved Hide resolved
Co-authored-by: hasezoey <hasezoey@gmail.com>
@vkarpov15
Copy link
Collaborator Author

Yeah, the strictQuery issue is exactly what I'm worried about with this PR. I'll think a little more about it, but I think in the interest of learning our lessons from strictQuery, we shouldn't hesitate to remove this behavior if it causes strictQuery-like problems.

@vkarpov15 vkarpov15 merged commit d5e9176 into 8.0 Sep 26, 2023
34 checks passed
@vkarpov15 vkarpov15 deleted the vkarpov15/gh-13782 branch September 26, 2023 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants