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

ObjectId is different from Type.ObjectId #12711

Closed
1 task done
DuJiming opened this issue Nov 21, 2022 · 7 comments
Closed
1 task done

ObjectId is different from Type.ObjectId #12711

DuJiming opened this issue Nov 21, 2022 · 7 comments
Labels
help This issue can likely be resolved in GitHub issues. No bug fixes, features, or docs necessary typescript Types or Types-test related issue / Pull Request

Comments

@DuJiming
Copy link

DuJiming commented Nov 21, 2022

Prerequisites

  • I have written a descriptive issue title

Mongoose version

6.7.2

Node.js version

18.7.0

MongoDB version

6.0

Operating system

macOS

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

13.0.1

Issue

I want parse string to ObjectId, and Previous versions are OK

"@nestjs/common": "^7.6.15"
"mongoose": "^5.11.12"

but there are some type errors when i upgrade version

"@nestjs/common": "^9.2.0"
"mongoose": "^6.7.2"

code:

import { PipeTransform, Injectable, BadRequestException } from '@nestjs/common';
import { Types } from 'mongoose';

@Injectable()
export class ParseObjectIdPipe implements PipeTransform<any, Types.ObjectId> {
  transform(value: any): Types.ObjectId {
    const validObjectId: boolean = Types.ObjectId.isValid(value);
    if (!validObjectId) {
      throw new BadRequestException('Invalid ObjectId');
    }
    return Types.ObjectId.createFromHexString(value);
  }
}

error:

type "import("/***/node_modules/bson/bson").ObjectID" Missing attribute in "_id",but type "import("mongoose").Types.ObjectId" is required in。

I have to use

import { PipeTransform, Injectable, BadRequestException } from '@nestjs/common';
import { Types } from 'mongoose';
import { ObjectId } from 'bson';

@Injectable()
export class ParseObjectIdPipe implements PipeTransform<any, ObjectId> {
  transform(value: any): ObjectId {
    const validObjectId: boolean = Types.ObjectId.isValid(value);
    if (!validObjectId) {
      throw new BadRequestException('Invalid ObjectId');
    }
    return Types.ObjectId.createFromHexString(value);
  }
}

to fix it
Is that right?

I saw a problem like me

#12537

@DuJiming DuJiming added help This issue can likely be resolved in GitHub issues. No bug fixes, features, or docs necessary help wanted labels Nov 21, 2022
@DuJiming DuJiming changed the title A type error A ObjectId type error Nov 21, 2022
@DuJiming DuJiming changed the title A ObjectId type error ObjectId is different from Type.ObjectId Nov 22, 2022
@vkarpov15 vkarpov15 added this to the 6.7.7 milestone Nov 28, 2022
@vkarpov15 vkarpov15 added needs repro script Maybe a bug, but no repro script. The issue reporter should create a script that demos the issue and removed help This issue can likely be resolved in GitHub issues. No bug fixes, features, or docs necessary help wanted labels Nov 28, 2022
@hasezoey
Copy link
Collaborator

hasezoey commented Dec 2, 2022

type "import("/***/node_modules/bson/bson").ObjectID" Missing attribute in "_id",but type "import("mongoose").Types.ObjectId" is required in。

are you sure you are using Types.ObjectId everywhere and not from bson directly? because mongoose's ObjectId is compatible with bson's, but not the other way around - at least in terms of types, because mongoose adds a extra property _id.

also did you already check that you not accidentally have multiple versions of mongoose installed? (yarn why mongoose / npm ls mongoose)

@hasezoey hasezoey added the typescript Types or Types-test related issue / Pull Request label Dec 2, 2022
@IslandRhythms IslandRhythms removed the needs repro script Maybe a bug, but no repro script. The issue reporter should create a script that demos the issue label Dec 5, 2022
@IslandRhythms
Copy link
Collaborator

import { PipeTransform, Injectable, BadRequestException } from '@nestjs/common';
import { Types } from 'mongoose';
import { ObjectId } from 'bson';

@Injectable()
export class ParseObjectIdPipe implements PipeTransform<any, ObjectId> {
  transform(value: any): ObjectId {
    const validObjectId: boolean = Types.ObjectId.isValid(value);
    if (!validObjectId) {
      throw new BadRequestException('Invalid ObjectId');
    }
    return Types.ObjectId.createFromHexString(value);
  }
}
/*
@Injectable()
export class ParseObjectIdPipe implements PipeTransform<any, Types.ObjectId> {
  transform(value: any): Types.ObjectId {
    const validObjectId: boolean = Types.ObjectId.isValid(value);
    if (!validObjectId) {
      throw new BadRequestException('Invalid ObjectId');
    }
    return Types.ObjectId.createFromHexString(value);
  }
}
*/

