-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
Feature/ts lean missing doc fixes #11761 without killing LeanDocument #11769
Feature/ts lean missing doc fixes #11761 without killing LeanDocument #11769
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A quick suggestion to hopefully make changes backwards compatible.
types/document.d.ts
Outdated
@@ -225,7 +225,7 @@ declare module 'mongoose' { | |||
toJSON<T = DocType>(options: ToObjectOptions & { flattenMaps: false }): LeanDocument<T>; | |||
|
|||
/** Converts this document into a plain-old JavaScript object ([POJO](https://masteringjs.io/tutorials/fundamentals/pojo)). */ | |||
toObject<T = DocType>(options?: ToObjectOptions): LeanDocument<T>; | |||
toObject<T = DocType>(options?: ToObjectOptions): Require_id<LeanDocument<T>>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the interest of backwards compat, why not make this just toObject<T = LeanDocument<DocType>>
and return Require_id<T>
? That should allow people to bypass going through LeanDocument
, which seems to be causing part of the problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely an option; by doing that you will not be converting any mongoose.Types.Array types to real javascript arrays and any embedded subdocuments won't get converted to POJOs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I guess that's my way of saying "it's up to you", but also answering "why not" =]
@vkarpov15 I added one more quick change -- .lean was not actually applying LeanDocument, which resulted in lean not actually propagating and fixing things like the arrays, subdocuments, etc. |
Summary
Per #11761
.toObject
doesn't add_id
to the document if it is missing; this fixes that.Examples
See unit tests in the code