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.

Sign up for GitHub

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

Jump to bottom

Fix: Populate query return type. #11560

Merged
merged 3 commits into from
Mar 27, 2022
Merged

Fix: Populate query return type. #11560

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4315ff5
Improve RawDocType returned by calling populate FN
mohammad0-0ahmad Mar 23, 2022
f286273
Refactor UnpackedIntersection type & some tests related to populate FN
mohammad0-0ahmad Mar 27, 2022
a955a08
Merge branch 'master' into #11532
vkarpov15 Mar 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
32 changes: 31 additions & 1 deletion test/types/populate.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,12 +170,14 @@ function gh11503() {
const User = model<IUser>('friends', userSchema);

User.findOne({}).populate('friends').then(user => {
expectType<Types.ObjectId | undefined>(user?.friends[0]);
if (!user) return;
expectType<Types.ObjectId>(user?.friends[0]);
expectError(user?.friends[0].blocked);
expectError(user?.friends.map(friend => friend.blocked));
});

User.findOne({}).populate<{ friends: Friend[] }>('friends').then(user => {
if (!user) return;
expectAssignable<Friend>(user?.friends[0]);
expectType<boolean>(user?.friends[0].blocked);
const firstFriendBlockedValue = user?.friends.map(friend => friend)[0];
Expand All @@ -197,4 +199,32 @@ function gh11544() {
User.findOne({}).populate({ path: 'friends', strictPopulate: false });
User.findOne({}).populate({ path: 'friends', strictPopulate: true });
User.findOne({}).populate({ path: 'friends', populate: { path: 'someNestedPath', strictPopulate: false } });
}

async function _11532() {
interface IParent {
name: string;
child: Types.ObjectId;
}
interface IChild {
name: string;
}

const parentSchema = new Schema(
{
name: { type: String, required: true },
child: { type: Schema.Types.ObjectId, ref: 'Child', required: true }
});

const parent = model<IParent>('Parent', parentSchema);

const populateQuery = parent.findOne().populate<{ child: IChild }>('child');
const populateResult = await populateQuery;
const leanResult = await populateQuery.lean();

if (!populateResult) return;
expectType<string>(populateResult.child.name);

if (!leanResult) return;
expectType<string>(leanResult.child.name);
}
6 changes: 3 additions & 3 deletions types/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1471,7 +1471,7 @@ declare module 'mongoose' {

type QueryWithHelpers<ResultType, DocType, THelpers = {}, RawDocType = DocType> = Query<ResultType, DocType, THelpers, RawDocType> & THelpers;

type UnpackedIntersection<T, U> = T extends (infer A)[]
type UnpackedIntersection<T, U> = T extends null ? null : T extends (infer A)[]
? (Omit<A, keyof U> & U)[]
: keyof U extends never
? T
Expand Down Expand Up @@ -1776,8 +1776,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

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!!

populate<Paths = {}, TRawDocType = UnpackedIntersection<ResultType, Paths>>(options: PopulateOptions | (PopulateOptions | string)[]): QueryWithHelpers<TRawDocType, DocType, THelpers, TRawDocType>;

/** Get/set the current projection (AKA fields). Pass `null` to remove the current projection. */
projection(): ProjectionFields<DocType> | null;
Expand Down