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

InferSchemaType: Improve type safety by inferring {required: boolean} as a required field instead of optional #12782

Closed
2 tasks done
JavaScriptBach opened this issue Dec 9, 2022 · 0 comments · Fixed by #12784
Labels
enhancement This issue is a user-facing general improvement that doesn't fix a bug or add a new feature new feature This change adds new functionality, like a new method or class

Comments

@JavaScriptBach
Copy link
Contributor

Prerequisites

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

🚀 Feature Proposal

If I write the following code:

const definition = { test: { type: String, required: true } };
const schema = new Schema(definition);
type SchemaType = InferSchemaType<typeof schema>;

SchemaType will think that test is an optional string. This is because TypeScript infers the type of definition as { test: { type: StringConstructor, required: boolean } } instead of { test: { type: StringConstructor, required: true } }, so Mongoose can't infer whether the field is required in https://github.com/Automattic/mongoose/blob/master/types/inferschematype.d.ts#L83, and defaults to optional.

The user can workaround this by writing required: true as const or defining schema definition inline into the new Schema() constructor, but if they don't realize something is wrong, they will end up with incorrectly optional types.

I propose that we infer required: boolean as a required field for the following reasons:

  1. required defaults to false. If the user specified required, it's usually true.
  2. It's better that we type an optional field as required (makes types unnecessarily strict, but won't break) instead of vice versa (incorrectly allows optional fields which will break at runtime).

Motivation

Improve type safety by defaulting ambiguity to required instead of optional.

Example

See above.

@JavaScriptBach JavaScriptBach added enhancement This issue is a user-facing general improvement that doesn't fix a bug or add a new feature new feature This change adds new functionality, like a new method or class labels Dec 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This issue is a user-facing general improvement that doesn't fix a bug or add a new feature new feature This change adds new functionality, like a new method or class
Projects
None yet
1 participant