Skip to content

Commit

Permalink
Fix one case of the reported issues.
Browse files Browse the repository at this point in the history
  • Loading branch information
@mohammad0-0ahmad
mohammad0-0ahmad committed Jul 4, 2022
1 parent 01c556a commit d092fdc
Showing 1 changed file with 18 additions and 4 deletions.
22 changes: 18 additions & 4 deletions types/inferschematype.d.ts
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,17 @@
import { Schema, InferSchemaType, SchemaType, SchemaTypeOptions, TypeKeyBaseType, Types, NumberSchemaDefinition, StringSchemaDefinition, BooleanSchemaDefinition, DateSchemaDefinition } from 'mongoose';
import {
Schema,
InferSchemaType,
SchemaType,
SchemaTypeOptions,
TypeKeyBaseType,
Types,
NumberSchemaDefinition,
StringSchemaDefinition,
BooleanSchemaDefinition,
DateSchemaDefinition,
ObtainDocumentType,
DefaultTypeKey
} from 'mongoose';

declare module 'mongoose' {
/**
Expand Down Expand Up @@ -138,7 +151,8 @@ type ObtainDocumentPathType<PathValueType, TypeKey extends TypeKeyBaseType> = Pa
? InferSchemaType<PathValueType>
: ResolvePathType<
PathValueType extends PathWithTypePropertyBaseType<TypeKey> ? PathValueType[TypeKey] : PathValueType,
PathValueType extends PathWithTypePropertyBaseType<TypeKey> ? Omit<PathValueType, TypeKey> : {}
PathValueType extends PathWithTypePropertyBaseType<TypeKey> ? Omit<PathValueType, TypeKey> : {},
TypeKey
>;

/**
Expand All @@ -153,7 +167,7 @@ type PathEnumOrString<T extends SchemaTypeOptions<string>['enum']> = T extends (
* @param {Options} Options Document definition path options except path type.
* @returns Number, "Number" or "number" will be resolved to string type.
*/
type ResolvePathType<PathValueType, Options extends SchemaTypeOptions<PathValueType> = {}> =
type ResolvePathType<PathValueType, Options extends SchemaTypeOptions<PathValueType> = {}, TypeKey extends TypeKeyBaseType = DefaultTypeKey> =
PathValueType extends Schema ? InferSchemaType<PathValueType> :
PathValueType extends (infer Item)[] ? IfEquals<Item, never, any, ResolvePathType<Item>>[] :
PathValueType extends StringSchemaDefinition ? PathEnumOrString<Options['enum']> :
Expand All @@ -169,5 +183,5 @@ type ResolvePathType<PathValueType, Options extends SchemaTypeOptions<PathValueT
IfEquals<PathValueType, ObjectConstructor> extends true ? any:
IfEquals<PathValueType, {}> extends true ? any:
PathValueType extends typeof SchemaType ? PathValueType['prototype'] :
PathValueType extends {} ? PathValueType :
PathValueType extends Record<string, any> ? ObtainDocumentType<PathValueType, any, TypeKey> :

This comment has been minimized.

Sign in to view

Copy link
@iammola

iammola Jul 28, 2022

Hey @mohammad0-0ahmad

This is a pretty lengthy explanation of an issue I'm having that could've been an issue on the mongoose repo, but I prefer to show It to you and if you're getting the issue as well, I'll create a proper issue.

I just updated a project of mine to https://github.com/Automattic/mongoose/releases/tag/6.5.0. You did really good work on the auto-typed virtuals!!

This is the schema, and it is auto-typed like I expected it to, when I hover over it,

const TermSchema = new Schema({
    session: { type: Schema.Types.ObjectId, required: true, ref: "..." },
    start: { type: Date, required: true, unique: true },
    end: { type: Date, required: true, unique: true },
  });

// Hovered TS type
// const TermSchema: Schema<any, Model<any, any, any, any, any>, {}, {}, {}, {}, "type", {
//    session: Types.ObjectId;
//    start: Date;
//    end: Date;
// }>

But when I make a model with the schema above, it's not as I expected and it's the same with the InferSchemaType helper.

const TermModel = model("...", TermSchema);

// Hovered TS type
// const TermModel: Model<{
//     session?: {
//         _id?: ... | undefined;
//         equals?: {} | undefined;
//         id?: {
//             [x: number]: unknown;
//             equals?: {} | undefined;
//             set?: {} | undefined;
//             toJSON?: {} | undefined;
//             [Symbol.iterator]?: {} | undefined;
//             ... 99 more ...;
//             valueOf: {};
//         } | undefined;
//         ... 6 more ...;
//         toString: {};
//     } | undefined;
//     start?: {
//         ...;
//     } | undefined;
//     end?: {
//         ...;
//     } | undefined;
// }, {}, {}, {}, Schema<...>>

I found this to be a very strange issue because the Schema.Types.ObjectId should've matched above and resulted in a Types.ObjectId type. So I reverted the change you made in 3feb8af and hardcoded the types an ObjectId is meant to match to and it worked, giving me the correct model definition.

This was the change I made. On https://github.com/Automattic/mongoose/blob/3feb8af37c4d65bdc2e68b230882723c33cdfe7c/types/inferschematype.d.ts#L180

