From 9b9e803187b647cb49bff948cc0c8e2d8ff823a0 Mon Sep 17 00:00:00 2001 From: Paul Endri Date: Tue, 16 Nov 2021 10:56:10 -0500 Subject: [PATCH 1/2] types: add withId to collection.find and collection.findOne --- src/collection.ts | 36 +++++----- .../collection/filterQuery.test-d.ts | 22 ++++--- .../community/collection/findX.test-d.ts | 66 ++++++++++++------- test/types/mongodb.test-d.ts | 4 +- test/types/union_schema.test-d.ts | 4 +- 5 files changed, 80 insertions(+), 52 deletions(-) diff --git a/src/collection.ts b/src/collection.ts index 471fa3e713..598af08d3b 100644 --- a/src/collection.ts +++ b/src/collection.ts @@ -676,12 +676,16 @@ export class Collection { * @param options - Optional settings for the command * @param callback - An optional callback, a Promise will be returned if none is provided */ - findOne(): Promise; - findOne(callback: Callback): void; - findOne(filter: Filter): Promise; - findOne(filter: Filter, callback: Callback): void; - findOne(filter: Filter, options: FindOptions): Promise; - findOne(filter: Filter, options: FindOptions, callback: Callback): void; + findOne(): Promise | null>; + findOne(callback: Callback | null>): void; + findOne(filter: Filter): Promise | null>; + findOne(filter: Filter, callback: Callback | null>): void; + findOne(filter: Filter, options: FindOptions): Promise | null>; + findOne( + filter: Filter, + options: FindOptions, + callback: Callback | null> + ): void; // allow an override of the schema. findOne(): Promise; @@ -695,10 +699,10 @@ export class Collection { ): void; findOne( - filter?: Filter | Callback, - options?: FindOptions | Callback, - callback?: Callback - ): Promise | void { + filter?: Filter | Callback | null>, + options?: FindOptions | Callback | null>, + callback?: Callback | null> + ): Promise | null> | void { if (callback != null && typeof callback !== 'function') { throw new MongoInvalidArgumentError( 'Third parameter to `findOne()` must be a callback or undefined' @@ -706,7 +710,7 @@ export class Collection { } if (typeof filter === 'function') { - callback = filter as Callback; + callback = filter as Callback | null>; filter = {}; options = {}; } @@ -725,10 +729,10 @@ export class Collection { * * @param filter - The filter predicate. If unspecified, then all documents in the collection will match the predicate */ - find(): FindCursor; - find(filter: Filter, options?: FindOptions): FindCursor; - find(filter: Filter, options?: FindOptions): FindCursor; - find(filter?: Filter, options?: FindOptions): FindCursor { + find(): FindCursor>; + find(filter: Filter>, options?: FindOptions): FindCursor>; + find(filter: Filter>, options?: FindOptions): FindCursor; + find(filter?: Filter>, options?: FindOptions): FindCursor> { if (arguments.length > 2) { throw new MongoInvalidArgumentError( 'Method "collection.find()" accepts at most two arguments' @@ -738,7 +742,7 @@ export class Collection { throw new MongoInvalidArgumentError('Argument "options" must not be function'); } - return new FindCursor( + return new FindCursor>( getTopology(this), this.s.namespace, filter, diff --git a/test/types/community/collection/filterQuery.test-d.ts b/test/types/community/collection/filterQuery.test-d.ts index 7902002a90..95fd1035c4 100644 --- a/test/types/community/collection/filterQuery.test-d.ts +++ b/test/types/community/collection/filterQuery.test-d.ts @@ -1,6 +1,6 @@ import { BSONRegExp, Decimal128, ObjectId } from 'bson'; import { expectAssignable, expectNotType, expectType } from 'tsd'; -import { Filter, MongoClient } from '../../../../src'; +import { Filter, MongoClient, WithId } from '../../../../src'; /** * test the Filter type using collection.find() method @@ -57,12 +57,16 @@ collectionT.find(spot); // a whole model definition is also a valid filter * test simple field queries e.g. `{ name: 'Spot' }` */ /// it should query __string__ fields -expectType(await collectionT.find({ name: 'Spot' }).toArray()); +expectType[]>(await collectionT.find({ name: 'Spot' }).toArray()); // it should query string fields by regex -expectType(await collectionT.find({ name: /Blu/i }).toArray()); +expectType[]>(await collectionT.find({ name: /Blu/i }).toArray()); // it should query string fields by RegExp object, and bson regex -expectType(await collectionT.find({ name: new RegExp('MrMeow', 'i') }).toArray()); -expectType(await collectionT.find({ name: new BSONRegExp('MrMeow', 'i') }).toArray()); +expectType[]>( + await collectionT.find({ name: new RegExp('MrMeow', 'i') }).toArray() +); +expectType[]>( + await collectionT.find({ name: new BSONRegExp('MrMeow', 'i') }).toArray() +); /// it should not accept wrong types for string fields expectNotType>({ name: 23 }); expectNotType>({ name: { suffix: 'Jr' } }); @@ -90,9 +94,9 @@ expectNotType>({ bestFriend: [{ family: 'Andersons' }] }); /// it should query __array__ fields by exact match await collectionT.find({ treats: ['kibble', 'bone'] }).toArray(); /// it should query __array__ fields by element type -expectType(await collectionT.find({ treats: 'kibble' }).toArray()); -expectType(await collectionT.find({ treats: /kibble/i }).toArray()); -expectType(await collectionT.find({ friends: spot }).toArray()); +expectType[]>(await collectionT.find({ treats: 'kibble' }).toArray()); +expectType[]>(await collectionT.find({ treats: /kibble/i }).toArray()); +expectType[]>(await collectionT.find({ friends: spot }).toArray()); /// it should not query array fields by wrong types expectNotType>({ treats: 12 }); expectNotType>({ friends: { name: 'not a full model' } }); @@ -206,7 +210,7 @@ await collectionT.find({ $where: 'function() { return true }' }).toArray(); await collectionT .find({ $where: function () { - expectType(this); + expectType>(this); return this.name === 'MrMeow'; } }) diff --git a/test/types/community/collection/findX.test-d.ts b/test/types/community/collection/findX.test-d.ts index 48e5f61634..0b851f8821 100644 --- a/test/types/community/collection/findX.test-d.ts +++ b/test/types/community/collection/findX.test-d.ts @@ -1,5 +1,13 @@ import { expectAssignable, expectNotType, expectType } from 'tsd'; -import { FindCursor, FindOptions, MongoClient, Document, Collection, Db } from '../../../../src'; +import { + FindCursor, + FindOptions, + MongoClient, + Document, + Collection, + Db, + WithId +} from '../../../../src'; import type { Projection, ProjectionOperators } from '../../../../src'; import type { PropExists } from '../../utility_types'; @@ -10,7 +18,7 @@ const collection = db.collection('test.find'); // Locate all the entries using find collection.find({}).toArray((_err, fields) => { - expectType(fields); + expectType[] | undefined>(fields); }); // test with collection type @@ -26,7 +34,7 @@ collectionT.find({ $and: [{ numberField: { $gt: 0 } }, { numberField: { $lt: 100 } }], readonlyFruitTags: { $all: ['apple', 'pear'] } }); -expectType>(collectionT.find({})); +expectType>>(collectionT.find({})); await collectionT.findOne( {}, @@ -72,22 +80,24 @@ interface Bag { const collectionBag = db.collection('bag'); -const cursor: FindCursor = collectionBag.find({ color: 'black' }); +const cursor: FindCursor> = collectionBag.find({ color: 'black' }); cursor.toArray((_err, bags) => { - expectType(bags); + expectType[] | undefined>(bags); }); cursor.forEach( bag => { - expectType(bag); + expectType>(bag); }, () => { return null; } ); -expectType(await collectionBag.findOne({ color: 'red' }, { projection: { cost: 1 } })); +expectType | null>( + await collectionBag.findOne({ color: 'red' }, { projection: { cost: 1 } }) +); const overrideFind = await collectionBag.findOne<{ cost: number }>( { color: 'white' }, @@ -150,40 +160,48 @@ const colorsFreeze: ReadonlyArray = Object.freeze(['blue', 'red']); const colorsWritable: Array = ['blue', 'red']; // Permitted Readonly fields -expectType>(colorCollection.find({ color: { $in: colorsFreeze } })); -expectType>(colorCollection.find({ color: { $in: colorsWritable } })); -expectType>(colorCollection.find({ color: { $nin: colorsFreeze } })); -expectType>( +expectType>>( + colorCollection.find({ color: { $in: colorsFreeze } }) +); +expectType>>( + colorCollection.find({ color: { $in: colorsWritable } }) +); +expectType>>( + colorCollection.find({ color: { $nin: colorsFreeze } }) +); +expectType>>( colorCollection.find({ color: { $nin: colorsWritable } }) ); // $all and $elemMatch works against single fields (it's just redundant) -expectType>(colorCollection.find({ color: { $all: colorsFreeze } })); -expectType>( +expectType>>( + colorCollection.find({ color: { $all: colorsFreeze } }) +); +expectType>>( colorCollection.find({ color: { $all: colorsWritable } }) ); -expectType>( +expectType>>( colorCollection.find({ color: { $elemMatch: colorsFreeze } }) ); -expectType>( +expectType>>( colorCollection.find({ color: { $elemMatch: colorsWritable } }) ); const countCollection = client.db('test_db').collection<{ count: number }>('test_collection'); -expectType>( +expectType>>( countCollection.find({ count: { $bitsAnySet: Object.freeze([1, 0, 1]) } }) ); -expectType>( +expectType>>( countCollection.find({ count: { $bitsAnySet: [1, 0, 1] as number[] } }) ); const listsCollection = client.db('test_db').collection<{ lists: string[] }>('test_collection'); await listsCollection.updateOne({}, { list: { $pullAll: Object.freeze(['one', 'two']) } }); -expectType>(listsCollection.find({ lists: { $size: 1 } })); +expectType>>(listsCollection.find({ lists: { $size: 1 } })); const rdOnlyListsCollection = client .db('test_db') .collection<{ lists: ReadonlyArray }>('test_collection'); -expectType }>>( +expectType }>>>( rdOnlyListsCollection.find({ lists: { $size: 1 } }) ); @@ -196,7 +214,9 @@ expectNotType } }>>( expectNotType>( colorCollection.find({ color: { $in: 3 as any } }) // `as any` is to let us make this mistake and still show the result type isn't broken ); -expectType>(colorCollection.find({ color: { $in: 3 as any } })); +expectType>>( + colorCollection.find({ color: { $in: 3 as any } }) +); // When you use the override, $in doesn't permit readonly colorCollection.find<{ color: string }>({ color: { $in: colorsFreeze } }); @@ -228,12 +248,12 @@ interface TypedDb extends Db { const typedDb = client.db('test2') as TypedDb; const person = typedDb.collection('people').findOne({}); -expectType>(person); +expectType | null>>(person); typedDb.collection('people').findOne({}, function (_err, person) { - expectType(person); // null is if nothing is found, undefined is when there is an error defined + expectType | null | undefined>(person); // null is if nothing is found, undefined is when there is an error defined }); typedDb.collection('things').findOne({}, function (_err, thing) { - expectType(thing); + expectType | null | undefined>(thing); }); diff --git a/test/types/mongodb.test-d.ts b/test/types/mongodb.test-d.ts index c03e942de1..4088c13148 100644 --- a/test/types/mongodb.test-d.ts +++ b/test/types/mongodb.test-d.ts @@ -5,7 +5,7 @@ import { AggregationCursor } from '../../src/cursor/aggregation_cursor'; import type { FindCursor } from '../../src/cursor/find_cursor'; import type { ChangeStreamDocument } from '../../src/change_stream'; import type { Document } from 'bson'; -import { Db } from '../../src'; +import { Db, WithId } from '../../src'; import { Topology } from '../../src/sdam/topology'; import * as MongoDBDriver from '../../src'; @@ -30,7 +30,7 @@ const client = new MongoClient(''); const db = client.db('test'); const coll = db.collection('test'); const findCursor = coll.find(); -expectType(await findCursor.next()); +expectType | null>(await findCursor.next()); const mappedFind = findCursor.map(obj => Object.keys(obj).length); expectType>(mappedFind); expectType(await mappedFind.next()); diff --git a/test/types/union_schema.test-d.ts b/test/types/union_schema.test-d.ts index 0ce775ddfc..4db6bbaaa9 100644 --- a/test/types/union_schema.test-d.ts +++ b/test/types/union_schema.test-d.ts @@ -2,7 +2,7 @@ import { expectType, expectError, expectNotType, expectNotAssignable, expectAssi import type { Collection } from '../../src/collection'; import { ObjectId } from '../../src/bson'; -import type { Filter } from '../../src/mongo_types'; +import type { Filter, WithId } from '../../src/mongo_types'; type InsertOneFirstParam = Parameters['insertOne']>[0]; @@ -31,7 +31,7 @@ expectAssignable({ height: 4, width: 4 }); expectAssignable({ radius: 4 }); const c: Collection = null as never; -expectType>(c.findOne({ height: 4, width: 4 })); +expectType | null>>(c.findOne({ height: 4, width: 4 })); // collection API can only respect TSchema given, cannot pick a type inside a union expectNotType>(c.findOne({ height: 4, width: 4 })); From 81b286323579a7ab912f8df40c32b8558d39eae0 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Tue, 16 Nov 2021 16:09:05 -0500 Subject: [PATCH 2/2] test: add a few example schemas with differing _id definitions --- .../community/collection/findX.test-d.ts | 35 ++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/test/types/community/collection/findX.test-d.ts b/test/types/community/collection/findX.test-d.ts index 0b851f8821..d84940ed87 100644 --- a/test/types/community/collection/findX.test-d.ts +++ b/test/types/community/collection/findX.test-d.ts @@ -6,7 +6,8 @@ import { Document, Collection, Db, - WithId + WithId, + ObjectId } from '../../../../src'; import type { Projection, ProjectionOperators } from '../../../../src'; import type { PropExists } from '../../utility_types'; @@ -19,6 +20,10 @@ const collection = db.collection('test.find'); // Locate all the entries using find collection.find({}).toArray((_err, fields) => { expectType[] | undefined>(fields); + if (fields) { + expectType(fields[0]._id); + expectNotType(fields[0]._id); + } }); // test with collection type @@ -257,3 +262,31 @@ typedDb.collection('people').findOne({}, function (_err, person) { typedDb.collection('things').findOne({}, function (_err, thing) { expectType | null | undefined>(thing); }); + +interface SchemaWithTypicalId { + _id: ObjectId; + name: string; +} +const schemaWithTypicalIdCol = db.collection('a'); +expectType | null>(await schemaWithTypicalIdCol.findOne()); +expectAssignable(await schemaWithTypicalIdCol.findOne()); + +interface SchemaWithOptionalTypicalId { + _id?: ObjectId; + name: string; +} +const schemaWithOptionalTypicalId = db.collection('a'); +expectType | null>(await schemaWithOptionalTypicalId.findOne()); +expectAssignable(await schemaWithOptionalTypicalId.findOne()); + +interface SchemaWithUserDefinedId { + _id: number; + name: string; +} +const schemaWithUserDefinedId = db.collection('a'); +expectType | null>(await schemaWithUserDefinedId.findOne()); +const result = await schemaWithUserDefinedId.findOne(); +if (result !== null) { + expectType(result._id); +} +expectAssignable(await schemaWithUserDefinedId.findOne());