feat(types+schema): allow defining schema paths using mongoose.Types.* to work around TS type inference issues #12352

vkarpov15 · 2022-08-29T19:56:59Z

Summary

I've spent quite some time debugging #12205 and haven't come up with quite a complete picture of why this is happening, but I got a decent enough workaround.

The issue comes down to the fact that, for some reason, InferSchemaType<> runs the ObtainDocumentType calculation twice: once with the schema definition, and once with the returned value from the first run. I suspect that's because of the self-referential DocType = ObtainDocumentType<DocType> line here, but I haven't been able to figure out how to remove that. Any thoughts on this issue @mohammad0-0ahmad ?

Also had to add some fixes to ResolvePathType<> because, apparently, Schema.Types.ObjectId extends typeof Schema.Types.ObjectId is false 🤷‍♂️ .

The workaround I came up with is to allow defining ObjectId and Decimal128 paths using mongoose.Types.ObjectId in addition to mongoose.Schema.Types.ObjectId (and same for Decimal128). This is likely a decent DX improvement anyway, since the difference between mongoose.Types.ObjectId and mongoose.Schema.Types.ObjectId can be confusing.

Examples

…* to work around TS type inference issues Fix #12205

mohammad0-0ahmad · 2022-08-29T22:35:09Z

Yes, that seems to be correct, I've not checked the changes you made in this PR, but I've noticed that we can solve it as well by making this change "Infer doc type directly without using ObtainSchemaGeneric helper":

// inferschematype.d.ts
// line 41
- type InferSchemaType<SchemaType> = ObtainSchemaGeneric<SchemaType, 'DocType'>;
+ type InferSchemaType<SchemaType> = SchemaType extends Schema<any, any, any, any, any, any, any, infer T> ? T : unknown;

OR by making the following change which I mentioned before in #12168:

// inferschematype.d.ts
// line 171
-PathValueType extends Record<string, any> ? ObtainDocumentType<PathValueType, any, TypeKey> :
+PathValueType extends Record<string, any> ? IfEquals<PathValueType, Types.ObjectId, Types.ObjectId, ObtainDocumentType<PathValueType, any, TypeKey>>:

After using TS for a while, sometimes it feels that TS can have unexpected behavior, for instance in the first solution ObtainSchemaGeneric it is literally making the same as the provided solution but gives two different results.
I wrote that

mongoose/types/index.d.ts

Line 175 in 0af4cb8

    
           DocType extends ObtainDocumentType<DocType, EnforcedDocType, TPathTypeKey> = ObtainDocumentType<any, EnforcedDocType, TPathTypeKey>>

for obtaining doc type for one time instead of calling ObtainDocumentType multiple time in the code and for be able to show the schema paths when user hovers on schema itself.
I don't really know right now how TS utilities works in details, "I've not read TS source code yet 😄" But I think it might be helpful if we ask someone of TS team about this specific case, It might be helpful to understand the TS behavior more.
@vkarpov15 can you mention someone here of TS team who can be able to help here quickly? I don't think we can find the answer in its docs. "I might be wrong"

vkarpov15 · 2022-08-30T16:08:52Z

I don't know of anyone on the TS team that might help with this.

I spent a long time wrangling with the type InferSchemaType<SchemaType> = SchemaType extends Schema<any, any, any, any, any, any, any, infer T> ? T : unknown; solution, it fixes this particular issue but introduces another issue with the #12030 test here. And I wasn't able to figure out any workaround for that issue.

IslandRhythms

LGTM

mohammad0-0ahmad · 2022-08-30T23:48:56Z

I don't know of anyone on the TS team that might help with this.

I spent a long time wrangling with the type InferSchemaType<SchemaType> = SchemaType extends Schema<any, any, any, any, any, any, any, infer T> ? T : unknown; solution, it fixes this particular issue but introduces another issue with the #12030 test here. And I wasn't able to figure out any workaround for that issue.

I will check that at the weekend.

mohammad0-0ahmad · 2022-09-05T13:23:10Z

test/types/schema.test.ts

@@ -640,6 +644,28 @@ function gh12030() {
    ]
  });

