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

Feature/ts type improvements #11650

Merged
merged 13 commits into from
Apr 28, 2022
Merged

Feature/ts type improvements #11650

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
a4532e7
d.ts: LeanDocument improvement: strip out functions from lean type
taxilian Apr 8, 2022
d2f9cc9
d.ts: Minor optimization so that `any` types don't go down a rabbithole
taxilian Apr 9, 2022
a16d077
d.ts: Optimize LeanDocument to get source type of Subdocuments if pos…
taxilian Apr 9, 2022
bac4a82
d.ts: Optimize LeanArray to infer underlying type when possible
taxilian Apr 9, 2022
659436d
d.ts: Optimize LeanDocument to infer underlying type when possible
taxilian Apr 9, 2022
2210de8
d.ts: Update Query types so helpers are path-aware, rely on FilterQuery
taxilian Apr 9, 2022
2594b79
Clean up toJSON, toObject -- remove redundant methods
taxilian Apr 11, 2022
a9792a0
Remove "strip functions" aspect of LeanDocument per PR feedback, will…
taxilian Apr 11, 2022
2423440
Improve LeanDocument type inference to better support union types
taxilian Apr 11, 2022
0ae960b
Fix broken type unit tests
taxilian Apr 11, 2022
e099a59
Add a test to ensure that #10293 has not recurred
taxilian Apr 11, 2022
e4b007b
Merge branch 'master' into feature/tsTypeImprovements
vkarpov15 Apr 28, 2022
9534282
fix typo
vkarpov15 Apr 28, 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
12 changes: 9 additions & 3 deletions types/document.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ declare module 'mongoose' {
/** A list of paths to skip. If set, Mongoose will validate every modified path that is not in this list. */
type pathsToSkip = string[] | string;

/**
* Generic types for Document:
* * T - the type of _id
* * TQueryHelpers - Object with any helpers that should be mixed into the Query type
* * DocType - the type of the actual Document created
*/
class Document<T = any, TQueryHelpers = any, DocType = any> {
constructor(doc?: any);

Expand Down Expand Up @@ -216,12 +222,12 @@ declare module 'mongoose' {
set(value: any): this;

/** The return value of this method is used in calls to JSON.stringify(doc). */
toJSON(options: ToObjectOptions & { flattenMaps: false }): LeanDocument<this>;
toJSON(options?: ToObjectOptions): FlattenMaps<LeanDocument<this>>;
toJSON(options: ToObjectOptions & { flattenMaps: false }): LeanDocument<DocType>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good change 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, I thought it was, but it turns out it breaks a few cases -- specifically cases where someone extends Document.

I'm still working through the tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After some tinkering I've made the following changes to toJSON and toObject:

  • I removed the redundant signatures -- the generic version has a default template type, so there is no need to specify a type without it, that just confuses things and could throw off type inference in some cases
  • I have left it using LeanDocument, though that can be overridden with a template.

The downside to this is that if your interface extends Document instead of using the new recommended way then you'll get a any type out of both methods unless you manually provide a type to the generic.

On the positive side, though, I could not find any way to use this in the return type which did not cause circular reference issues with my code in multiple places -- this change fixes that. Additionally, the "worst case" is that you get an any type out, so it won't break existing code it just may not catch issues that someone would expect it to catch. That can be mitigated by either migrating to the recommended method or manually providing the type.

toJSON(options?: ToObjectOptions): FlattenMaps<LeanDocument<DocType>>;
toJSON<T = FlattenMaps<DocType>>(options?: ToObjectOptions): T;

/** Converts this document into a plain-old JavaScript object ([POJO](https://masteringjs.io/tutorials/fundamentals/pojo)). */
toObject(options?: ToObjectOptions): LeanDocument<this>;
toObject(options?: ToObjectOptions): LeanDocument<DocType>;
toObject<T = DocType>(options?: ToObjectOptions): T;

/** Clears the modified state on the specified path. */
Expand Down
77 changes: 58 additions & 19 deletions types/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1482,6 +1482,26 @@ declare module 'mongoose' {

type ProjectionFields<DocType> = { [Key in keyof Omit<LeanDocument<DocType>, '__v'>]?: any } & Record<string, any>;

type _QueryManyForModel<T, ResultDoc, TQueryHelpers = {}> =
QueryWithHelpers<Array<ResultDoc>, ResultDoc, TQueryHelpers, T>;
type _QueryOneForModel<T, ResultDoc, TQueryHelpers = {}> =
QueryWithHelpers<ResultDoc, ResultDoc, TQueryHelpers, T>;

/**
* Helper type for getting a find Query type for a given Model (return of Model.find)
*/
type QueryForModel<T extends Model<any>> =
T extends Model<infer MT, infer TQueryHelpers, infer TMethodsAndOverrides, infer TVirtuals> ?
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm very wary of using infer because it has caused some serious performance issues for us in the past. Can you please provide an example of why this type is helpful?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just a helper utility type, so you can always not use it if it causes performance issues. Example of use would just be any time you need to return a query or create one but want to assign it to a variable declared earlier.

How I was using it:

function applyQueryRestrictions(userQ: mongoose.QueryForModel<typeof UserModel>, requiredPerm?: string) {
    if (requiredPerm) {
        userQ.where('permissions', requiredPerm);
    }
    userQ.select('passInvalid username call first last email modified permissions')
        .select('verifiedCall.callsign verifiedCall.displayName verifiedCall.sigdate')
        .select('providers.type');
    userQ.populate({
        path: 'veDocs',
        select: 'isTeamLead active deleted number display_name vec call',
        populate: {
            path: 'ulsRecord',
            select: 'callsign frn license_class expired_date',
        }
    });
    return userQ;
}

I certainly could have constructed the query differently, but I wanted to keep type checking as much as possible and it's much easier to do QueryForModel<MyModel> than it is to do QueryWithHelpers<ResultDoc[], ResultDoc, {}, DocType> -- not to mention that you have to figure out what all of those are. My original plan was to just do type MyQueryType = ReturnType<typeof MyModel.find> but since the return type of the different .find overrides aren't all identical that doesn't return a useful type.

Since this is just a utility method I can pull it and put it in my own code -- just seemed like something that others would also find useful and since it's purely optional it didn't seem like something which would cost anything to put in. Your call, just let me know.

_QueryManyForModel<MT, HydratedDocument<MT, TMethodsAndOverrides, TVirtuals>, TQueryHelpers>
: never;
/**
* Helper type for getting a findOne Query type for a given Model (return of Model.findOne)
*/
type QueryOneForModel<T extends Model<any>> =
T extends Model<infer MT, infer TQueryHelpers, infer TMethodsAndOverrides, infer TVirtuals> ?
_QueryOneForModel<MT, HydratedDocument<MT, TMethodsAndOverrides, TVirtuals>, TQueryHelpers>
: never;

class Query<ResultType, DocType, THelpers = {}, RawDocType = DocType> {
_mongooseOptions: MongooseQueryOptions;

Expand Down Expand Up @@ -1583,7 +1603,7 @@ declare module 'mongoose' {

/** Specifies a `$elemMatch` query condition. When called with one argument, the most recent path passed to `where()` is used. */
elemMatch(val: Function | any): this;
elemMatch(path: string, val: Function | any): this;
elemMatch<KEY extends keyof FilterQuery<DocType>>(path: KEY, val: FilterQuery<DocType>[KEY]['$elemMatch']): this;

/**
* Gets/sets the error flag on this query. If this flag is not null or
Expand All @@ -1600,7 +1620,7 @@ declare module 'mongoose' {

/** Specifies a `$exists` query condition. When called with one argument, the most recent path passed to `where()` is used. */
exists(val: boolean): this;
exists(path: string, val: boolean): this;
exists<KEY extends keyof FilterQuery<DocType>>(path: KEY, val: FilterQuery<DocType>[KEY]['$exists']): this;

/**
* Sets the [`explain` option](https://docs.mongodb.com/manual/reference/method/cursor.explain/),
Expand Down Expand Up @@ -1680,18 +1700,18 @@ declare module 'mongoose' {

/** Specifies a `$gt` query condition. When called with one argument, the most recent path passed to `where()` is used. */
gt(val: number): this;
gt(path: string, val: number): this;
gte<KEY extends keyof FilterQuery<DocType>>(path: KEY, val: FilterQuery<DocType>[KEY]['$gt']): this;
Copy link
Contributor

@iammola iammola May 2, 2022 •
edited

Choose a reason for hiding this comment

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

Hey, I was just reading through the diffs and noticed this minor error.

You changed the name of the function. From gt to gte and there is now only one overload for the gt function.

@vkarpov15 @taxilian

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ack! You're right, good catch! @vkarpov15 very sorry about that! I'm assuming you'll want to just fix it since it's merged already? Let me know if you need me for anything

Copy link
Collaborator

Choose a reason for hiding this comment

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

covered by #11760


/** Specifies a `$gte` query condition. When called with one argument, the most recent path passed to `where()` is used. */
gte(val: number): this;
gte(path: string, val: number): this;
gte<KEY extends keyof FilterQuery<DocType>>(path: KEY, val: FilterQuery<DocType>[KEY]['$gte']): this;

/** Sets query hints. */
hint(val: any): this;

/** Specifies an `$in` query condition. When called with one argument, the most recent path passed to `where()` is used. */
in(val: Array<any>): this;
in(path: string, val: Array<any>): this;
in<KEY extends keyof FilterQuery<DocType>>(path: KEY, val: FilterQuery<DocType>[KEY]['$in']): this;

/** Declares an intersects query for `geometry()`. */
intersects(arg?: any): this;
Expand All @@ -1707,11 +1727,11 @@ declare module 'mongoose' {

/** Specifies a `$lt` query condition. When called with one argument, the most recent path passed to `where()` is used. */
lt(val: number): this;
lt(path: string, val: number): this;
lt<KEY extends keyof FilterQuery<DocType>>(path: KEY, val: FilterQuery<DocType>[KEY]['$lt']): this;

/** Specifies a `$lte` query condition. When called with one argument, the most recent path passed to `where()` is used. */
lte(val: number): this;
lte(path: string, val: number): this;
lte<KEY extends keyof FilterQuery<DocType>>(path: KEY, val: FilterQuery<DocType>[KEY]['$lte']): this;

/**
* Runs a function `fn` and treats the return value of `fn` as the new value
Expand All @@ -1738,7 +1758,7 @@ declare module 'mongoose' {

/** Specifies a `$mod` condition, filters documents for documents whose `path` property is a number that is equal to `remainder` modulo `divisor`. */
mod(val: Array<number>): this;
mod(path: string, val: Array<number>): this;
mod<KEY extends keyof FilterQuery<DocType>>(path: KEY, val: FilterQuery<DocType>[KEY]['$mod']): this;

/** The model this query was created from */
model: typeof Model;
Expand All @@ -1751,15 +1771,15 @@ declare module 'mongoose' {

/** Specifies a `$ne` query condition. When called with one argument, the most recent path passed to `where()` is used. */
ne(val: any): this;
ne(path: string, val: any): this;
ne<KEY extends keyof FilterQuery<DocType>>(path: KEY, val: FilterQuery<DocType>[KEY]['$ne']): this;

/** Specifies a `$near` or `$nearSphere` condition */
near(val: any): this;
near(path: string, val: any): this;
near<KEY extends keyof FilterQuery<DocType>>(path: KEY, val: FilterQuery<DocType>[KEY]['$near']): this;

/** Specifies an `$nin` query condition. When called with one argument, the most recent path passed to `where()` is used. */
nin(val: Array<any>): this;
nin(path: string, val: Array<any>): this;
nin<KEY extends keyof FilterQuery<DocType>>(path: KEY, val: FilterQuery<DocType>[KEY]['$nin']): this;

/** Specifies arguments for an `$nor` condition. */
nor(array: Array<FilterQuery<DocType>>): this;
Expand Down Expand Up @@ -1795,7 +1815,7 @@ declare module 'mongoose' {

/** Specifies a `$regex` query condition. When called with one argument, the most recent path passed to `where()` is used. */
regex(val: string | RegExp): this;
regex(path: string, val: string | RegExp): this;
regex<KEY extends keyof FilterQuery<DocType>>(path: KEY, val: FilterQuery<DocType>[KEY]['$regex']): this;

/**
* Declare and/or execute this query as a remove() operation. `remove()` is
Expand Down Expand Up @@ -1848,7 +1868,7 @@ declare module 'mongoose' {

/** Specifies an `$size` query condition. When called with one argument, the most recent path passed to `where()` is used. */
size(val: number): this;
size(path: string, val: number): this;
size<KEY extends keyof FilterQuery<DocType>>(path: KEY, val: FilterQuery<DocType>[KEY]['$size']): this;

/** Specifies the number of documents to skip. */
skip(val: number): this;
Expand Down Expand Up @@ -2084,13 +2104,30 @@ declare module 'mongoose' {
type TreatAsPrimitives = actualPrimitives |
NativeDate | RegExp | symbol | Error | BigInt | Types.ObjectId;

// This will -- when possible -- extract the original type of the subdocument in question
type LeanSubdocument<T> = T extends (Types.Subdocument<Require_id<T>['_id']> & infer U) ? LeanDocument<U> : Omit<LeanDocument<T>, '$isSingleNested' | 'ownerDocument' | 'parent'>;

type LeanType<T> =
0 extends (1 & T) ? T : // any
T extends TreatAsPrimitives ? T : // primitives
T extends Types.Subdocument ? Omit<LeanDocument<T>, '$isSingleNested' | 'ownerDocument' | 'parent'> : // subdocs
T extends Types.Subdocument ? LeanSubdocument<T> : // subdocs
LeanDocument<T>; // Documents and everything else

type LeanArray<T extends unknown[]> = T extends unknown[][] ? LeanArray<T[number]>[] : LeanType<T[number]>[];
// Used only when collapsing lean arrays for ts performance reasons:
type LeanTypeOrArray<T> = T extends unknown[] ? LeanArray<T> : LeanType<T>;
taxilian marked this conversation as resolved.
Show resolved Hide resolved

type LeanArray<T extends unknown[]> =
// By checking if it extends Types.Array we can get the original base type before collapsing down,
// rather than trying to manually remove the old types. This matches both Array and DocumentArray
T extends Types.Array<infer U> ? LeanTypeOrArray<U>[] :
// If it isn't a custom mongoose type we fall back to "do our best"
T extends unknown[][] ? LeanArray<T[number]>[] : LeanType<T[number]>[];

type _WithoutFunctions<T extends {}> = {
[K in keyof T]: [0] extends [1 & T[K]] ? K : // keep any
T[K] extends Function ? never : K;
};
type StripFunctions<T extends {}> = Pick<T, _WithoutFunctions<T>[keyof T]>;
taxilian marked this conversation as resolved.
Show resolved Hide resolved

export type _LeanDocument<T> = {
[K in keyof T]: LeanDocumentElement<T[K]>;
Expand All @@ -2100,9 +2137,10 @@ declare module 'mongoose' {
// This way, the conditional type is distributive over union types.
// This is required for PopulatedDoc.
type LeanDocumentElement<T> =
T extends unknown[] ? LeanArray<T> : // Array
T extends Document ? LeanDocument<T> : // Subdocument
T;
0 extends (1 & T) ? T :// any
T extends unknown[] ? LeanArray<T> : // Array
T extends Document ? LeanDocument<T> : // Subdocument
T;

export type SchemaDefinitionType<T> = T extends Document ? Omit<T, Exclude<keyof Document, '_id' | 'id' | '__v'>> : T;

Expand All @@ -2111,7 +2149,8 @@ declare module 'mongoose' {
* Plain old JavaScript object documents (POJO).
* @see https://mongoosejs.com/docs/tutorials/lean.html
*/
export type LeanDocument<T> = Omit<_LeanDocument<T>, Exclude<keyof Document, '_id' | 'id' | '__v'> | '$isSingleNested'>;
export type LeanDocument<T> = T extends mongoose.HydratedDocument<infer DocType, infer MethodsOverrides, infer TVirtuals> ? _LeanDocument<StripFunctions<DocType>> :
Omit<_LeanDocument<StripFunctions<T>>, Exclude<keyof Document, '_id' | 'id' | '__v'> | '$isSingleNested'>;

export type LeanDocumentOrArray<T> = 0 extends (1 & T) ? T :
T extends unknown[] ? LeanDocument<T[number]>[] :
Expand Down