Automattic / Automattic/mongoose

Unexpected TS types for _id and _v in inferred types, particularly for subdocument arrays

Open
#12,959 2 comments 0 reactions 0 assignees View on GitHub
typescript
Dominant language
JavaScript
Stars
27.5k
Forks
4k
Avg merge
2d 7h
Merged PRs (30d)
35

Description

### Prerequisites

- [X] I have written a descriptive issue title
- [X] I have searched existing issues to ensure the bug has not already been reported

### Mongoose version

6.9.0

### Node.js version

16.18.1

### MongoDB server version

4.4.18

### Typescript version (if applicable)

4.9.4

### Description

I'm encountering a few unexpected behaviors with the types being inferred from both schema helpers (`InferSchemaType` and `HydratedDocumentFromSchema`), the resulting types when executing a model method, and the resulting types when converting that result to a POJO., particularly as it pertains to subdocument arrays. The included sample code hopefully tells the story, but this is a summary of what I have found that _feels_ incorrect.

### Subdocument Array inference is returning types for `_id` and `__v` when using `InferSchemaType`
This is less of an error than a surprise based on what I read in #11967. As per that issue, `InferSchemaType` does not return an `_id` type. Although it's not indicated in that issue, it also doesn't have a type for `__v` but that seems to be consistent with not having an `_id` type. However, I was very surprised to see that subdocument arrays DO have a type for the `_id` and the `__v` value. It seems like neither the parent document nor the subdocuments should have these types.

### Hydrated documents have wide types for `_id` and `__v` for subdocument arrays
I ran my schema through `HydratedDocumentFromSchema`. This correctly provided a type of `mongoose.types.ObjectId` for the parent document's `_id` property. However, the subdocument arrays have a type of `mongoose.Types.ObjectId | undefined`. I would have not expected this to be an optional property.

Additionally, the types for `__v` on both the parent document as well as the subdocuments is `any` with the optional flag. I traced through some history and it seems like `any` was put there instead of number to try to avoid some other issues. I was surprised to see the optional flag on this type, although with `any` it just widens it to `any`, so maybe it doesn't matter. If this type ever was able to be more accurate and be `number` then I wouldn't think this should be `number | undefined`.

### Document instance exhibits the same types as the `HydratedDocumentFromSchema`
This seems to track as I would expect them to be consistent with each other. However, all of what I described that feels off in that section also applies to the document instances.

### Document instance converted to POJO additionally lacks a type for `__v`
After running a document instance through `toObject()`, the resulting object does not have a type for the `__v` property on the parent document, although it is a valid property on the subdocuments. This surprised me, because a type of `LeanDocument>` does have an `__v` property, so there is a behavioral difference between those two things.

The POJO also continues to have the same types as `HydratedDocumentFromSchema` otherwise the things I feel like are off with that are also off here.

### Steps to Reproduce

```ts
import type { HydratedDocumentFromSchema, InferSchemaType, LeanDocument } from 'mongoose';
import mongoose, { Schema } from 'mongoose';

const subdocSchema = new Schema({ foo: { type: 'string', required: true } });

const schema = new Schema({
subdocArray: { type: [subdocSchema], required: true },
});

type Inferred = InferSchemaType;
type InferrredId = Inferred['_id']; // ERROR (expected per #11967)
type InferredSubdocArrayId = Inferred['subdocArray'][number]['_id']; // Surprised -- mongoose.Types.ObjectId | undefined
type InferredVersion = Inferred['__v']; // ERROR (seems to be the same as #11967)
type InferredSubdocArrayVersion = Inferred['subdocArray'][number]['__v']; // Surprised -- any | undefined (widens to any)

type Hydrated = HydratedDocumentFromSchema;
type HydratedId = Hydrated['_id']; // mongoose.Types.ObjectId (expected per #11967)
type HydratedSubdocArrayId = Hydrated['subdocArray'][number]['_id']; // mongoose.Types.ObjectId | undefined
type HydratedVersion = Hydrated['__v']; // any | undefined
type HydratedSubdocArrayVersion = Hydrated['subdocArray'][number]['__v']; // any | undefined (widens to any)

type LeanHydrated = LeanDocument;
type LeanHydratedId = LeanHydrated['_id']; // mongoose.Types.ObjectId
type LeanHydratedSubdocArrayId = LeanHydrated['subdocArray'][number]['_id']; // mongoose.Types.ObjectId | undefined
type LeanHydratedVersion = LeanHydrated['__v']; // any | undefined
type LeanHydratedSubdocArrayVersion = LeanHydrated['subdocArray'][number]['__v']; // any | undefined (widens to any)

const Model = mongoose.model('test', schema);

const document = Model.findById('id')
.then((result) => {
if (result == null) {
return Promise.reject();
}
console.log(result._id); // mongoose.Types.ObjectId
console.log(result.__v); // any | undefined (widens to any);
console.log(result.subdocArray[0]._id); // mongoose.Types.ObjectId | undefined
console.log(result.subdocArray[0].__v); // any | undefined (widens to any);
return result.toObject();
})
.then((resultPOJO) => {
console.log(resultPOJO._id); // mongoose.Types.ObjectId
console.log(resultPOJO.__v); // ERROR (seems like this should be there?)
console.log(resultPOJO.subdocArray[0]._id); // mongoose.Types.ObjectID | undefined
console.log(resultPOJO.subdocArray[0].__v); // any | undefined (widens to any)
});

```

### Expected Behavior

1. Neither parent documents not subdocuments should have types for `_id` and `__v` properties for consistency when using `InferSchemaType`.
2. The type of `_id` in a subdocument should be based on the schema options value for `_id`. If the option is set to `false` then there should be no `_id` property on a subdocument. If the value is set to `true` or left as default then `_id` should not be optional and should be an ObjectId. This applies to the inferred Hydrated types as well as the document instance types.
3. The type of `__v` in both parent and subdocuments should be based on the schema options value for `skipVersioning`. If `skipVersioning` is `true` then there should be no `__v` property. If `skipVersioning` is `false` or left as default then `__v` should be a `number`. This applies to the inferred Hydrated types as well as the document instance types.
4. `__v` should be a valid property for documents converted to POJO format.

Contributor guide

Open the contributing guide

Research direction

Start with the reproduced type checks for InferSchemaType, HydratedDocumentFromSchema, LeanDocument, and the document toObject() result. Trace how subdocument _id and __v are inferred from schema options such as _id and skipVersioning. Done means the expected presence, absence, and types for _id and __v are consistent across inferred, hydrated, document-instance, and POJO results.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, node.js, typescript
Domain
backend, databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.