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

use .lean() liberally in fetch's? #51

Closed
jamesjnadeau opened this issue Feb 9, 2016 · 7 comments
Closed

use .lean() liberally in fetch's? #51

jamesjnadeau opened this issue Feb 9, 2016 · 7 comments

Comments

@jamesjnadeau
Copy link
Contributor

I would argue that a lot of the time, you don't need to fetch a full mongoose doc, and fetching a regular object without the mongoose overhead would be a great performance feature/optimization.

I think there should be a query param called $lean that defaults to being set to true.

When $lean is false, real mongoose objects are created, and this note would still actually apply:
http://docs.feathersjs.com/databases/mongoose.html#modifying-results-with-the-toobject-hook

This way it would highlight what to do if you need to actually work with the mongoose object, but still provide the most performance by default.

Just an idea I had while looking through the source and from my own experience. Feel free to ignore, I won't have time/energy to work on anytime soon.

Thanks for this awesome project guys! Good work! 👍 🚀 🎸

@marshallswain
Copy link
Member

I find that with all of the functionality that hooks give you, the only feature of Mongoose that I actually use is the schema enforcement. It makes perfect sense for me to default to using lean.

@daffl
Copy link
Member

daffl commented Feb 9, 2016

Does that mean that you get faster fetches but the schema is still being enforced when someone creates a new instance? That sounds good to me.

@ekryski
Copy link
Member

ekryski commented Feb 9, 2016

hmmmm. Not gonna lie my gut reaction was "just use the mongo adapter" (if it were up to date 💩 😝) but I like this idea of defaulting to lean queries.

It looks like .lean() just prunes out all the virtuals, instance methods, static methods, etc. so it does make it quite a bit faster. My concern is are hooks enough to cover this, specifically in the case of virtuals?

I'm thinking that rather than passing on a per query basis you pass {lean: false} as an option when initializing a mongoose service instead.

I'm wrestling with the idea of this being a special query param. It's definitely more flexible this way but I think we really need to add special query params sparingly because if we introduce a bunch that are database specific it weakens our ability to query with a unified interface. Most of the database specific config options currently happen at initialization.

Do you guys think it's common that you'd have times where you'd want lean and not lean queries from a single service? cc/ @jamesjnadeau

Apologies for the long comment....

@daffl
Copy link
Member

daffl commented Feb 9, 2016

If we make { lean: true } an options flag (instead of the default) we don't even have to publish a new major version and it could go in fairly quick.

@ekryski
Copy link
Member

ekryski commented Feb 9, 2016

Ya I actually like that better because then it is explicit that you are deviating from how mongoose behaves by default. Otherwise, I could see us getting questions about why virtuals and instance methods aren't available in hooks.

@jamesjnadeau
Copy link
Contributor Author

👍 to the options flag, better way of going about this. Doesn't really matter to me if it's on or off by default.

You can always call Model.hydrate(object) later to get it back to a mongoose doc if you need it.

For ease of use, might be best to leave the full mongoose docs there unless you choose this setting.

@ekryski
Copy link
Member

ekryski commented Feb 9, 2016

@jamesjnadeau good to know. We should document that Model.hydrate call.

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

No branches or pull requests

4 participants