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

TS: _id not required on DocumentArray properties of documents returned from query #14660

Closed
1 task done
alukach opened this issue Jun 10, 2024 · 6 comments · Fixed by #14735
Closed
1 task done

TS: _id not required on DocumentArray properties of documents returned from query #14660

alukach opened this issue Jun 10, 2024 · 6 comments · Fixed by #14735
Labels
help wanted help This issue can likely be resolved in GitHub issues. No bug fixes, features, or docs necessary
Milestone

Comments

@alukach
Copy link

alukach commented Jun 10, 2024

Prerequisites

  • I have written a descriptive issue title

Mongoose version

8.2.3

Node.js version

20.14.0

MongoDB version

?

Operating system

macOS

Operating system version (i.e. 20.04, 11.3, 10)

14.1

Issue

I have a Mongoose model which has a subdocument. I have a function that queries the model via the .find() method. The function's output is expected to conform to an output interface, however it is not playing nicely with the the auto-generated TypeScript types for the model. The mongoose docs state: "Each subdocument has an _id by default", however the types for the DocumentArray properties returned from a find() operation do not have _id marked as required.

This feels like a bug on the Mongoose side but I am new to using the library so it's possible that I'm missing something.

Questions:

  • Am I correct that it is safe to assume that any subdocuments will have an _id property when a document is retrieved via the .find() operations? (safe for any situations where I explicitly instruct Mongoose to not place an _id on the subdocument, of course)
  • Short of a change in mongoose code itself, how can I customize the output of the types in a succinct manner? I'd prefer to not write my own types for the Schema and Model as mongoose does a good job inferring these values and I would like to avoid maintaining redundant code. I see that .find() is a generic function (link), so I could provide my own type that instructs mongoose that the _id is required, however I'm struggling to achieve that.

Simplified example:

import mongoose from 'mongoose';

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

const ParentSchema = new mongoose.Schema({
  _id: { type: String, required: true },
  name: { type: String, required: true },
  parts: { type: [ChildSchema] },
});

const ParentModel = mongoose.model('Parent', ParentSchema);


export const getParents = async (): Promise<ExpectedParent[]> => {
  const parents = await ParentModel.find();
  return parents;  // TypeScript error here!
};

interface ExpectedParent {
  _id: string;
  name: string;
  parts: Array<{ _id: string; name: string }>;
}

I see the following error from TS:

Type '(Document<unknown, {}, { name: string; _id: string; parts: DocumentArray<{ name: string; }>; }> & { name: string; _id: string; parts: DocumentArray<{ name: string; }>; } & Required<...>)[]' is not assignable to type 'ExpectedParent[]'.
  Type 'Document<unknown, {}, { name: string; _id: string; parts: DocumentArray<{ name: string; }>; }> & { name: string; _id: string; parts: DocumentArray<{ name: string; }>; } & Required<...>' is not assignable to type 'ExpectedParent'.
    The types returned by 'parts.pop()' are incompatible between these types.
      Type '(Subdocument<ObjectId> & { name: string; }) | undefined' is not assignable to type '{ _id: string; name: string; } | undefined'.
        Type 'Subdocument<ObjectId> & { name: string; }' is not assignable to type '{ _id: string; name: string; } | undefined'.
          Type 'Subdocument<ObjectId> & { name: string; }' is not assignable to type '{ _id: string; name: string; }'.
            Types of property '_id' are incompatible.
              Type 'ObjectId | undefined' is not assignable to type 'string'.
                Type 'undefined' is not assignable to type 'string'.ts(2322)
@alukach alukach added help This issue can likely be resolved in GitHub issues. No bug fixes, features, or docs necessary help wanted labels Jun 10, 2024
@alukach
Copy link
Author

alukach commented Jun 10, 2024

I could do something like this:

export const getParents = async (): Promise<ExpectedParent[]> => {
  const parents = await ParentModel.find<ExpectedParent>();
  return parents;
};

However, this seems a bit heavy-handed as I am now overriding the entirety of the types stored within the mongoose schemas.

I ended up landing on this utility type:

// Recursive type to enforce `_id` in all subdocuments
type EnforceIdInSubdocuments<T> = {
  [K in keyof T]: T[K] extends mongoose.Types.DocumentArray<infer U>
    ? mongoose.Require_id<U>[]
    : T[K] extends object
    ? EnforceIdInSubdocuments<T[K]>
    : T[K];
};

// Utility type to extract and enforce _id in subdocuments
type ModelWithEnforcedIds<T extends mongoose.Model<any>> = EnforceIdInSubdocuments<
  mongoose.InferSchemaType<T['schema']>
>;

Full example:

import mongoose from 'mongoose';

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

const ParentSchema = new mongoose.Schema({
  _id: { type: String, required: true },
  name: { type: String, required: true },
  parts: { type: [ChildSchema] },
});

const ParentModel = mongoose.model('Parent', ParentSchema);


export const getParents = async (): Promise<ExpectedParent[]> => {
  const parents = await ParentModel.find<ModelWithEnforcedIds<typeof ParentModel>>();
  return parents;
};

interface ExpectedParent {
  _id: string;
  name: string;
  parts: Array<{ _id: string; name: string }>;
}

// Recursive type to enforce `_id` in all subdocuments
type EnforceIdInSubdocuments<T> = {
  [K in keyof T]: T[K] extends mongoose.Types.DocumentArray<infer U>
    ? mongoose.Require_id<U>[]
    : T[K] extends object
    ? EnforceIdInSubdocuments<T[K]>
    : T[K];
};

