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

TypeScript: Type improvements for filterQuery, LeanDocument #11673

Closed
wants to merge 15 commits into from
Closed

TypeScript: Type improvements for filterQuery, LeanDocument #11673

wants to merge 15 commits into from

Conversation

taxilian
Copy link
Contributor

Summary

This PR enables useful types on FilterQuery -- currently FilterQuery knows field names but not types. It has a "LoosenType" typescript helper which allows automatic mongoose typecasting for things like ObjectId/String, Date/number, etc, and it also filters out functions. I've added a lot of tests to make sure everything is working the way we expect it to.

Note that this PR is based on #11650 and can't be merged separately

Examples

See the updated types/ unit tests

@@ -27,7 +27,7 @@ ParentModel.findOne({}).populate('child').orFail().then((doc: Parent & Document)
} else {
useChildDoc(child);
}
const lean = doc.toObject();
const lean = doc.toObject<Parent>();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do tests fail without this change? I'd prefer if we didn't require devs to add generics to every single toObject() call

/**
* Helper type for getting a find Query type for a given Model (return of Model.find)
*/
type QueryForModel<T extends Model<any>> =
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does this type do? Doesn't look like it is used

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was added in #11650 -- there is an open question in the other branch on if you want me to pull it out. I find it to be a useful helper for getting the type of Query but it can be removed

type LoosenType<R> = R extends infer T ?
[T] extends [string] ? T | RegExp :
[T] extends [ObjectIdLike] ? T | ObjectIdLike | { _id: ObjectIdLike | string } | string :
[T] extends [globalThis.Date] ? T | number :
Copy link
Collaborator

Choose a reason for hiding this comment

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

We do also allow certain strings for dates

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Easily added, I'll do that

@AbdelrahmanHafez
Copy link
Collaborator

Great work, I was always wondering when/if we'll be able to support types casting and still get type safety.
Thanks @taxilian 👍

@vkarpov15
Copy link
Collaborator

Realistically, I'm gonna have to close this. We had to revert most of #11650 because of performance issues.

@vkarpov15 vkarpov15 closed this Aug 1, 2022
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.

None yet

3 participants