mongoose icon indicating copy to clipboard operation
mongoose copied to clipboard

Ditch `AnyKeys` in `Model.create` and `Model.insertOne`

Open WillsterJohnson opened this issue 7 months ago • 2 comments

Prerequisites

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

🚀 Feature Proposal

Only reference I can find to this is #11148, which is now several years old.

I initially suggested this be a feature in typegoose (see here), but was informed that mongoose now has the features necessary to infer enough type information to properly type Model.create and Model.insertOne.

I mention in the issue linked above that AnyKeys destroys type information, and is equivalent to the following type, which only preserves keys and not any information about their values, ie the use of T[P] | any is equivalent to any.

// given type
type AnyKeys<T> = { [P in keyof T]?: T[P] | any };
// equivalent to
type AnyKeys<T> = Partial<Record<keyof T, any>>

Likewise, if you put this type in union as T | AnyKeys<T> (as is default in Model.create), it is equivalent to AnyKeys<T>, as typescript favors the least restrictive type in the union if types overlap.

In other words, TRawDocType | AnyKeys<TRawDocType> means Partial<Record<keyof TRawDocType, any>>.

Motivation

Suppose you're storing something of the following shape;

{
  foo: {
    bar: string
    baz: number
  }
}

foo is a required property. It must be an object. It must have the key bar of type string, and it must have the key baz of type `number.

It is unhelpful for the type inference in the first argument to model.create to propose the shape;

{
  foo?: any
}

foo is an optional property. It can be anything at all.

The only way to get around this is to explicitly specify the type on every call to model.create or model.insertOne. This should not be needed. The correct type is already stored in the model via TRawDocType, specifying it again on every call is redundant.

Example

export interface Model/*...*/ {
  //...
-  create<DocContents = AnyKeys<TRawDocType>>(docs: Array<TRawDocType | DocContents>, options: CreateOptions & { aggregateErrors: true }): Promise<(THydratedDocumentType | Error)[]>;
-  create<DocContents = AnyKeys<TRawDocType>>(docs: Array<TRawDocType | DocContents>, options?: CreateOptions): Promise<THydratedDocumentType[]>;
-  create<DocContents = AnyKeys<TRawDocType>>(doc: DocContents | TRawDocType): Promise<THydratedDocumentType>;
-  create<DocContents = AnyKeys<TRawDocType>>(...docs: Array<TRawDocType | DocContents>): Promise<THydratedDocumentType[]>;
+  create(docs: Array<TRawDocType>, options: CreateOptions & { aggregateErrors: true }): Promise<(THydratedDocumentType | Error)[]>;
+  create(docs: Array<TRawDocType>, options?: CreateOptions): Promise<THydratedDocumentType[]>;
+  create(doc: TRawDocType): Promise<THydratedDocumentType>;
+  create(...docs: Array<TRawDocType>): Promise<THydratedDocumentType[]>;
  //...
-  insertOne<DocContents = AnyKeys<TRawDocType>>(doc: DocContents | TRawDocType, options?: SaveOptions): Promise<THydratedDocumentType>;
+  insertOne(doc: TRawDocType, options?: SaveOptions): Promise<THydratedDocumentType>;
}

Or, if it is still the case that TRawDocType is unreliable for whether a property is optional, Partial<TRawDocType> could be used. If its unreliable for nested keys too, a DeepPartial type (eg, ts-essentials DeepPartial) could be created and used.

WillsterJohnson avatar Apr 15 '25 19:04 WillsterJohnson

As i dont recall there having been a explicit issue about it and previous attempts at making the existing functions "strict" have been reverted or closed before merging, how about just adding new functions that are strict by default?

hasezoey avatar Apr 16 '25 11:04 hasezoey

That would work too - I'm fine with using a different name if it means stricter type safety

WillsterJohnson avatar Apr 20 '25 03:04 WillsterJohnson

Came here looking for a solution. The use of AnyKeys<T> makes .create() essentially useless for type safety.

Creating a new function to enforce types would defeat the purpose of type safety (why not just use .create<MyType>()?).

A better default would be for .create() to accept Partial<TRawDocType>, and fall back to a generic type only when necessary — i.e., .create<MyType>().

Suggested Signature:

/** Creates a new document or documents */
create<DocContents extends Partial<TRawDocType> = Partial<TRawDocType>>(
docs: Array<TRawDocType | DocContents>,
options: CreateOptions & { aggregateErrors: true }
): Promise<(THydratedDocumentType | Error)[]>;

create<DocContents extends Partial<TRawDocType> = Partial<TRawDocType>>(
docs: Array<TRawDocType | DocContents>,
options?: CreateOptions
): Promise<THydratedDocumentType[]>;

create<DocContents extends Partial<TRawDocType> = Partial<TRawDocType>>(
doc: DocContents | TRawDocType
): Promise<THydratedDocumentType>;

create<DocContents extends Partial<TRawDocType> = Partial<TRawDocType>>(
...docs: Array<TRawDocType | DocContents>
): Promise<THydratedDocumentType[]>;

This approach will avoid issues with document defaults like: _id beeing required at document creation and provide enough type enforcement to prevent most of the mistakes that can arise from current implementation

Juugis avatar May 07 '25 06:05 Juugis

+1. I would also like to have this, and not only for create and insertOne

nik-webdevelop avatar Jun 11 '25 05:06 nik-webdevelop

A better default would be for .create() to accept Partial<TRawDocType>

Accepting a partial would also be useless, as it would permit invalid data. If the field is optional the type would already specify that. If the type does not specify that, then the field is not optional.

Consider the example in my first post;

type MyData = {
  foo: {
    bar: string
    baz: number
  }
}

model.create({}) would be allowed in a Partial<MyData>, but would fail at runtime for not having the proper fields.

I simply don't see the use case for allowing incomplete data.

It's actually supposed to be 'incomplete'? Then the type should specify that a key is optional. The validation won't actually block it? Then it's supposed to be 'incomplete' - see above. The missing data is stuck behind a promise? await or .then.

I would strongly resist a potential solution which destroys type information, as the destruction of type information is the crux of the issue.

WillsterJohnson avatar Jun 12 '25 21:06 WillsterJohnson

@WillsterJohnson The problem is that required keys can be set by middleware, plugins, or Mongoose internals.

or example, __v and _id are "required" in the sense that they are always defined on a new document. const doc = new TestModel({ name: 'test' }) will always have a doc._id and doc.__v. But you don't need to explicitly set _id or __v when calling await TestModel.create().

vkarpov15 avatar Jun 14 '25 19:06 vkarpov15