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

Why is the default type of the generic field _id in the class Document of type "any"? #14520

Closed
1 task done
a1rwulf opened this issue Apr 12, 2024 · 0 comments · Fixed by #14541
Closed
1 task done
Labels
typescript Types or Types-test related issue / Pull Request
Milestone

Comments

@a1rwulf
Copy link

a1rwulf commented Apr 12, 2024

Prerequisites

  • I have written a descriptive issue title

Mongoose version

8.3.1

Node.js version

20.12.1

MongoDB version

6.0

Operating system

macOS

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

14.2.1 (23C71)

Issue

I just fell victim to an issue similar to:
#11085

Which was caused by legacy code we've been using in NestJS:
nestjs/docs.nestjs.com#2517

After researching this for quite some time, I undersand we are supposed to use the HydratedDocument these days, but I'd still be nice if somebody can shade some light into why it is what it is at the moment.

So here is my example:
Our schemas are defined like this (which is how it was found in NestJS docs, a few years ago):

import { Prop, Schema, SchemaFactory } from '@nestjs/mongoose';
import { Document } from 'mongoose';
import { Types } from 'mongoose';

@Schema()
export class Cat {
  _id: Types.ObjectId;

  @Prop()
  name: string;

  @Prop()
  age: number;

  @Prop()
  breed: string;
}

export type CatDocument = Cat & Document;
export const CatSchema = SchemaFactory.createForClass(Cat);

The interesting part is here is:
export type CatDocument = Cat & Document;

Which basically makes _id be of type any instead of Types.ObjectId.
As per above issue, I wonder why document is defined as class Document<T = any, TQueryHelpers = any, DocType = any> in https://github.com/Automattic/mongoose/blob/master/types/document.d.ts#L19 while the HydratedDocument was changed to use unknown to circumvent the problem with any inference.

Wouldn't it make sense to either use unkown or class Document<T = Types.ObjectId, TQueryHelpers = any, DocType = any> to default to ObjectId and leave the user a way to use any instead?

Thanks for any answers to this question.

@a1rwulf a1rwulf added help This issue can likely be resolved in GitHub issues. No bug fixes, features, or docs necessary help wanted labels Apr 12, 2024
@vkarpov15 vkarpov15 added this to the 8.3.3 milestone Apr 20, 2024
@vkarpov15 vkarpov15 added typescript Types or Types-test related issue / Pull Request and removed help This issue can likely be resolved in GitHub issues. No bug fixes, features, or docs necessary help wanted labels Apr 20, 2024
@vkarpov15 vkarpov15 modified the milestones: 8.3.3, 8.4 Apr 26, 2024
vkarpov15 added a commit that referenced this issue Apr 27, 2024
vkarpov15 added a commit that referenced this issue Apr 27, 2024
types(document): make document _id type default to unknown instead of any
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

Successfully merging a pull request may close this issue.

2 participants