-
-
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
Add Path generic #13117
Add Path generic #13117
Conversation
It wasn't yelling at me
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.
For the future, please reference the closed issue in the commit message (#13070) in this case. And please avoid spelling mistakes in git commit messages.
}); | ||
|
||
const parent = model<IParent>('Parent', parentSchema); | ||
const child = model<IChild>('Child', childSchema); |
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.
We always use capitalized names for models
const child = model<IChild>('Child', childSchema); | ||
|
||
const doc = await parent.findOne(); | ||
await child.populate<{child: IChild}>(doc, 'child'); |
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.
Didn't actually test that the result doc is correctly populated. Missed this:
mongoose/test/types/populate.test.ts
Line 126 in 7d2c511
parents.map(p => p.child.name); |
@@ -303,7 +303,8 @@ declare module 'mongoose' { | |||
callback?: Callback<(HydratedDocument<T, TMethodsAndOverrides, TVirtuals>)[]>): Promise<Array<HydratedDocument<T, TMethodsAndOverrides, TVirtuals>>>; | |||
populate(doc: any, options: PopulateOptions | Array<PopulateOptions> | string, | |||
callback?: Callback<HydratedDocument<T, TMethodsAndOverrides, TVirtuals>>): Promise<HydratedDocument<T, TMethodsAndOverrides, TVirtuals>>; | |||
|
|||
populate<Paths={}>(docs: Array<any>, options: PopulateOptions | Array<PopulateOptions> | string): Promise<MergeType<this, Paths>>; |
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.
If you had actually tested the populate doc result, you'd see that this
is the wrong type to use here. In this context, this
is the Model class, not the document. Use the other populate()
calls on Model
as an example for how to do this correctly.
It wasn't yelling at me.
Also, while working on this issue I noticed the populate section is more focused on query population than calling
Model.populate()
. Maybe we should include a small section about that function since we have a section titledPopulate
in the docs.