Definitelytyped: @types/mongodb _id is not necessarily typeof ObjectId

[smeijer]

There are quite a few places that define id being an ObjectID, which isn't necessarily true. When using meteorjs, id's are strings. But when just executing collection.insertOne({ _id: x }), id can be a number, string, object or ObjectID.

There is one place in the definition that already defines this logic, but it isn't being used everywhere.

https://github.com/DefinitelyTyped/DefinitelyTyped/blob/c15bcd2e68aa2c9c3172adfae9b262ad67594daf/types/mongodb/index.d.ts#L2062

Not even where I would expected it to be used:

https://github.com/DefinitelyTyped/DefinitelyTyped/blob/c15bcd2e68aa2c9c3172adfae9b262ad67594daf/types/mongodb/index.d.ts#L1943-L1946

I propose to add an union type for the ObjectID, and use that type for all _id or id references. That is; also upsertedId, insertedId, etc.

import { ObjectID as BsonObjectID } from 'bson';

type ObjectID = string | number | object | BsonObjectID;

What do you think?

---

cc:

@CaselIT, @alanmarcell, @bitjson, @dante-101, @mcortesi, @EnricoPicci, @AJCStriker, @julien-c, @daprahamian, @denys-bushulyak, @sindbach, @geraldinelemeur, @jishi, @various89, @angela-1, @lirbank, @hector7, @floric, @erikc5000, @Manc, @jloveridge, @Ranguna

[smeijer]

Or would it be possible to define _id (and insertedId, upsertedId, ...) as TSchema['_id']?. I guess that would be a better option.

[Ranguna]

Hey, I usually go with Types.ObjectId(id) whenever I use mongoose/mongodb but I've read somewhere that some functions actually accept the types you listed.

It would be great to have a reference to mongodb's documentation/source stating/showing that those types are actually accepted, just so we can be sure that they really are.

[CaselIT]

An alternative could be adding a type parameter with a default type for the id, so user can customize it if they want. Something like

export class GridFSBucket<IdType = ObjectID> { 
     constructor(db: Db, options?: GridFSBucketOptions); 
     /** http://mongodb.github.io/node-mongodb-native/3.1/api/GridFSBucket.html#delete */ 
     delete(id: IdType, callback?: GridFSBucketErrorCallback): void; 

I think also using the union type that you suggest would be ok. I would not call it ObjectID though, since it can lead to confusion if one is reading the type definition.

[smeijer]

Sorry. Lost track of this one.

@Ranguna, See here: https://docs.mongodb.com/manual/core/document/#field-names

The field name _id is reserved for use as a primary key; its value must be unique in the collection, is immutable, and may be of any type other than an array.

@CaselIT, that suggestion sounds fine to me. Just note that Collection already accepts a schema type. So that would require a second generic for the _id field?

I believe your approach is preferred, as that prevents the need for type checking/casting.

https://github.com/DefinitelyTyped/DefinitelyTyped/blob/c15bcd2e68aa2c9c3172adfae9b262ad67594daf/types/mongodb/index.d.ts#L850-L854

export interface Collection<TSchema = Default, TID = ObjectID> { 

[HosseinAgha]

@smeijer I think this is fixed now in the latest version. We use something with the following logic TSchema['_id'] ? TSchema['_id'] : ObjectId for insertedId, etc.