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

schema type auto infer bugs #12030

Closed
2 tasks done
imranbarbhuiya opened this issue Jul 1, 2022 · 13 comments
Closed
2 tasks done

schema type auto infer bugs #12030

imranbarbhuiya opened this issue Jul 1, 2022 · 13 comments

Comments

@imranbarbhuiya
Copy link

imranbarbhuiya commented Jul 1, 2022

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the bug has not already been reported

Mongoose version

6.4.2

Node.js version

18

MongoDB server version

5.0

Description

I've found a few more bugs in auto infer type

  1. Adding an object directly to the array doesn't infer type correctly
const userSchema = new Schema({
	users: [
		{
			username: { type: String }
		}
	]
});
  1. passing function in default makes the key optional
const userSchema = new Schema({
	createdAt: { type: Date, default: Date.now }
});
  1. Array type becomes Array instead of Types.DocumentArray which makes pushing fields difficult
const userSchema = new Schema({
	users: [
		new Schema({
			username: { type: String },
			credit: { type: Number, default: 0 }
		})
	]
});

const userModel = model('user', userSchema);

const data = await userModel.findOne();
data!.users.push({ username: 'parbez' });
// ts warns to add createdAt which can be solved using Types.DocumentArray
  1. Adding empty object as default doesn't makes the key required
const userSchema = new Schema({
	data: { type: { role: Object }, default: {} }
});
  1. Assigning an object directly to a key doesn't infer type correctly
const userSchema = new Schema({
	data: {
		username: String
	}
});

Steps to Reproduce

added code blocks above

Expected Behavior

No response

@mohammad0-0ahmad
Copy link
Contributor

@imranbarbhuiya Thanks for reporting this one as well, I will check the new changes and see what we can do 👍

@imranbarbhuiya
Copy link
Author

I've updated code block 4 and added the 5th point which is the same as 1 but for an object.

@shiva-prasad-reddy
Copy link

const schema = new Schema({
  track: {
    backupCount: {
      type: Number,
      default: 0,
    },
    count: {
      type: Number,
      default: 0,
    },
  },
});
type ISchema = InferSchemaType<typeof schema>;

resulting to a type of

type ISchema = {
    track: {
        backupCount: {
            type: NumberConstructor;
            default: number;
        };
        count: {
            type: NumberConstructor;
            default: number;
        };
    };
}

but it should be

type ISchema = {
    track: {
        backupCount: NumberConstructor;
        count: NumberConstructor;
    };
}

@mohammad0-0ahmad
Copy link
Contributor

mohammad0-0ahmad commented Jul 3, 2022

const schema = new Schema({
  track: {
    backupCount: {
      type: Number,
      default: 0,
    },
    count: {
      type: Number,
      default: 0,
    },
  },
});
type ISchema = InferSchemaType<typeof schema>;

resulting to a type of

type ISchema = {
    track: {
        backupCount: {
            type: NumberConstructor;
            default: number;
        };
        count: {
            type: NumberConstructor;
            default: number;
        };
    };
}

but it should be

type ISchema = {
    track: {
        backupCount: NumberConstructor;
        count: NumberConstructor;
    };
}

Do you mean?

type ISchema = {
    track?: {
      backupCount: number;
      count: number;
    };
  }

@mohammad0-0ahmad
Copy link
Contributor

mohammad0-0ahmad commented Jul 4, 2022

@imranbarbhuiya @shiva-prasad-reddy Thanks for being a part of improving auto inferred schema.
The provided issues got fixed now and will be hopefully merged soon.

Best regards.

@iammola
Copy link
Contributor

iammola commented Jul 4, 2022

const schema = new Schema({
  track: {
    backupCount: {
      type: Number,
      default: 0,
    },
    count: {
      type: Number,
      default: 0,
    },
  },
});
type ISchema = InferSchemaType<typeof schema>;

resulting to a type of

type ISchema = {
    track: {
        backupCount: {
            type: NumberConstructor;
            default: number;
        };
        count: {
            type: NumberConstructor;
            default: number;
        };
    };
}

but it should be

type ISchema = {
    track: {
        backupCount: NumberConstructor;
        count: NumberConstructor;
    };
}

Hey, @mohammad0-0ahmad @shiva-prasad-reddy @vkarpov15 @imranbarbhuiya

IMO, what @shiva-prasad-reddy is expecting is wrong. Based on the documentation, https://mongoosejs.com/docs/subdocs.html#altsyntaxsingle.

