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

AutoInferredSchema class #14962

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

AutoInferredSchema class #14962

wants to merge 2 commits into from

Conversation

vkarpov15
Copy link
Collaborator

Summary

#14954 points out an unfortunate issue: with our current automatic schema inference, there's no good way for InferRawDocType to handle nested schemas. The core issue there is that we don't have access to the SchemaDefinition type in TypeScript generics. class Schema<DocType> has the constructor schema definition param typed as SchemaDefinition<SchemaDefinitionType<RawDocType>, RawDocType> | DocType, and that type isn't something that we can use to infer the raw doc type.

This PR adds a separate AutoInferredSchema class that takes the schema definition as the first generic param. There's a separate class for backwards compatibility, to avoid breaking code like new Schema<UserType>.

In the long term, I think AutoInferredSchema is a step in the right direction, because we want the exact schema definition so we can infer both the correct raw document type and the correct hydrated document type, rather than trying to infer the raw document type and hydrated document type from the "both but not quite" behavior of InferSchemaType<>.

One big outstanding question: changing RawDocType = any to RawDocType = InferRawDocType<SchemaDef> in the AutoInferredSchema generics causes the test for #14954 test to fail with the following error, which makes it seem like 2-level deep subdoc schemas are treated as POJOs somewhere. While tests pass with the current setup, this error makes me feel like I'm missing something, so I'll take a step back and come back to this issue later.

  ✖  70:5  Argument of type { email: string; docArr: { test: string; }[]; password: string; dateOfBirth: NativeDate; subdoc?: { childSchemas: DocumentArray<{ model?: unknown; schema?: { [x: string]: unknown; } | null | undefined; }>; ... 49 more ...; eventNames?: {} | ... 1 more ... | undefined; } | null | undefined; } is not assignable to parameter of type { email: string; password: string; dateOfBirth: Date; subdoc?: { name: string; l2?: { myProp: number; } | null | undefined; } | null | undefined; docArr: { test: string; }[]; }.
  Types of property subdoc are incompatible.
    Type { childSchemas: DocumentArray<{ model?: unknown; schema?: { [x: string]: unknown; } | null | undefined; }>; index?: {} | null | undefined; set?: {} | null | undefined; once?: {} | null | undefined; ... 46 more ...; eventNames?: {} | ... 1 more ... | undefined; } | null | undefined is not assignable to type { name: string; l2?: { myProp: number; } | null | undefined; } | null | undefined.
      Property name is missing in type { childSchemas: DocumentArray<{ model?: unknown; schema?: { [x: string]: unknown; } | null | undefined; }>; index?: {} | null | undefined; set?: {} | null | undefined; once?: {} | null | undefined; ... 46 more ...; eventNames?: {} | ... 1 more ... | undefined; } but required in type { name: string; l2?: { myProp: number; } | null | undefined; }.  

  1 error

Examples

@vkarpov15 vkarpov15 changed the title Vkarpov15/gh 14954 AutoInferredSchema class Oct 14, 2024
Copy link
Collaborator

@hasezoey hasezoey left a comment

Choose a reason for hiding this comment

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

I have also tried to find where the error comes from, but i also have not found it.

While trying to find where the problem was, i wanted to get the generics for type debugging, but i found that using ObtainSchemaGeneric<> on a AutoInferredSchema somehow only returns unknown for EnforcedDocType and TSchemaOptions, all other properties (like M, DocType, TInstanceMethods) return the correct position, i could fix this specific issue with the following patch:

diff --git a/types/inferschematype.d.ts b/types/inferschematype.d.ts
index 07ee5087b..58b14977d 100644
--- a/types/inferschematype.d.ts
+++ b/types/inferschematype.d.ts
@@ -57,18 +57,29 @@ declare module 'mongoose' {
    * @param {alias} alias Targeted generic alias.
    */
   type ObtainSchemaGeneric<TSchema, alias extends 'EnforcedDocType' | 'M' | 'TInstanceMethods' | 'TQueryHelpers' | 'TVirtuals' | 'TStaticMethods' | 'TSchemaOptions' | 'DocType'> =
-   TSchema extends Schema<infer EnforcedDocType, infer M, infer TInstanceMethods, infer TQueryHelpers, infer TVirtuals, infer TStaticMethods, infer TSchemaOptions, infer DocType>
-     ? {
-       EnforcedDocType: EnforcedDocType;
-       M: M;
-       TInstanceMethods: TInstanceMethods;
-       TQueryHelpers: TQueryHelpers;
-       TVirtuals: TVirtuals;
-       TStaticMethods: TStaticMethods;
-       TSchemaOptions: TSchemaOptions;
-       DocType: DocType;
-     }[alias]
-     : unknown;
+    TSchema extends AutoInferredSchema<infer SchemaDef, infer EnforcedDocType, infer M, infer TInstanceMethods, infer TQueryHelpers, infer TVirtuals, infer TStaticMethods, infer TSchemaOptions, infer DocType> ?
+      {
+        EnforcedDocType: EnforcedDocType;
+        M: M;
+        TInstanceMethods: TInstanceMethods;
+        TQueryHelpers: TQueryHelpers;
+        TVirtuals: TVirtuals;
+        TStaticMethods: TStaticMethods;
+        TSchemaOptions: TSchemaOptions;
+        DocType: DocType;
+      }[alias]
+    : TSchema extends Schema<infer EnforcedDocType, infer M, infer TInstanceMethods, infer TQueryHelpers, infer TVirtuals, infer TStaticMethods, infer TSchemaOptions, infer DocType>
+      ? {
+        EnforcedDocType: EnforcedDocType;
+        M: M;
+        TInstanceMethods: TInstanceMethods;
+        TQueryHelpers: TQueryHelpers;
+        TVirtuals: TVirtuals;
+        TStaticMethods: TStaticMethods;
+        TSchemaOptions: TSchemaOptions;
+        DocType: DocType;
+      }[alias]
+      : unknown;
 
   type ResolveSchemaOptions<T> = MergeType<DefaultSchemaOptions, T>;

(adding a special case for AutoInferredSchema)

@@ -6,6 +6,7 @@ import {
PathWithTypePropertyBaseType,
PathEnumOrString
} from './inferschematype';
import { InferSchemaType } from 'mongoose';
Copy link
Collaborator

Choose a reason for hiding this comment

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

shouldnt it import it relatively?

@hasezoey hasezoey added the typescript Types or Types-test related issue / Pull Request label Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
typescript Types or Types-test related issue / Pull Request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants