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

Ensure we convert mongoose models to regular objects. Closes #110. #112

Merged
merged 1 commit into from
Aug 23, 2016

Conversation

ekryski
Copy link
Member

@ekryski ekryski commented Aug 23, 2016

No description provided.

@daffl
Copy link
Member

daffl commented Aug 23, 2016

Do we want to do that? This change might have weird side effects for people that are using models (e.g. with getters etc.). We could make a breaking release that sets lean: true by default or only calls toObject in those methods if lean: true (but then it should do remove as well).

@corymsmith
Copy link
Contributor

I had a similar thought as @daffl, are there implications / side effects to doing this?

@ekryski
Copy link
Member Author

ekryski commented Aug 23, 2016

@daffl @corymsmith no it's not an issue or at least no worse than what currently happens. Right now, we're not calling save on the data object. If this was a mongoose model and we did this that is what would trigger pre and post mongoose hooks. Since we're not doing that, it's just treated as a normal JS object and mongoose will return a mongoose model after the DB call (unless you're using lean: true), so it shouldn't affect getters or virtuals.

Currently if you pass a mongoose model it just blows up. Hence #110.

We don't need to do it for remove because there is no data being passed, (hence no model).

@daffl
Copy link
Member

daffl commented Aug 23, 2016

Oh sorry... I totally read that wrong. All good, :shipit:

@ekryski ekryski merged commit dbc3bae into master Aug 23, 2016
@ekryski ekryski deleted the fix-110 branch August 23, 2016 21:35
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.

3 participants