Skip to content

Conversation

vkarpov15
Copy link
Collaborator

Fix #14876

Summary

Right now doc.depopulate() just returns this, which may be incorrect if paths are depopulated. This PR adds generic parameter, similar to how doc.populate<{ user: UserType }>('user') returns document with user property updated, doc.depopulate<{ user: ObjectId }>('user') also updates the user property type.

Examples

@vkarpov15 vkarpov15 added this to the 8.6.3 milestone Sep 16, 2024
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.

LGTM, if i see this correctly, it will now merge the types instead of overwriting? example, instead of PopulatedType changing to ObjectId it is now PopulatedType | ObjectId?

@hasezoey hasezoey added the typescript Types or Types-test related issue / Pull Request label Sep 17, 2024
@vkarpov15 vkarpov15 merged commit 90f8af3 into master Sep 17, 2024
5 checks passed
@vkarpov15
Copy link
Collaborator Author

No, MergeType explicitly overwrites property type. So user property will be of type ObjectId, overwriting the populated type

@hasezoey
Copy link
Collaborator

nevermind, didnt see that in the last expectType's they were different objects, though they were the same (populatedCar, depopulatedCar)

@hasezoey hasezoey deleted the vkarpov15/gh-14876 branch September 17, 2024 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
typescript Types or Types-test related issue / Pull Request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Return type of document.depopulate() is still the populated document type
2 participants