+  type A = ResolvePathType<[


these tests are a bit wrong, I think they should be:

const a = [ { username: { type: String } } ]; expectType<{ username?: string }[]>({} as ResolvePathType<typeof a>); const b = { users: [ { username: { type: String } } ] }; expectType<{ users: { username?: string }[]; }>({} as ResolvePathType<typeof b>); const Schema1 = new Schema({ users: [ { username: { type: String } } ] });

mohammad0-0ahmad · 2022-09-05T13:25:54Z

types/inferschematype.d.ts

-                              PathValueType extends typeof SchemaType ? PathValueType['prototype'] :
-                                PathValueType extends Record<string, any> ? ObtainDocumentType<PathValueType, any, TypeKey> :
-                                  unknown;
+        IfEquals<PathValueType, Schema.Types.String> extends true ? PathEnumOrString<Options['enum']> :


I think we need to be more careful before making changes to ResolvePathType,
I've not checked all changes yet, but I prefer to avoid making all of these changes. " I might be wrong"
Will come back with more details.

mohammad0-0ahmad · 2022-09-05T13:49:36Z

test/types/schema.test.ts

+  const campaignSchema = new Schema(
+    {
+      client: {
+        type: new Types.ObjectId(),


Is ObjectId instance an acceptable value to pass ?

mohammad0-0ahmad · 2022-09-05T13:51:42Z

I've not a lot of time to check this, but I've left some comments, I will get more time after 12 SEP.

HiiiiD · 2022-09-16T11:30:27Z

Guys, I think I figured out the solution: String doesn't extend typeof String, but it extends String, same thing for ObjectId and for Decimal128
And this is for every class in TS
@vkarpov15 @mohammad0-0ahmad

HiiiiD · 2022-09-16T13:35:26Z

BTW the fix that I proposed also solves #12373 and #12431

HiiiiD · 2022-09-20T09:50:57Z

Any news? Do I have to create another PR to address this?

mohammad0-0ahmad · 2022-09-20T15:27:36Z

Guys, I think I figured out the solution: String doesn't extend typeof String, but it extends String, same thing for ObjectId and for Decimal128 And this is for every class in TS @vkarpov15 @mohammad0-0ahmad

I think that is incorrect, I think the entire issue as @vkarpov15 mentioned that InferSchemaType reinfer the already inferred type. I might misunderstand you, but check this example first:
https://www.typescriptlang.org/play?ssl=5&ssc=26&pln=1&pc=1#code/MYewdgzgLgBAHjAvDAylATgSzAcwNwBQBUAngA4CmMAKhdADzUB8y1MFcUFYAJhDKUogAZqgzYcMAPwD0AVyoAuGMICGAGwgVCxclTjokNOlHqCKI+E0JA

mohammad0-0ahmad · 2022-09-20T15:32:07Z

In ResolvePathType helper, we can do this for fixing ObjectId issue:

-PathValueType extends ObjectIdSchemaDefinition ? Types.ObjectId :
+PathValueType extends ObjectIdSchemaDefinition | Types.ObjectId ? Types.ObjectId :

And the same for the other mentioned types.
But it might be better to find another solution which break the loop from beginning.

mohammad0-0ahmad · 2022-09-20T19:36:34Z

types/inferschematype.d.ts

   * @example
   * const userSchema = new Schema({userName:String});
   * type UserType = InferSchemaType<typeof userSchema>;
   * // result
   * type UserType = {userName?: string}
   */
-  type InferSchemaType<SchemaType> = ObtainSchemaGeneric<SchemaType, 'DocType'>;
+  type InferSchemaType<TSchema> = ObtainSchemaGeneric<TSchema, 'DocType'>;


Would you like to test this ?
note: revert all changes in this file while you testing.

type InferSchemaType<TSchema> = TSchema extends Schema<any, any, any, any, any, any, string, infer DocType> ? DocType : unknown;

mohammad0-0ahmad · 2022-09-20T19:40:07Z

test/types/schema.test.ts

+
+  expectType<'type'>({} as ObtainSchemaGeneric<typeof campaignSchema, 'TPathTypeKey'>);
+
+  type A = ObtainDocumentType<{ client: { type: Schema.Types.ObjectId, required: true } }>;


You use Schema.Types.ObjectId as type, I think this is incorrect, or I might be wrong.
Same thing for the following tests.

feat(types+schema): allow defining schema paths using mongoose.Types.…

78eb098

…* to work around TS type inference issues Fix #12205

vkarpov15 added this to the 6.6 milestone Aug 29, 2022

vkarpov15 requested review from Uzlopak, AbdelrahmanHafez and IslandRhythms August 29, 2022 19:56

IslandRhythms approved these changes Aug 30, 2022

View reviewed changes

mohammad0-0ahmad reviewed Sep 5, 2022

View reviewed changes

vkarpov15 modified the milestones: 6.6, 6.7 Sep 8, 2022

hasezoey mentioned this pull request Sep 15, 2022

Issues with Decimal128 type #12431

Closed

2 tasks

mohammad0-0ahmad reviewed Sep 20, 2022

View reviewed changes

vkarpov15 mentioned this pull request Sep 26, 2022

Fix types resolved from InferSchemaType #12450

Closed

test(types): add test case from #12450 to #12352

25652c2

vkarpov15 mentioned this pull request Oct 2, 2022

Incompatibility with Typescript 4.8.2 - Schemas that use query population (ObjectId and Ref) fail with serialization errors #12373

Closed

2 tasks

Merge branch '6.7' into vkarpov15/gh-12205

d290d9a

vkarpov15 changed the base branch from master to 6.7 October 2, 2022 21:08

vkarpov15 merged commit 24a768b into 6.7 Oct 2, 2022

vkarpov15 deleted the vkarpov15/gh-12205 branch October 2, 2022 21:08

hasezoey mentioned this pull request Oct 4, 2022

InferSchemaType says unknown type for everything #12510

Closed

2 tasks

vkarpov15 mentioned this pull request Oct 5, 2022

Throw readable error when using mongoose.Types. instead of mongoose.Schema.Types. when creating schema #8905

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(types+schema): allow defining schema paths using mongoose.Types.* to work around TS type inference issues #12352

feat(types+schema): allow defining schema paths using mongoose.Types.* to work around TS type inference issues #12352

vkarpov15 commented Aug 29, 2022

mohammad0-0ahmad commented Aug 29, 2022 •

edited

Loading

vkarpov15 commented Aug 30, 2022

IslandRhythms left a comment

mohammad0-0ahmad commented Aug 30, 2022

mohammad0-0ahmad Sep 5, 2022

mohammad0-0ahmad Sep 5, 2022 •

edited

Loading

mohammad0-0ahmad Sep 5, 2022

mohammad0-0ahmad commented Sep 5, 2022

HiiiiD commented Sep 16, 2022 •

edited

Loading

HiiiiD commented Sep 16, 2022 •

edited

Loading

HiiiiD commented Sep 20, 2022

mohammad0-0ahmad commented Sep 20, 2022

mohammad0-0ahmad commented Sep 20, 2022 •

edited

Loading

mohammad0-0ahmad Sep 20, 2022 •

edited

Loading

mohammad0-0ahmad Sep 20, 2022 •

edited

Loading


		expectType<'type'>({} as ObtainSchemaGeneric<typeof campaignSchema, 'TPathTypeKey'>);

		type A = ObtainDocumentType<{ client: { type: Schema.Types.ObjectId, required: true } }>;

feat(types+schema): allow defining schema paths using mongoose.Types.* to work around TS type inference issues #12352

feat(types+schema): allow defining schema paths using mongoose.Types.* to work around TS type inference issues #12352

Conversation

vkarpov15 commented Aug 29, 2022

mohammad0-0ahmad commented Aug 29, 2022 • edited Loading

vkarpov15 commented Aug 30, 2022

IslandRhythms left a comment

Choose a reason for hiding this comment

mohammad0-0ahmad commented Aug 30, 2022

mohammad0-0ahmad Sep 5, 2022

Choose a reason for hiding this comment

mohammad0-0ahmad Sep 5, 2022 • edited Loading

Choose a reason for hiding this comment

mohammad0-0ahmad Sep 5, 2022

Choose a reason for hiding this comment

mohammad0-0ahmad commented Sep 5, 2022

HiiiiD commented Sep 16, 2022 • edited Loading

HiiiiD commented Sep 16, 2022 • edited Loading

HiiiiD commented Sep 20, 2022

mohammad0-0ahmad commented Sep 20, 2022

mohammad0-0ahmad commented Sep 20, 2022 • edited Loading

mohammad0-0ahmad Sep 20, 2022 • edited Loading

Choose a reason for hiding this comment

mohammad0-0ahmad Sep 20, 2022 • edited Loading

Choose a reason for hiding this comment

mohammad0-0ahmad commented Aug 29, 2022 •

edited

Loading

mohammad0-0ahmad Sep 5, 2022 •

edited

Loading

HiiiiD commented Sep 16, 2022 •

edited

Loading

HiiiiD commented Sep 16, 2022 •

edited

Loading

mohammad0-0ahmad commented Sep 20, 2022 •

edited

Loading

mohammad0-0ahmad Sep 20, 2022 •

edited

Loading

mohammad0-0ahmad Sep 20, 2022 •

edited

Loading