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): allow populating deeply nested models as strings #11168

Merged
merged 10 commits into from
Jan 4, 2022

Conversation

AbdelrahmanHafez
Copy link
Collaborator

fixes #11160

@vkarpov15
This change means that we won't report the full path if a user uses populate: 'websiteId' because we can't assign a property to a string.

Can you think of a way to overcome this?
Even with this caveat, I think it's best we have populate in the first place, then we can work on improving the error message being reported.

@AbdelrahmanHafez AbdelrahmanHafez added this to the 6.1.5 milestone Jan 3, 2022
Copy link
Collaborator

@vkarpov15 vkarpov15 left a comment

Choose a reason for hiding this comment

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

Do if (subPopulate != null && typeof subPopulate === 'object')

Remember that typeof null === 'object'

lib/model.js Outdated Show resolved Hide resolved
@AbdelrahmanHafez
Copy link
Collaborator Author

@vkarpov15
Good catch, thanks.

@Kamikadze4GAME
Copy link
Contributor

@vkarpov15, at first I also thought that this check is needed. But then I looked around the code and realized that this check was unnecessary. Line 4614 already excludes falsy values. Is not it?

@vkarpov15
Copy link
Collaborator

@Kamikadze4GAME is right, thanks for pointing that out. I'll merge this and fix in master so we can get this out.

@vkarpov15 vkarpov15 merged commit 2350bff into Automattic:master Jan 4, 2022
vkarpov15 added a commit that referenced this pull request Jan 4, 2022
@AbdelrahmanHafez AbdelrahmanHafez deleted the gh-11160 branch January 6, 2022 12:43
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.

Mongoose 6: Deep-populate with string causing errors (undocumented breaking change?)
3 participants