If you want to specify the options for a schema path, you have to use a Schema, if not you're making a mixed path.

This is the example on the documentation.

const schema = new Schema({
  nested: {
    // Do not do this! This makes `nested` a mixed path in Mongoose 5
    type: { prop: String },
    required: true
  }
});

const schema = new Schema({
  nested: {
    // This works correctly
    type: new Schema({ prop: String }),
    required: true
  }
});

So your code should be.

const schema = new Schema({
  track: new Schema({ // note the Schema constructor
    backupCount: {
      type: Number,
      default: 0,
    },
    count: {
      type: Number,
      default: 0,
    },
  }),
});

Which will infer exactly what you're expecting

type ISchema = {
	track?: {
		backupCount: number;
		count: number;
	} | undefined
}

@mohammad0-0ahmad
Copy link
Contributor

mohammad0-0ahmad commented Jul 4, 2022

Thanks @iammola
We can be more comfortable by asking
@vkarpov15 @Uzlopak before merging #12042.

If I understood correctly, You mean that

const schema = new Schema({
  track: new Schema({ // note the Schema constructor
    backupCount: {
      type: Number,
      default: 0,
    },
    count: {
      type: Number,
      default: 0,
    },
  }),
});
type Schema = inferSchemaType<typeof schema>;
// gives

// {
// 	track?: {
// 		backupCount: number;
// 		count: number;
// 	} | undefined
// }

while the provided code by @shiva-prasad-reddy :

const schema = new Schema({
  track: {
    backupCount: {
      type: Number,
      default: 0,
    },
    count: {
      type: Number,
      default: 0,
    },
// Note: track path has none path options like type, setters or getters
  },
});
type Schema = InferSchemaType<typeof schema>;
// gives

// {
// 	track?: any
// }

Is that correct?

@iammola
Copy link
Contributor

iammola commented Jul 5, 2022

Thanks @iammola

We can be more comfortable by asking

@vkarpov15 @Uzlopak before merging #12042.

If I understood correctly, You mean that

const schema = new Schema({

  track: new Schema({ // note the Schema constructor

    backupCount: {

      type: Number,

      default: 0,

    },

    count: {

      type: Number,

      default: 0,

    },

  }),

});

type Schema = inferSchemaType<typeof schema>;

// gives



// {

// 	track?: {

// 		backupCount: number;

// 		count: number;

// 	} | undefined

// }

while the provided code by @shiva-prasad-reddy :

const schema = new Schema({

  track: {

    backupCount: {

      type: Number,

      default: 0,

    },

    count: {

      type: Number,

      default: 0,

    },

// Note: track path has none path options like type, setters or getters

  },

});

type Schema = InferSchemaType<typeof schema>;

// gives



// {

// 	track?: any

// }

Is that correct?

Pretty much @mohammad0-0ahmad, But currently it doesn't give any, and I don’t expect any as the inferred type

It should give this.

type ISchema = {
    track: {
        backupCount: {
            type: number;
            default: number;
        };
        count: {
            type: number;
            default: number;
        };
    };
}

@mohammad0-0ahmad
Copy link
Contributor

@iammola Thanks again for your feedback.
I've not refactored the related PR yet I am just waiting for @Uzlopak @vkarpov15 response to make sure which behavior user should get in simillar cases.

@jeetkn
Copy link

jeetkn commented Jul 10, 2022

Is fixed in 6.4.5?

@mohammad0-0ahmad
Copy link
Contributor

Is fixed in 6.4.5?

Waiting for maintainer response.

@vkarpov15
Copy link
Collaborator

@iammola just to clarify. The correct TS type for the below schema:

const schema = new Schema({
  track: {
    backupCount: {
      type: Number,
      default: 0,
    },
    count: {
      type: Number,
      default: 0,
    },
  },
});

should be:

type ISchema = {
    track: {
        backupCount: NumberConstructor;
        count: NumberConstructor;
    };
}

as @shiva-prasad-reddy indicated. track is a nested path, so it will never be null or undefined. Mongoose will always set it to at least an empty object.

The info from https://mongoosejs.com/docs/subdocs.html#altsyntaxsingle is specific to Mongoose 5 and also specific to cases where type is an object. We should delete that section of the docs for Mongoose 6.

vkarpov15 added a commit that referenced this issue Jul 15, 2022
@iammola
Copy link
Contributor

iammola commented Jul 15, 2022

Ahh, I see @vkarpov15. My bad!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants