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

Introduce a simpler way to "populate unless populated" #14979

Open
iliakan opened this issue Oct 22, 2024 · 7 comments
Open

Introduce a simpler way to "populate unless populated" #14979

iliakan opened this issue Oct 22, 2024 · 7 comments
Labels
enhancement This issue is a user-facing general improvement that doesn't fix a bug or add a new feature new feature This change adds new functionality, like a new method or class
Milestone

Comments

@iliakan
Copy link

iliakan commented Oct 22, 2024

🚀 Feature Proposal

Is there a method to populate only if the value isn't already there? If not, maybe it's worth adding?

Motivation

Currently, a populate call (both on a Model and an instance) queries the database, even if the value is already populated.

However, it's quite common that objects come from different code paths, and they may have paths populated already. So, performance-wise, we should not do it again.

For a single instance, we can write

if (!user.populated('friends')) {
  await user.populate('friends');
}

...But it's cumbersome to repeat this often.

Also, for an array of users we need to check each item, which makes even longer code compared to a single call, e.g. User.populateMissing(users, 'friends'), which could work just as User.populate(single query to fetch all users), but omit already-populated paths.

Additional thoughts

To say the truth, after many years of development with Mongoose, my experience is that the current behaviour (always querying the database) is much less needed than the lazy one (query only if populated path doesn't exist).

@iliakan iliakan added enhancement This issue is a user-facing general improvement that doesn't fix a bug or add a new feature new feature This change adds new functionality, like a new method or class labels Oct 22, 2024
@iliakan iliakan changed the title Does a method exist: "populate if not populated" Introduce a simpler way to "populate unless populated" Oct 22, 2024
@satyamkale27
Copy link

satyamkale27 commented Oct 25, 2024

@vkarpov15 I want to work on this feature please assign me.

@vkarpov15
Copy link
Collaborator

I agree with the point that if (!user.populated()) is cumbersome. A couple of potential APIs we can support:

  1. user.populate('friends', { forceRepopulate: false }) to opt out of automatic repopulating
  2. user.populateIfNecessary('friends')

I think I prefer option (1), and likely switch forceRepopulate to be false by default in Mongoose 9. I agree that user.populate('friends') likely shouldn't populate friends if it is already populated - you can use User.populate(user, 'friends') or user.populate('friends', { forceRepopulate: true }).

What do you think @iliakan ?

@vkarpov15 vkarpov15 added this to the 8.9 milestone Nov 12, 2024
@iliakan
Copy link
Author

iliakan commented Nov 12, 2024

@vkarpov15 The option flag looks like a good solution.

The good thing about it is that a global default setting can be added as well.

So people don't need to wait till Mongoose 9, they can set e.g. mongoose.set('forceRepopulate', false) and have this behavior enabled even in Mongoose 8. The next Mongoose can just change this default.

Will the collection variant look like this User.populate(user, 'friends', { forceRepopulate: false })?

@vkarpov15
Copy link
Collaborator

I was thinking we would just support forceRepopulate option on user.populate() (document variant) rather than User.populate() (model variant). User.populate() seems a bit more explicit. What do you think?

Agree 💯 on the global option.

@iliakan
Copy link
Author

iliakan commented Nov 13, 2024

I believe, both Model and instance methods deserve to have the "proper" populate mechanics. Both use cases are equally important.

Also, both instance and Model populate should work in the same way.

@vkarpov15
Copy link
Collaborator

@iliakan what do you think of #15044?

@iliakan
Copy link
Author

iliakan commented Nov 19, 2024

@vkarpov15 looks good to me, I'm not familiar with Mongoose internal population mechanics, but the tests seem ok.

Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This issue is a user-facing general improvement that doesn't fix a bug or add a new feature new feature This change adds new functionality, like a new method or class
Projects
None yet
Development

No branches or pull requests

3 participants