@DuJiming
Copy link
Author

DuJiming commented Dec 6, 2022

type "import("/***/node_modules/bson/bson").ObjectID" Missing attribute in "_id",but type "import("mongoose").Types.ObjectId" is required in。

are you sure you are using Types.ObjectId everywhere and not from bson directly? because mongoose's ObjectId is compatible with bson's, but not the other way around - at least in terms of types, because mongoose adds a extra property _id.

also did you already check that you not accidentally have multiple versions of mongoose installed? (yarn why mongoose / npm ls mongoose)

I think I can roughly understand that both types can be used, but only one can be used. I can't mix them. I was just wondering why the types are incompatible after upgrading

for example Types.ObjectId.createFromHexString() return bson.ObjectId

Thanks for the reminder

@DuJiming
Copy link
Author

DuJiming commented Dec 6, 2022

like this, The import of Type.ObjectId is completely unnecessary

import { PipeTransform, Injectable, BadRequestException } from '@nestjs/common';
import { ObjectId } from 'bson';

@Injectable()
export class ParseObjectIdPipe implements PipeTransform<any, ObjectId> {
  transform(value: any): ObjectId {
    const validObjectId: boolean = ObjectId.isValid(value);
    if (!validObjectId) {
      throw new BadRequestException('Invalid ObjectId');
    }
    return ObjectId.createFromHexString(value);
  }
}

@hasezoey
Copy link
Collaborator

hasezoey commented Dec 6, 2022

I was just wondering why the types are incompatible after upgrading

that is because back then the mongoose ObjectId interface did not add the helper _id as a type yet

interface ObjectId extends mongodb.ObjectID { }

but now it does and the type is not defined as optional, so the types are only one-way compatible in terms of types

class ObjectId extends mongodb.ObjectId {
_id: this;
}

i dont know if the _id helper at runtime is a added by mongoose or bson, if it is bson then they could update their types to include it so mongoose's type can be two-way compatible again

for example Types.ObjectId.createFromHexString() return bson.ObjectId

just as a note, you can also just use new Types.ObjectId(string) which will also create a new ObjectId instance from the hex string (with the proper mongoose type)

@DuJiming
Copy link
Author

DuJiming commented Dec 7, 2022

new Types.ObjectId(string)

yes new Types.ObjectId(string) is better , think you

@vkarpov15
Copy link
Collaborator

I took a closer look and @hasezoey is right, this is due to the _id getter. Here's a simpler repro script that doesn't involve nestjs:

import { Types } from 'mongoose';
import { ObjectId } from 'bson';

function foo(value: any): Types.ObjectId {
  return ObjectId.createFromHexString(value);
}

Mongoose adds an _id getter to ObjectIds to make it more convenient to work with populated paths, so you can use .path._id to get the _id of the document regardless of whether it is populated. But there's no good way for Mongoose to add additional properties to a MongoDB class in TypeScript, so Mongoose's ObjectId type extends from MongoDB driver's.

@hasezoey is right, no need to mix bson ObjectId and Mongoose ObjectId anyway. You can do new Types.ObjectId(). Or Types.ObjectId.createFromHexString() if you really want.

@vkarpov15 vkarpov15 removed this from the 6.8.2 milestone Dec 23, 2022
@vkarpov15 vkarpov15 added the help This issue can likely be resolved in GitHub issues. No bug fixes, features, or docs necessary label Dec 23, 2022
ziedHamdi pushed a commit to ziedHamdi/user-credits that referenced this issue Oct 5, 2023
  ● Test suite failed to run

    src/impl/mongoose/dao/MongooseDaoFactory.ts:45:5 - error TS2322: Type 'OfferDao' is not assignable to type 'IOfferDao<ObjectId, IOffer<ObjectId>>'.
      Types of property 'loadSubOffers' are incompatible.
        Type '(parentOfferId: ObjectId) => Promise<IMongooseOffer[]>' is not assignable to type '(parentOfferId: ObjectId) => Promise<IOffer<ObjectId>[]>'.
          Types of parameters 'parentOfferId' and 'parentOfferId' are incompatible.
            Type 'ObjectId' is missing the following properties from type 'ObjectId': _bsontype, id, toHexString, toJSON, and 3 more.

    45     return this.offerDao;

 Known issue:
 Automattic/mongoose#12711
 Automattic/mongoose#12537
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help This issue can likely be resolved in GitHub issues. No bug fixes, features, or docs necessary typescript Types or Types-test related issue / Pull Request
Projects
None yet
Development

No branches or pull requests

4 participants