fix(NODE-3770): Filter type uses WithId on the schema

[avaly]

Description

What is changing?

This is changing the internal definition of the Filter type to always apply WithId to the given TSchema.

FYI: I had to remove the old unused union schema unit tests, because it was throwing a TS error. I've reached out to @nbbeeken on another PR and he confirmed that we can remove those test assertions: #2972 (comment)

Is there new documentation needed for these changes?

No

What is the motivation for this change?

Closes https://jira.mongodb.org/browse/NODE-3770

See discussion in #3044

Double check the following

• Ran npm run check:lint script
• Self-review completed using the steps outlined here
• PR title follows the correct format: <type>(NODE-xxxx)<!>: <description>
• Changes are covered by tests
• New TODOs have a related JIRA ticket

[avaly]

@dariakp Can I please get any feedback on this PR?

I'd love if we can push this through soon, since the change introduced in 4.2.0 in the find Filter type is not consistent with the rest of the find* methods.

[dariakp]

@avaly Sorry for the delay, most of the team was out last week for the U.S. Thanksgiving holiday. We should be able to get this reviewed this week. Thanks for your patience!

[dariakp]

Thanks again for submitting this! I added some tests and removed unneeded WithId code from the find filter type.

[nbbeeken]

Thanks!

[xonuzofa]

How do I use the WithoutID option? I use strings, not ObjectIDs. I cannot upgrade to this version now because it breaks my code everywhere.

[xonuzofa]

There is a bug in the typings. When using the OptionalId to have it automatically use InferIdType, it is still making me use ObjectId. It will not pick up that my _id is a string

export interface LogMethodLatencyModel {
_id?: string;
__v?: number;
createdAt?: Date;
updatedAt?: Date;
date_start: Date;
date_end?: Date;
latency_ms: number;
method: string;
}

(method) Collection.deleteOne(filter?: Filter<Pick<LogMethodLatencyModel, "__v" | "createdAt" | "updatedAt" | "date_start" | "date_end" | "latency_ms" | "method"> & {
_id?: ObjectId;
}>, options?: FindOneAndDeleteOptions, bypassLogs?: boolean): Promise<...>

[dariakp]

@xonuzofa Hi there, thanks for reaching out. When specifying your own _id type in the collection model, you should not make it optional, because if you insert an object without specifying the _id, MongoDB will create an ObjectID type _id on your behalf, leading to an inconsistent schema in your database. The problem you are seeing is due to the way the types for the _id are currently reconciled (see NODE-3761), and you should not run into this issue if you specify the _id as required in your model.

[xonuzofa]

Even when I change this:

export interface LogMethodLatencyModel {
_id: string;
__v?: number;
createdAt?: Date;
updatedAt?: Date;
date_start: Date;
date_end?: Date;
latency_ms: number;
method: string;
}

I have a helper method here:

findById(id: string, options?: FindOptions<T>): Promise<T> { return new Promise((resolve, reject) => { ResolveIOServer.getMainDB().collection(this.collectionName).findOne<T>({_id: id}, options).then(res => { resolve(<T>res); }, err => { console.log(new Date(), 'Error Find By Id', this.collectionName, {_id: id}, options, err); reject(err); }); }); }

It gives me an error here for _id being a string and not an ObjectId:

No overload matches this call. Overload 1 of 11, '(filter: Filter<Document>, options?: FindOptions<Document>): Promise<T>', gave the following error. Type 'string' is not assignable to type 'Condition<ObjectId>'. Overload 2 of 11, '(filter: Filter<Document>, options?: FindOptions<Document>, callback?: Callback<T>): void', gave the following error. Type 'string' is not assignable to type 'Condition<ObjectId>'.ts(2769) mongodb.d.ts(5858, 5): The expected type comes from property '_id' which is declared here on type 'Filter<Document>' mongodb.d.ts(5858, 5): The expected type comes from property '_id' which is declared here on type 'Filter<Document>' (property) _id?: Condition<ObjectId>

[dariakp]

@xonuzofa It doesn't look like your helper method actually populates your intended schema type through the generic. Here are some examples from our typescript definition tests that demonstrate a custom _id with find: findX.test-d.ts.

If you need further help with your specific use case, I recommend asking in our community forums, as we try to limit github discussions to active PRs for driver bugs and feature requests.