// Utility type to extract and enforce _id in subdocuments
type ModelWithEnforcedIds<T extends mongoose.Model<any>> = EnforceIdInSubdocuments<
  mongoose.InferSchemaType<T['schema']>
>;

I will leave this open as I feel like there's probably a better way to do this.

@vkarpov15
Copy link
Collaborator

This looks like expected behavior: you expect your parts subdocs to have _id: string, but you don't define an _id property in ChildSchema, so Mongoose assumes that parts subdocs have ObjectId _id's. To make this script compile correctly, just add an _id property below:

const ChildSchema = new mongoose.Schema({
  _id: { type: String, required: true }, // <-- add _id here
  name: { type: String, required: true },
});

Or, if you intend to have ObjectId _id for ChildSchema, do the following:

const ChildSchema = new mongoose.Schema({
  _id: { type: 'ObjectId', required: true }, // <-- change _id type here
  name: { type: String, required: true },
});

interface ExpectedParent {
  _id: string;
  name: string;
  parts: Array<{ _id: mongoose.Types.ObjectId; name: string }>; // <-- change _id type here
}

Does this help?

@alukach
Copy link
Author

alukach commented Jun 17, 2024

Thanks for the reply @vkarpov15.

This looks like expected behavior: you expect your parts subdocs to have _id: string, but you don't define an _id property in ChildSchema, so Mongoose assumes that parts subdocs have ObjectId _id's

From my vantage point, the issue is that Mongoose's types do not assume that the parts subdocs have ObjectId _id's.

As you are aware, each subdocument has an _id by default. However, the TS logic does not demonstrate this. I wouldn't classify that as "expected behavior". I do understand that I could change my schema to coerce the type system to acknowledge an _id property, but that seems more like a work-around. If the Mongoose docs & code are written so that an _id is to be assumed, I believe the corresponding types should reflect that (hence my view that this is a bug).

@vkarpov15
Copy link
Collaborator

Mongoose does add an ObjectId _id property by default, but ExpectedParent in your original post has _id as a string

@vkarpov15 vkarpov15 removed this from the 8.4.5 milestone Jul 2, 2024
@alukach
Copy link
Author

alukach commented Jul 2, 2024

Mongoose does add an ObjectId _id property by default, but ExpectedParent in your original post has _id as a string

What you said is true, however I don't believe that it's particularly important. If we review the last two lines of the TS error output, we see:

              Type 'ObjectId | undefined' is not assignable to type 'string'.
                Type 'undefined' is not assignable to type 'string'.ts(2322)

The issue is about the _id being possibly undefined, not about string vs ObjectId (it seems that Mongoose impressively handles ObjectId -> string very well).

To demonstrate this, we can update the example to something like this:

import mongoose, { Mongoose } from 'mongoose';

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

const ParentSchema = new mongoose.Schema({
  // _id: { type: String, required: true },
  name: { type: String, required: true },
  parts: { type: [ChildSchema] },
});

const ParentModel = mongoose.model('Parent', ParentSchema);

export const getParents = async (): Promise<ExpectedParent[]> => {
  const parents = await ParentModel.find();
  return parents; // TypeScript error here!
};

interface ExpectedParent {
  _id: mongoose.Types.ObjectId;
  name: string;
  parts: Array<{ _id: mongoose.Types.ObjectId; name: string }>;
}

which results in the following error:

Type '(Document<unknown, {}, { name: string; parts: DocumentArray<{ name: string; }>; }> & { name: string; parts: DocumentArray<{ name: string; }>; } & { _id: ObjectId; })[]' is not assignable to type 'ExpectedParent[]'.
  Type 'Document<unknown, {}, { name: string; parts: DocumentArray<{ name: string; }>; }> & { name: string; parts: DocumentArray<{ name: string; }>; } & { _id: ObjectId; }' is not assignable to type 'ExpectedParent'.
    The types returned by 'parts.pop()' are incompatible between these types.
      Type '(Subdocument<ObjectId> & { name: string; }) | undefined' is not assignable to type '{ _id: ObjectId; name: string; } | undefined'.
        Type 'Subdocument<ObjectId> & { name: string; }' is not assignable to type '{ _id: ObjectId; name: string; } | undefined'.
          Type 'Subdocument<ObjectId> & { name: string; }' is not assignable to type '{ _id: ObjectId; name: string; }'.
            Types of property '_id' are incompatible.
              Type 'ObjectId | undefined' is not assignable to type 'ObjectId'.
                Type 'undefined' is not assignable to type 'ObjectId'.ts(2322)

The core issue is that subdocument arrays come back with possible undefined ID values.

@vkarpov15 vkarpov15 added this to the 8.4.6 milestone Jul 6, 2024
@vkarpov15
Copy link
Collaborator

Workaround is to define _id in your schema as follows:

const ChildSchema = new mongoose.Schema({
  _id: { type: mongoose.Schema.ObjectId, required: true, default: () => new mongoose.Types.ObjectId() },
  name: { type: String, required: true },
});

We're investigating to see if we can fix this issue

vkarpov15 added a commit that referenced this issue Jul 9, 2024
vkarpov15 added a commit that referenced this issue Jul 10, 2024
types: make `_id` required on Document type
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted help This issue can likely be resolved in GitHub issues. No bug fixes, features, or docs necessary
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants