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

fix(document): make sure depopulate does not convert hydrated arrays to vanilla arrays #14803

Merged
merged 1 commit into from
Aug 13, 2024

Conversation

vkarpov15
Copy link
Collaborator

Summary

One issue that popped up while I was working on #1635 is that depopulate() converts Mongoose arrays to vanilla JavaScript arrays. So band.depopulate('members'); band.members.addToSet(id); will error out because band.members is no longer a Mongoose array.

The approach I came up with is to use getValue(): if getValue() returns a Mongoose array, then we just modify the array in-place to avoid triggering setters. If not a Mongoose array, then we have a case where mpath is returning an array of values underneath a document array, like in the #10592 test case doc.depopulate('docArr.members'), so we fall back to setValue().

I left assert.ok(!band.embeddedMembers[0].$populated('member')); commented out because that assertion fails, but I want to look at it some more when we come back to #1635.

Examples

assert.ok(!band.embeddedMembers[0].member.name);
// assert.ok(!band.embeddedMembers[0].$populated('member'));
Copy link
Collaborator

Choose a reason for hiding this comment

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

I left assert.ok(!band.embeddedMembers[0].$populated('member')); commented out because that assertion fails, but I want to look at it some more when we come back to #1635.

was this working before the PR?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I confirmed this assertion fails before this PR. We will need to do some extra work to make sure depopulate() also updates subdocs' $populated().

@vkarpov15 vkarpov15 merged commit 2291182 into master Aug 13, 2024
46 checks passed
@vkarpov15 vkarpov15 deleted the vkarpov15/gh-1635 branch August 13, 2024 18:42
@vkarpov15 vkarpov15 restored the vkarpov15/gh-1635 branch August 13, 2024 18:42
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.

2 participants