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

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

Open
2 tasks done
shawnmcknight opened this issue Jan 30, 2023 · 1 comment
Open
2 tasks done
Labels
typescript Types or Types-test related issue / Pull Request
Milestone

Comments

@shawnmcknight
Copy link

shawnmcknight commented Jan 30, 2023

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.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<HydratedDocumentFromSchema<typeof schema>> 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

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<typeof schema>;
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<typeof schema>;
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<Hydrated>;
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.
@vkarpov15 vkarpov15 added this to the TypeScript backlog milestone Feb 6, 2023
@IslandRhythms IslandRhythms added the typescript Types or Types-test related issue / Pull Request label Feb 7, 2023
@vkarpov15 vkarpov15 modified the milestones: TypeScript backlog, 8.6.3 Sep 11, 2024
@vkarpov15 vkarpov15 modified the milestones: 8.6.3, 8.7 Sep 16, 2024
vkarpov15 added a commit that referenced this issue Sep 17, 2024
types: make __v a number, only set __v on top-level documents
@vkarpov15 vkarpov15 modified the milestones: 8.7, 8.8 Sep 17, 2024
vkarpov15 added a commit that referenced this issue Oct 27, 2024
@vkarpov15
Copy link
Collaborator

It looks like the _id widening issue was fixed in 8.6.0, here's the passing tests (passing commit is here). We will look into backporting that.

vkarpov15 added a commit that referenced this issue Oct 29, 2024
types: add __v to lean() result type and ModifyResult
@vkarpov15 vkarpov15 modified the milestones: 8.8, 8.9 Oct 29, 2024
@vkarpov15 vkarpov15 modified the milestones: 8.9, 8.10 Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
typescript Types or Types-test related issue / Pull Request
Projects
None yet
Development

No branches or pull requests

3 participants