 PathValueType extends BooleanSchemaDefinition ? boolean :
+  PathValueType extends 'objectId' | 'ObjectId' | typeof Schema.Types.ObjectId ? Types.ObjectId :
-  PathValueType extends ObjectIdSchemaDefinition ? Types.ObjectId :
     PathValueType extends 'decimal128' | 'Decimal128' | typeof Schema.Types.Decimal128 ? Types.Decimal128 :
const TermModel: Model<{
    session: Types.ObjectId;
    start: Date;
    end: Date;
}, {}, {}, {}, Schema<any, Model<any, any, any, any, any>, {}, {}, {}, {}, "type", {
    session: Types.ObjectId;
    start: Date;
    end: Date;
}>>

This change was already weird to me because the ObjectIdSchemaDefinition and the type before are virtually the same, with only the casing of one of the strings (which I didn't use) that was changed.

I kept on trying, and found another change that solved the issue for me.

It was with

mongoose/types/inferschematype.d.ts

Line 186 in d092fdc

PathValueType extends Record<string, any> ? ObtainDocumentType<PathValueType, any, TypeKey> :
where you used the ObtainDocumentType to infer the types of nested objects. From the Automattic#12030 issue. I don't know why this is thee case, because the ObjectId should've been matched lines before this one so it wouldn't have to bee deeply checked for other schema types.

 PathValueType extends typeof SchemaType ? PathValueType['prototype'] :
+  PathValueType extends Record<string, any> ? ObtainDocumentType<PathValueType, any, TypeKey> :
-  PathValueType extends Record<string, any> ? PathValueType :
     unknown;

This comment has been minimized.

Sign in to view

Copy link
@mohammad0-0ahmad

mohammad0-0ahmad Jul 28, 2022 •
edited
Loading

Author

Thanks, @iammola, for feedback, I will check that as soon as possible.
I've created new PR to improve auto type schema as much as possible, so please feel free to review the PR or maybe contribute, so we can work on it together.

Best regards.

This comment has been minimized.

Sign in to view

Copy link
@mohammad0-0ahmad

mohammad0-0ahmad Jul 28, 2022

Author

I've checked the code snippet you provided, yes it is a bug there as I can see, I've got the same behavior that you have got. All details are correct, we can fix that in separate PR than that mentioned above, because that should be fixed as soon as possible, feel free to fix that if you have time, and I will try to give code review, just mention me to get notification. Thanks a lot @iammola.

This comment has been minimized.

Sign in to view

Copy link
@mohammad0-0ahmad

mohammad0-0ahmad Jul 28, 2022 •
edited
Loading

Author

We can only revert the first change for now:

+ PathValueType extends 'objectId' | 'ObjectId' | typeof Schema.Types.ObjectId ? Types.ObjectId :
-  PathValueType extends ObjectIdSchemaDefinition ? Types.ObjectId :

This comment has been minimized.

Sign in to view

Copy link
@mohammad0-0ahmad

mohammad0-0ahmad Jul 28, 2022 •
edited
Loading

Author

We can check that further more to find perhaps better solution or at least understand the issue in this one.
But please create a separate one to cover this regression 👍

This comment has been minimized.

Sign in to view

Copy link
@iammola

iammola Jul 29, 2022

Aight @mohammad0-0ahmad. Thank you.

But I'm really curious as to why it's working like this. Why does the casing of one letter affect the inferred type and why is it deeply checked when you use the InfereSchemaType helper or convert it to a model, but it looks fine when it's still the Schema you check? Any idea why?
unknown;

0 comments on commit d092fdc

Please sign in to comment.