diff --git a/src/collection.ts b/src/collection.ts index f69d368d0b..9f9863c47a 100644 --- a/src/collection.ts +++ b/src/collection.ts @@ -12,7 +12,7 @@ import type { PkFactory } from './mongo_client'; import type { Filter, Flatten, - OptionalId, + OptionalUnlessRequiredId, TODO_NODE_3286, UpdateFilter, WithId, @@ -264,16 +264,22 @@ export class Collection { * @param options - Optional settings for the command * @param callback - An optional callback, a Promise will be returned if none is provided */ - insertOne(doc: OptionalId): Promise>; - insertOne(doc: OptionalId, callback: Callback>): void; - insertOne(doc: OptionalId, options: InsertOneOptions): Promise>; + insertOne(doc: OptionalUnlessRequiredId): Promise>; insertOne( - doc: OptionalId, + doc: OptionalUnlessRequiredId, + callback: Callback> + ): void; + insertOne( + doc: OptionalUnlessRequiredId, + options: InsertOneOptions + ): Promise>; + insertOne( + doc: OptionalUnlessRequiredId, options: InsertOneOptions, callback: Callback> ): void; insertOne( - doc: OptionalId, + doc: OptionalUnlessRequiredId, options?: InsertOneOptions | Callback>, callback?: Callback> ): Promise> | void { @@ -308,19 +314,22 @@ export class Collection { * @param options - Optional settings for the command * @param callback - An optional callback, a Promise will be returned if none is provided */ - insertMany(docs: OptionalId[]): Promise>; - insertMany(docs: OptionalId[], callback: Callback>): void; + insertMany(docs: OptionalUnlessRequiredId[]): Promise>; + insertMany( + docs: OptionalUnlessRequiredId[], + callback: Callback> + ): void; insertMany( - docs: OptionalId[], + docs: OptionalUnlessRequiredId[], options: BulkWriteOptions ): Promise>; insertMany( - docs: OptionalId[], + docs: OptionalUnlessRequiredId[], options: BulkWriteOptions, callback: Callback> ): void; insertMany( - docs: OptionalId[], + docs: OptionalUnlessRequiredId[], options?: BulkWriteOptions | Callback>, callback?: Callback> ): Promise> | void { @@ -1526,7 +1535,7 @@ export class Collection { * @param callback - An optional callback, a Promise will be returned if none is provided */ insert( - docs: OptionalId[], + docs: OptionalUnlessRequiredId[], options: BulkWriteOptions, callback: Callback> ): Promise> | void { diff --git a/src/index.ts b/src/index.ts index 609d2607df..e3d71891d0 100644 --- a/src/index.ts +++ b/src/index.ts @@ -286,11 +286,13 @@ export type { KeysOfAType, KeysOfOtherType, MatchKeysAndValues, + NonObjectIdLikeDocument, NotAcceptedFields, NumericType, OneOrMore, OnlyFieldsOfType, OptionalId, + OptionalUnlessRequiredId, Projection, ProjectionOperators, PullAllOperator, diff --git a/src/mongo_types.ts b/src/mongo_types.ts index fd06ae7831..5705908861 100644 --- a/src/mongo_types.ts +++ b/src/mongo_types.ts @@ -1,3 +1,4 @@ +import type { ObjectIdLike } from 'bson'; import { EventEmitter } from 'events'; import type { @@ -17,13 +18,15 @@ import type { Sort } from './sort'; export type TODO_NODE_3286 = any; /** Given an object shaped type, return the type of the _id field or default to ObjectId @public */ -export type InferIdType = TSchema extends { _id: infer IdType } // user has defined a type for _id - ? // eslint-disable-next-line @typescript-eslint/ban-types - {} extends IdType // TODO(NODE-3285): Improve type readability - ? // eslint-disable-next-line @typescript-eslint/ban-types - Exclude - : unknown extends IdType - ? ObjectId +export type InferIdType = TSchema extends { _id: infer IdType } + ? // user has defined a type for _id + Record extends IdType + ? never // explicitly forbid empty objects as the type of _id + : IdType + : TSchema extends { _id?: infer IdType } + ? // optional _id defined - return ObjectId | IdType + unknown extends IdType + ? ObjectId // infer the _id type as ObjectId if the type of _id is unknown : IdType : ObjectId; // user has not defined _id on schema @@ -33,17 +36,23 @@ export type WithId = EnhancedOmit & { _id: InferIdType< /** * Add an optional _id field to an object shaped type * @public + */ +export type OptionalId = EnhancedOmit & { _id?: InferIdType }; + +/** + * Adds an optional _id field to an object shaped type, unless the _id field is requried on that type. + * In the case _id is required, this method continues to require_id. + * + * @public * * @privateRemarks * `ObjectId extends TSchema['_id']` is a confusing ordering at first glance. Rather than ask * `TSchema['_id'] extends ObjectId` which translated to "Is the _id property ObjectId?" * we instead ask "Does ObjectId look like (have the same shape) as the _id?" */ -export type OptionalId = TSchema extends { _id?: any } - ? ObjectId extends TSchema['_id'] // a Schema with ObjectId _id type or "any" or "indexed type" provided - ? EnhancedOmit & { _id?: InferIdType } // a Schema provided but _id type is not ObjectId - : WithId - : EnhancedOmit & { _id?: InferIdType }; // TODO(NODE-3285): Improve type readability +export type OptionalUnlessRequiredId = TSchema extends { _id: any } + ? TSchema + : OptionalId; /** TypeScript Omit (Exclude to be specific) does not work for objects with an "any" indexed type, and breaks discriminated unions @public */ export type EnhancedOmit = string extends keyof TRecordOrUnion @@ -91,8 +100,16 @@ export interface RootFilterOperators extends Document { $comment?: string | Document; } +/** + * @public + * A type that extends Document but forbids anything that "looks like" an object id. + */ +export type NonObjectIdLikeDocument = { + [key in keyof ObjectIdLike]?: never; +} & Document; + /** @public */ -export interface FilterOperators extends Document { +export interface FilterOperators extends NonObjectIdLikeDocument { // Comparison $eq?: TValue; $gt?: TValue; diff --git a/test/types/basic_schema.test-d.ts b/test/types/basic_schema.test-d.ts index 6c2ab62dbe..19634023cc 100644 --- a/test/types/basic_schema.test-d.ts +++ b/test/types/basic_schema.test-d.ts @@ -1,4 +1,4 @@ -import { expectAssignable, expectNotType, expectType } from 'tsd'; +import { expectAssignable, expectNotAssignable, expectNotType, expectType } from 'tsd'; import { ObjectId } from '../../src/bson'; import { Collection } from '../../src/collection'; @@ -31,7 +31,7 @@ expectNotType>({ a: 2, b: 34 }); // With _id expectAssignable>({ _id: new ObjectId(), a: 3 }); // Without _id -expectAssignable>({ a: 3 }); +expectNotAssignable>({ a: 3 }); // Does not permit extra keys expectNotType>({ a: 2, b: 34 }); diff --git a/test/types/community/collection/filterQuery.test-d.ts b/test/types/community/collection/filterQuery.test-d.ts index 06d337e98e..b3c8ebe723 100644 --- a/test/types/community/collection/filterQuery.test-d.ts +++ b/test/types/community/collection/filterQuery.test-d.ts @@ -1,7 +1,7 @@ import { BSONRegExp, Decimal128, ObjectId } from 'bson'; -import { expectAssignable, expectNotType, expectType } from 'tsd'; +import { expectAssignable, expectError, expectNotType, expectType } from 'tsd'; -import { Filter, MongoClient, WithId } from '../../../../src'; +import { Collection, Filter, MongoClient, WithId } from '../../../../src'; /** * test the Filter type using collection.find() method @@ -236,3 +236,51 @@ expectNotType>({ type: { $size: 2 } }); // dot key case that shows it is assignable even when the referenced key is the wrong type expectAssignable>({ 'bestFriend.name': 23 }); // using dot notation permits any type for the key expectNotType>({ bestFriend: { name: 23 } }); + +// ObjectId are not allowed to be used as a query predicate (issue described here: NODE-3758) +// this only applies to schemas where the _id is not of type ObjectId. +declare const nonObjectIdCollection: Collection<{ _id: number; otherField: string }>; +expectError( + nonObjectIdCollection.find({ + _id: new ObjectId() + }) +); +expectError( + nonObjectIdCollection.find({ + otherField: new ObjectId() + }) +); +nonObjectIdCollection.find({ + fieldThatDoesNotExistOnSchema: new ObjectId() +}); + +// we only forbid objects that "look like" object ids, so other random objects are permitted +nonObjectIdCollection.find({ + _id: { + hello: 'world' + } +}); +nonObjectIdCollection.find({ + otherField: { + hello: 'world' + } +}); + +declare const nonSpecifiedCollection: Collection; +nonSpecifiedCollection.find({ + _id: new ObjectId() +}); +nonSpecifiedCollection.find({ + otherField: new ObjectId() +}); + +nonSpecifiedCollection.find({ + _id: { + hello: 'world' + } +}); +nonSpecifiedCollection.find({ + otherField: { + hello: 'world' + } +}); diff --git a/test/types/community/collection/insertX.test-d.ts b/test/types/community/collection/insertX.test-d.ts index 8e325ffe1f..8029d49fdd 100644 --- a/test/types/community/collection/insertX.test-d.ts +++ b/test/types/community/collection/insertX.test-d.ts @@ -123,12 +123,35 @@ await collectionWithObjectId.insertOne({ numberField: 23, fruitTags: ['hi'] }); -// should not demand _id if it is ObjectId -await collectionWithObjectId.insertOne({ - stringField: 'hola', - numberField: 23, - fruitTags: ['hi'] -}); +// if _id is defined on the schema, it must be passed to insert operations +expectError( + collectionWithObjectId.insertOne({ + stringField: 'hola', + numberField: 23, + fruitTags: ['hi'] + }) +); + +// defined _id's that are not of type ObjectId cannot be cast to ObjectId +const collectionWithRequiredNumericId = + db.collection<{ _id: number; otherField: string }>('testCollection'); + +expectError( + collectionWithRequiredNumericId.insertOne({ + _id: new ObjectId(), + otherField: 'hola' + }) +); + +const collectionWithOptionalNumericId = + db.collection<{ _id?: number; otherField: string }>('testCollection'); + +expectError( + collectionWithOptionalNumericId.insertOne({ + _id: new ObjectId(), + otherField: 'hola' + }) +); /** * test indexed types diff --git a/test/types/schema_helpers.test-d.ts b/test/types/schema_helpers.test-d.ts index 8281a90aa8..c3c2994d5f 100644 --- a/test/types/schema_helpers.test-d.ts +++ b/test/types/schema_helpers.test-d.ts @@ -1,10 +1,11 @@ import { Document, ObjectId } from 'bson'; -import { expectAssignable, expectNotType, expectType } from 'tsd'; +import { expectAssignable, expectError, expectNotType, expectType } from 'tsd'; import type { EnhancedOmit, InferIdType, OptionalId, + OptionalUnlessRequiredId, WithId, WithoutId } from '../../src/mongo_types'; @@ -13,6 +14,11 @@ import type { expectType>(new ObjectId()); expectType>(1 + 1); expectType>(new ObjectId()); +expectType>(1 + 1); +expectType>(new ObjectId()); +expectError }>>({}); + +// union types could have an id of either type expectAssignable>(new ObjectId()); expectAssignable>(1 + 1); @@ -38,6 +44,14 @@ class MyId {} expectNotType>({ a: 3 }); expectNotType>({ _id: new ObjectId(), a: 3 }); +declare function functionReturningOptionalId(): OptionalId<{ + _id?: ObjectId | undefined; + a: number; +}>; +// OptionalUnlessRequiredId +expectType>({ a: 3, _id: new ObjectId() }); +expectType>(functionReturningOptionalId()); + // WithoutId removes _id whether defined in the schema or not expectType>({ a: 2 }); expectNotType>({ _id: 3, a: 2 }); diff --git a/test/types/union_schema.test-d.ts b/test/types/union_schema.test-d.ts index cff5efe6fa..65012beeda 100644 --- a/test/types/union_schema.test-d.ts +++ b/test/types/union_schema.test-d.ts @@ -18,7 +18,7 @@ interface Rectangle { type Shape = Circle | Rectangle; type ShapeInsert = InsertOneFirstParam; -expectAssignable({ height: 2, width: 2, radius: 2 }); // This is permitted... +expectNotAssignable({ height: 2, width: 2, radius: 2 }); // This is not permitted... // error cases, should not insert a portion of a type expectNotAssignable({ height: 2 }); expectError({ @@ -27,8 +27,8 @@ expectError({ _id: new ObjectId() }); // valid cases -expectAssignable({ height: 4, width: 4 }); -expectAssignable({ radius: 4 }); +expectAssignable({ height: 4, width: 4, _id: new ObjectId() }); +expectAssignable({ radius: 4, _id: new ObjectId() }); const c: Collection = null as never; expectType | null>>(c.findOne({ height: 4, width: 4 }));