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

Could someone help explain/fix the type for Schema? #10308

Closed
bozdoz opened this issue May 30, 2021 · 5 comments
Closed

Could someone help explain/fix the type for Schema? #10308

bozdoz opened this issue May 30, 2021 · 5 comments
Assignees
Labels
docs This issue is due to a mistake or omission in the mongoosejs.com documentation
Milestone

Comments

@bozdoz
Copy link

bozdoz commented May 30, 2021

What are the versions of Node.js, Mongoose and MongoDB you are using? Note that "latest" is not a version.
Node: 14.17.0
Mongoose: 5.12.10
Mongo: latest (docker image)

So I'm writing a Schema based on an interface User:

export interface BaseModel {
  readonly createdAt: Date;
  readonly updatedAt: Date;
  readonly _id: string;
  readonly __v: string;
}

export interface User extends BaseModel {
  name: string;
  active?: boolean;
}

My Schema is:

const UserSchema = new Schema<User, Model<User>>(
  {
    name: {
      type: String,
      required: [true, "Please provide a name."],
      maxlength: [50, "Name cannot be more than 50 characters"],
    },
    active: {
      type: Boolean,
      default: true,
    },
  },
  {
    timestamps: true,
  }
);

After looking through the type for the Schema class, I've realized I need to add a third generic to the UserSchema:

const UserSchema = new Schema<User, Model<User>, User>(

This is already a surprising interface; but that successfully reports errors for typos in fields. That's good, but it doesn't report if a field is missing.

Here's something close to what I would have expected for a type:

    constructor(definition: Omit<{
      [P in keyof DocType]: SchemaTypeOptions<DocType[P]>
    }, '_id' | 'createdAt' | 'updatedAt' | '__v'>, options?: SchemaOptions);

instead of what's currently there (https://github.com/Automattic/mongoose/blob/master/index.d.ts#L1103):

constructor(definition?: SchemaDefinition<DocumentDefinition<SchemaDefinitionType>>, options?: SchemaOptions);

Which appears to be much more complicated, and I can't figure out why my suggested replacement wouldn't be much better. What am I missing? Wouldn't it be better to report if a field is missing? Why is the current type so convoluted and based on a third Generic being passed?

Any thoughts/comments would be much appreciated. Thanks!

@IslandRhythms IslandRhythms added the discussion If you have any thoughts or comments on this issue, please share them! label Jun 1, 2021
@IslandRhythms IslandRhythms added the typescript Types or Types-test related issue / Pull Request label Jun 1, 2021
@vkarpov15 vkarpov15 added this to the 5.12.14 milestone Jun 4, 2021
@vkarpov15 vkarpov15 removed the discussion If you have any thoughts or comments on this issue, please share them! label Jun 4, 2021
@vkarpov15
Copy link
Collaborator

The Omit<> in your example is why Mongoose doesn't complain about fields that are in the interface but not in the schema. Mongoose supports a lot of different ways of defining your schema, and paths may be added to the schema by plugins or future calls to add(). So we can't reasonably Omit<> every path that will be added to the schema later.

The reason for the 3rd generic arg is backwards compatibility. Schema<DocType, M, SchemaDefinitionType> uses DocType as this for document middleware by default, but Mongoose does not use DocType to check for field name typos, etc. So we added a 3rd generic arg for backwards compatibility.

We'll add some more about this to our TypeScript docs.

@vkarpov15 vkarpov15 added docs This issue is due to a mistake or omission in the mongoosejs.com documentation and removed typescript Types or Types-test related issue / Pull Request labels Jun 14, 2021
@bozdoz
Copy link
Author

bozdoz commented Jun 14, 2021

There might be a misunderstanding. I'm asking why my Omit example isn't superior (or something closer to it). My understanding from what you wrote is:

we can't reasonably Omit<> every path that will be added to the schema later

If a dev is using plugins, or future calls to add(), can't that be part of the dev's definition? Why would mongoose need to know about that?

For that matter, you don't even need the Omit<>: If the definition was just [P in keyof DocType]: SchemaTypeOptions<DocType[P]> it would work for determining missing fields.

To be clear (in reference to your first sentence), my example is what did make it complain if a field was missing in the schema; the current definition does not complain.

@vkarpov15
Copy link
Collaborator

Because we'd need to know all the paths to list in '_id' | 'createdAt' | 'updatedAt' | '__v' - how would Mongoose figure out what extra paths need to be there?

@bozdoz
Copy link
Author

bozdoz commented Jun 14, 2021

As I said, I don't understand why you would need that: Forget the Omit<>. Why does the current type not include some variation of the following:

constructor(definition?: {
  [P in keyof DocType]: SchemaTypeOptions<DocType[P]>
}, options?: SchemaOptions);

In your example, the above should work: 584e630#diff-4c048a457328a2cbcb214eba5e9c0639ab3b5cdb8710a57e251b31ccc774dde5

@vkarpov15
Copy link
Collaborator

The current type is a somewhat more complex option of what you specified. Take a look at what SchemaDefinition is in our index.d.ts:

: { [path in keyof T]?: SchemaDefinitionProperty<T[path]>; };

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This issue is due to a mistake or omission in the mongoosejs.com documentation
Projects
None yet
Development

No branches or pull requests

3 participants