-
-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
types: avoid unnecessary MergeType<> if TOverrides not set, clean up statics and insertMany() type issues #13577
Conversation
…statics and insertMany() type issues Fix #13529
to my knowledge |
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.
Looks good to me, also mostly compiles fine with current typegoose
currently typegoose defines HydratedDocument<Something, any>
, which makes it incompatible with HydratedDocument<Something, {}>
, changing to the following helps, but i dont know if this is the best
TOverrides extends Record<string, never> ?
Document<unknown, TQueryHelpers, DocType> & Require_id<DocType> :
IfAny<TOverrides, Document<unknown, TQueryHelpers, DocType> & Require_id<DocType>, Document<unknown, TQueryHelpers, DocType> & MergeType<
Require_id<DocType>,
IfAny<TOverrides, {}>
>>
another option (on typegoose's side) would be to remove the , any
from the HydratedDocument
call
The idea is to make |
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.
Looks good to me and compiles and runs without problems in the current latest typegoose
Thank you! |
Summary
#13529 points out some issues with
MergeType
, this PR makes it so that we avoid usingMergeType
inHydratedDocument<>
where possible.However, for a future release, we should remove HydratedDocument, and instead rely on inferring the hydrated document type from the schema. Inferring HydratedDocument type from the raw document type isn't possible in general, and relies on
Omit<>
, which is super brittle in TypeScript - breaks on private/protected fields, unions with generics, and other cases.Examples