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

Fix: Populate query return type. #11560

Merged
merged 3 commits into from
Mar 27, 2022

Conversation

mohammad0-0ahmad
Copy link
Contributor

@mohammad0-0ahmad mohammad0-0ahmad commented Mar 23, 2022

Regarding #11532 #11518 #11585

@mohammad0-0ahmad mohammad0-0ahmad force-pushed the #11532 branch 2 times, most recently from 478d856 to 470ac4e Compare March 26, 2022 23:23
@mohammad0-0ahmad mohammad0-0ahmad changed the title Fix: Lean query results lose populated types Fix: Populate query return type. Mar 27, 2022
@vkarpov15 vkarpov15 added this to the 6.2.9 milestone Mar 27, 2022
@vkarpov15 vkarpov15 merged commit c2103cb into Automattic:master Mar 27, 2022
@@ -1774,8 +1774,8 @@ declare module 'mongoose' {
polygon(path: string, ...coordinatePairs: number[][]): this;

/** Specifies paths which should be populated with other documents. */
populate<Paths = {}>(path: string | string[], select?: string | any, model?: string | Model<any, THelpers>, match?: any): QueryWithHelpers<UnpackedIntersection<ResultType, Paths>, DocType, THelpers, RawDocType>;
populate<Paths = {}>(options: PopulateOptions | (PopulateOptions | string)[]): QueryWithHelpers<UnpackedIntersection<ResultType, Paths>, DocType, THelpers, RawDocType>;
populate<Paths = {}, TRawDocType = UnpackedIntersection<ResultType, Paths>>(path: string | string[], select?: string | any, model?: string | Model<any, THelpers>, match?: any): QueryWithHelpers<TRawDocType, DocType, THelpers, TRawDocType>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ResultType & RawDocType type arguments are different. The ResultType is like a mongoose object, while the RawDocType is a POJO of your schema.

So the lean method uses the RawDocType to correctly specify the return type, so you shouldn't use the UnpackedIntersection<ResultType, Paths> for both arguments to the QueryWithHelpers generic type.

So the only replacement should be the RawDocType argument to the QueryWitHelpers with UnpackedIntersection<RawDocType, Paths>, for both overloads.

I just had this issue and wanted to open a PR before seeing yours, hopefully they'll merge it now.

@vkarpov15

Suggested change
populate<Paths = {}, TRawDocType = UnpackedIntersection<ResultType, Paths>>(path: string | string[], select?: string | any, model?: string | Model<any, THelpers>, match?: any): QueryWithHelpers<TRawDocType, DocType, THelpers, TRawDocType>;
populate<Paths = {}>(path: string | string[], select?: string | any, model?: string | Model<any, THelpers>, match?: any): QueryWithHelpers<UnpackedIntersection<ResultType, Paths>, DocType, THelpers, UnpackedIntersection<RawDocType, Paths>>;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought I had submitted this review 2 days ago when I got this issue as well 🤦‍♂️. It's my first review and I didn't know I had to submit the review after starting it again.

Am I wrong to think it's better to use the RawDocType in the UnpackedIntersection for the 4th argument? Because using the ResultType, you get the properties of a Mongoose Document. @vkarpov15

Copy link
Contributor Author

@mohammad0-0ahmad mohammad0-0ahmad Mar 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your comment @iammola, You are correct, and sorry for this mistake.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's all good man. Thanks for the PR!!

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