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

How do you get type checking for schema definitions to work when using Model.create or other Model methods? #11148

Closed
barca-reddit opened this issue Dec 27, 2021 · 8 comments
Milestone

Comments

@barca-reddit
Copy link

Do you want to request a feature or report a bug?

I have a question.

What is the current behavior?

Hello. I have been using Mongoose for years, and recently I started with Typescript so currently in the process of converting, and I have a couple of quick questions. Let's say I have a very simple schema, nearly identical as the one provided in the TypeScript Support section of Mongoose docs.

import { model, Schema } from 'mongoose';

interface IUser {
    name: string;
    age: number;
}

const UserSchema = new Schema<IUser>({
    name: { type: String, required: true },
    age: { type: Number, required: true },
});

const User = model<IUser>('User', UserSchema);

What is the expected behavior?

What I want to be able to have is a type checking against the schema definition, whenever I use methods such as Model.create or when I create a new instance of the User model. Here for example, I expect Typescript to complain (but it doesn't):

const createUser = async () => {
    const user = new User({ foo: 'bar' });
    await User.create({ foo: 'bar' });

    // or even

    const user2 = new User({ name: false, age: 'invalid' });
};

In this example, it lets me use completely arbitrary object properties, while also letting me omit name and age keys as defined in by IUser interface in UserSchema and User model. As far as Typescript is concerned, all of this code is valid and no errors are detected. I can slightly improve this if I force the IUser interface when creating the document, however, as you can see this only solves half of the problem.

const createUser = async () => {
    await User.create(<IUser>{ foo: 'bar' });       // works as intended, problem is detected (wavy red line)

    await User.create(<IUser>{});                   // not working, empty object
    await User.create(<IUser>{ name: 'bar' });      // not working, "age" property is missing
    await User.create(<IUser>{ age: 30 });          // not working, "name" property is missing
};

image

In VSCode, if I hover with my mouse over Schema where I am defining it, it lets me see how the definition works, and that tells me that all the object properties are set as optional and not respecting the IUser interface where in fact they are required:

image

But my knowledge in TypeScript is not good enough to understand why this is happening or if it's necessary.

Surprisingly, I was able to find very little on this topic online, closest one is this question on StackOverflow from 2 months ago, but it has no answers.

So overall my question is, how do I achieve type checking using model methods, is it even possible, and if it is, what is the correct way to do it? I apologize in advance in case I am missing something obvious here, as I mentioned was only starting with TS and TS Mongoose so there is a possibility that I am doing something incorrectly. Nevertheless, any guidance or quick example would be highly appreciated. Cheers!

What are the versions of Node.js, Mongoose and MongoDB you are using? Note that "latest" is not a version.

Node.js 16.13.1
Mongoose 6.1.3
VSCode 1.63.2
tsconfig.json

{
    "compilerOptions": {
        "target": "ES2020",
        "module": "commonjs",
        "rootDir": "./src",
        "outDir": "./out",
        "strict": true,
        "forceConsistentCasingInFileNames": true,
        "preserveWatchOutput": true,
        "esModuleInterop": true,
        /* ESBuild specific config */
        "noEmit": true,
        "watch": true
    },
    "exclude": [
        "**/node_modules",
        "**/.*/",
        "src/assets/**",
    ],
    "include": [
        "src"
    ]
}
@IslandRhythms IslandRhythms added the help This issue can likely be resolved in GitHub issues. No bug fixes, features, or docs necessary label Dec 27, 2021
@saalihou
Copy link

Yes surprisingly this is the definition for the create method

    /** Creates a new document or documents */
    create(docs: (AnyKeys<T> | AnyObject)[], options?: SaveOptions): Promise<HydratedDocument<T, TMethods, TVirtuals>[]>;
    create(docs: (AnyKeys<T> | AnyObject)[], callback: Callback<HydratedDocument<T, TMethods, TVirtuals>[]>): void;
    create(doc: AnyKeys<T> | AnyObject): Promise<HydratedDocument<T, TMethods, TVirtuals>>;
    create(doc: AnyKeys<T> | AnyObject, callback: Callback<HydratedDocument<T, TMethods, TVirtuals>>): void;
    create<DocContents = AnyKeys<T>>(docs: DocContents[], options?: SaveOptions): Promise<HydratedDocument<T, TMethods, TVirtuals>[]>;
    create<DocContents = AnyKeys<T>>(docs: DocContents[], callback: Callback<HydratedDocument<T, TMethods, TVirtuals>[]>): void;
    create<DocContents = AnyKeys<T>>(doc: DocContents): Promise<HydratedDocument<T, TMethods, TVirtuals>>;
    create<DocContents = AnyKeys<T>>(...docs: DocContents[]): Promise<HydratedDocument<T, TMethods, TVirtuals>[]>;
    create<DocContents = AnyKeys<T>>(doc: DocContents, callback: Callback<HydratedDocument<T, TMethods, TVirtuals>>): void;

I'm sure there is a reasoning behind allowing any document through but I'm not sure I grasp it. Isn't the role of typescript to prevent objects not conforming to the type definition to not be instantiated thus saved to the database ?
Shouldn't it just be create(doc: T, ...) and create(docs: T[], ...) ? (I tried that and got the expected type error when instantiating a model wrong)

image

@vkarpov15
Copy link
Collaborator

Unfortunately this is expected behavior for several reasons. await User.create({ foo: 'bar' }); is fine because Mongoose doesn't necessarily know at compile time whether bar is required or whether bar has a default. Similarly, new User({ name: false, age: 'invalid' }) is OK because we don't know at compile time whether 'invalid' is a string that Mongoose can cast to a number. new User({ name: 'test', age: '42' }) is perfectly valid as far as Mongoose is concerned.

Also, your createUser() syntax looks a little weird. Shouldn't await User.create(<IUser>{}); be await User.create<IUser>({}) ? That should be how you enforce that whatever you pass into create() strictly matches IUser.

@barca-reddit
Copy link
Author

barca-reddit commented Jan 3, 2022

@vkarpov15 thanks for reaching out!

because Mongoose doesn't necessarily know at compile time whether bar is required or whether bar has a default.

I totally get that required is a Mongoose specific property, but what I though would happen OR could be made to happen is you can enforce it as required by the Typescript interface (IUser), rather than the required property in the schema. But for reasons beyond my understanding, all the required properties from the IUser interface are set as optional inside the schema definition (screenshot 2 from my original comment).

Also, your createUser() syntax looks a little weird. Shouldn't await User.create(<IUser>{}); be await User.create<IUser>({}) ? That should be how you enforce that whatever you pass into create() strictly matches IUser.

This works, however on small downside is that you lose type suggestions/Intellisense.

image

IIRC before submitting this issue on github I tried this syntax but I thought I must be doing something wrong and I couldn't find more info about it online.


const createUser = async () => {
    await User.create<IUser>({ name: 'bar', age: 50 });            // ✅ valid
    await User.create<IUser>({ name: 'bar' });                     // ❌ missing key
    await User.create<IUser>({ age: 50 });                         // ❌ missing key
    await User.create<IUser>({ name: false, age: 50 });            // ❌ wrong type
    await User.create<IUser>({ name: 'bar', age: 50, test: 123 }); // ❌ arbitrary key
};

The code above works as expected (with the few downsides I mentioned) and typescript correctly tells if there are missing keys, wrong types or arbitrary properties. I guess it's not totally ideal (for me personally) but if provides type checking then it's good enough.

I have one quick question, is there a way or specific syntax which would let me achieve the same or similar behavior, but when creating a new instance of the model? new User<IUser>({...}), won't work because User doesn't accept any generic types, and new User(<IUser>{...}) has the same issues as the original example with create.

@vkarpov15
Copy link
Collaborator

It's very strange that intellisense doesn't pick up that User.create<IUser>() takes IUser as a param. If that's the case, I'm not sure what else we can do to support intellisense.

Re new User<IUser>({...}), we don't currently support that, but I just added that and we'll release that in v6.1.6

@vkarpov15 vkarpov15 removed the help This issue can likely be resolved in GitHub issues. No bug fixes, features, or docs necessary label Jan 5, 2022
@vkarpov15 vkarpov15 added this to the 6.1.6 milestone Jan 5, 2022
@barca-reddit
Copy link
Author

It's very strange that intellisense doesn't pick up that User.create<IUser>() takes IUser as a param. If that's the case, I'm not sure what else we can do to support intellisense.

Slightly off-topic, but allow me to point out that other Model methods that involve an update object, but don't accept a generic type parameter are also producing mixed results (in terms of type checking and intellisense).

const createUser = async () => {
    await User.findByIdAndUpdate('123', { valid: false });
    await User.findOneAndUpdate({ name: 'foo' }, { valid: false });
    await User.findOneAndReplace({ name: 'foo' }, { valid: false, }); // ✅ intellisense (suggestions)
    await User.replaceOne({ name: 'foo' }, { valid: false }) // ✅ intellisense (suggestions)
    await User.updateMany({ name: 'foo' }, { valid: false })
    await User.updateOne({ name: 'foo' }, { valid: false })
};

All of these are considered valid by TS/Mongoose, while obviously they are not. I just wanted to put these examples out there in case you want to look into them, but I totally understand that implementing/improving this might not be a priority (or a planned feature) at this point of time.

Re new User<IUser>({...}), we don't currently support that, but I just added that and we'll release that in v6.1.6

That would be awesome, thank you and looking forward to try it out, cheers!

@vkarpov15
Copy link
Collaborator

await User.findByIdAndUpdate('123', { valid: false }) is not invalid, or at least not always invalid. Mongoose strips out keys that aren't in the schema from the update by default, so putting extraneous keys in an update isn't an issue unless you use strict: 'throw'.

@hmeerlo
Copy link

hmeerlo commented Jun 29, 2022

@vkarpov15 What is the reasoning behind this? When would I ever want to perform an update with properties that are not in the schema? Isn't this exactly why we would want TypeScript in the first place?

@sayandcode
Copy link

I have found somewhat of a workaround. I just extend my Mongoose Model with another class

const testSchema = new Schema({
  name: { type: String, required: true },
});

const TestModel = model("Test", testSchema);
class Test extends TestModel {}

const myTest = new Test({ name: 123 }); // gives typescript error
const myOtherTest = new Test({ name: '123' }); // no errors, as expected

Don't know why it works. Don't know if there are any implications on using the methods on the Model. I thought it might help you guys. Try it out

@Automattic Automattic locked as resolved and limited conversation to collaborators Feb 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants