diff --git a/src/collection.ts b/src/collection.ts index 58f74dde209..1eb7e0fe7af 100644 --- a/src/collection.ts +++ b/src/collection.ts @@ -682,18 +682,6 @@ export class Collection { options: FindOptions, callback: Callback ): void; - - // allow an override of the schema. - findOne(): Promise; - findOne(callback: Callback): void; - findOne(filter: Filter): Promise; - findOne(filter: Filter, options?: FindOptions): Promise; - findOne( - filter: Filter, - options?: FindOptions, - callback?: Callback - ): void; - findOne( filter?: Filter | Callback, options?: FindOptions | Callback, @@ -729,7 +717,6 @@ export class Collection { */ 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( diff --git a/src/cursor/abstract_cursor.ts b/src/cursor/abstract_cursor.ts index ab579e5194b..719d3a33ca5 100644 --- a/src/cursor/abstract_cursor.ts +++ b/src/cursor/abstract_cursor.ts @@ -279,8 +279,7 @@ export abstract class AbstractCursor< return done(undefined, true); } - next(this, true, (err, doc) => { - // FIXME(NODE): + next(this, true, (err, doc) => { if (err) return done(err); if (doc) { @@ -295,9 +294,9 @@ export abstract class AbstractCursor< } /** Get the next available document from the cursor, returns null if no more documents are available. */ - next(): Promise; - next(callback: Callback): void; - next(callback?: Callback): Promise | void { + next(): Promise; + next(callback: Callback): void; + next(callback?: Callback): Promise | void { return maybePromise(callback, done => { if (this[kId] === Long.ZERO) { return done(new MongoCursorExhaustedError()); @@ -310,9 +309,9 @@ export abstract class AbstractCursor< /** * Try to get the next available document from the cursor or `null` if an empty batch is returned */ - tryNext(): Promise; - tryNext(callback: Callback): void; - tryNext(callback?: Callback): Promise | void { + tryNext(): Promise; + tryNext(callback: Callback): void; + tryNext(callback?: Callback): Promise | void { return maybePromise(callback, done => { if (this[kId] === Long.ZERO) { return done(new MongoCursorExhaustedError()); @@ -328,10 +327,10 @@ export abstract class AbstractCursor< * @param iterator - The iteration callback. * @param callback - The end callback. */ - forEach(iterator: (doc: T) => boolean | void): Promise; - forEach(iterator: (doc: T) => boolean | void, callback: Callback): void; - forEach( - iterator: (doc: T) => boolean | void, + forEach(iterator: (doc: TSchema) => boolean | void): Promise; + forEach(iterator: (doc: TSchema) => boolean | void, callback: Callback): void; + forEach( + iterator: (doc: TSchema) => boolean | void, callback?: Callback ): Promise | void { if (typeof iterator !== 'function') { @@ -340,7 +339,7 @@ export abstract class AbstractCursor< return maybePromise(callback, done => { const transform = this[kTransform]; const fetchDocs = () => { - next(this, true, (err, doc) => { + next(this, true, (err, doc) => { if (err || doc == null) return done(err); let result; // NOTE: no need to transform because `next` will do this automatically @@ -357,7 +356,7 @@ export abstract class AbstractCursor< for (let i = 0; i < internalDocs.length; ++i) { try { result = iterator( - (transform ? transform(internalDocs[i]) : internalDocs[i]) as T // TODO(NODE-3283): Improve transform typing + (transform ? transform(internalDocs[i]) : internalDocs[i]) as TSchema // TODO(NODE-3283): Improve transform typing ); } catch (error) { return done(error); @@ -395,15 +394,15 @@ export abstract class AbstractCursor< * * @param callback - The result callback. */ - toArray(): Promise; - toArray(callback: Callback): void; - toArray(callback?: Callback): Promise | void { + toArray(): Promise; + toArray(callback: Callback): void; + toArray(callback?: Callback): Promise | void { return maybePromise(callback, done => { - const docs: T[] = []; + const docs: TSchema[] = []; const transform = this[kTransform]; const fetchDocs = () => { // NOTE: if we add a `nextBatch` then we should use it here - next(this, true, (err, doc) => { + next(this, true, (err, doc) => { if (err) return done(err); if (doc == null) return done(undefined, docs); @@ -413,7 +412,7 @@ export abstract class AbstractCursor< // these do need to be transformed since they are copying the rest of the batch const internalDocs = (transform ? this[kDocuments].splice(0, this[kDocuments].length).map(transform) - : this[kDocuments].splice(0, this[kDocuments].length)) as T[]; // TODO(NODE-3283): Improve transform typing + : this[kDocuments].splice(0, this[kDocuments].length)) as TSchema[]; // TODO(NODE-3283): Improve transform typing if (internalDocs) { docs.push(...internalDocs); diff --git a/src/db.ts b/src/db.ts index 595ed7a8f46..cad9031d18f 100644 --- a/src/db.ts +++ b/src/db.ts @@ -289,7 +289,7 @@ export class Db { * @param pipeline - An array of aggregation stages to be executed * @param options - Optional settings for the command */ - aggregate(pipeline: Document[] = [], options?: AggregateOptions): AggregationCursor { + aggregate(pipeline: Document[] = [], options?: AggregateOptions): AggregationCursor { if (arguments.length > 2) { throw new MongoInvalidArgumentError('Method "db.aggregate()" accepts at most two arguments'); } @@ -370,17 +370,9 @@ export class Db { filter: Document, options: Exclude & { nameOnly: false } ): ListCollectionsCursor; - listCollections< - T extends Pick | CollectionInfo = - | Pick - | CollectionInfo - >(filter?: Document, options?: ListCollectionsOptions): ListCollectionsCursor; - listCollections< - T extends Pick | CollectionInfo = - | Pick - | CollectionInfo - >(filter: Document = {}, options: ListCollectionsOptions = {}): ListCollectionsCursor { - return new ListCollectionsCursor(this, filter, resolveOptions(this, options)); + listCollections(filter?: Document, options?: ListCollectionsOptions): ListCollectionsCursor; + listCollections(filter: Document = {}, options: ListCollectionsOptions = {}): ListCollectionsCursor { + return new ListCollectionsCursor(this, filter, resolveOptions(this, options)); } /** diff --git a/test/types/community/collection/findX.test-d.ts b/test/types/community/collection/findX.test-d.ts index 16c596807d2..ab47ec49628 100644 --- a/test/types/community/collection/findX.test-d.ts +++ b/test/types/community/collection/findX.test-d.ts @@ -91,16 +91,18 @@ expectType( await collectionBag.findOne({ color: 'red' }, { projection: { cost: 1 } }) ); -const overrideFind = await collectionBag.findOne<{ cost: number }>( - { color: 'white' }, - { projection: { cost: 1 } } -); -expectType>(false); +// NODE-3468 Overriding the return type with a generic is not permitted +// const overrideFind = await collectionBag.findOne<{ cost: number }>( +// { color: 'white' }, +// { projection: { cost: 1 } } +// ); +// expectType>(false); // Overriding findOne, makes the return that exact type -expectType<{ cost: number } | undefined>( - await collectionBag.findOne<{ cost: number }>({ color: 'red' }, { projection: { cost: 1 } }) -); +// NODE-3468 Overriding the return type with a generic is not permitted +// expectType<{ cost: number } | undefined>( +// await collectionBag.findOne<{ cost: number }>({ color: 'red' }, { projection: { cost: 1 } }) +// ); interface Car { make: string; @@ -182,16 +184,22 @@ expectType }>>( ); // Before NODE-3452's fix we would get this strange result that included the filter shape joined with the actual schema +// NODE-3468 Overriding the return type with a generic is not permitted expectNotType } }>>( colorCollection.find({ color: { $in: colorsFreeze } }) ); -// This is related to another bug that will be fixed in NODE-3454 -expectType>(colorCollection.find({ color: { $in: 3 } })); +// NODE-3454: Using the incorrect $in value doesn't mess with the resulting schema +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 } })); // When you use the override, $in doesn't permit readonly -colorCollection.find<{ color: string }>({ color: { $in: colorsFreeze } }); -colorCollection.find<{ color: string }>({ color: { $in: ['regularArray'] } }); +// NODE-3468 Overriding the return type with a generic is not permitted +// colorCollection.find<{ color: string }>({ color: { $in: colorsFreeze } }); +// colorCollection.find<{ color: string }>({ color: { $in: ['regularArray'] } }); + // This is a regression test that we don't remove the unused generic in FindOptions const findOptions: FindOptions<{ a: number }> = {}; expectType(findOptions); diff --git a/test/types/community/cursor.test-d.ts b/test/types/community/cursor.test-d.ts index 503d70d408f..25f60712b1c 100644 --- a/test/types/community/cursor.test-d.ts +++ b/test/types/community/cursor.test-d.ts @@ -62,22 +62,35 @@ expectType<{ name: string }[]>( ); // override the collection type -typedCollection - .find<{ name2: string; age2: number }>({ name: '123' }, { projection: { age2: 1 } }) - .map(x => x.name2 && x.age2); +// NODE-3468: Overriding the return type is no longer allowed +// typedCollection +// .find<{ name2: string; age2: number }>({ name: '123' }, { projection: { age2: 1 } }) +// .map(x => x.name2 && x.age2); + +// Chaining map calls changes the final cursor +expectType>( + typedCollection + .find({ name: '123' }) + .map(x => ({ name2: x.name, age2: x.age })) + .map(({ name2, age2 }) => ({ a: `${name2}_${age2}` })) +); + typedCollection.find({ name: '123' }, { projection: { age: 1 } }).map(x => x.tag); // A known key with a constant projection -expectType<{ name: string }[]>(await typedCollection.find().project({ name: 1 }).toArray()); +// NODE-3468: projection returns a generic type and the override removal means no automatic type coercion +// expectType<{ name: string }[]>(await typedCollection.find().project({ name: 1 }).toArray()); +// +expectType(await typedCollection.find().project({ name: 1 }).toArray()); expectNotType<{ age: number }[]>(await typedCollection.find().project({ name: 1 }).toArray()); -// An unknown key -expectType<{ notExistingField: unknown }[]>( +// An unknown key -- NODE-3468: when using the project, your default return type is Document +expectNotType<{ notExistingField: unknown }[]>( await typedCollection.find().project({ notExistingField: 1 }).toArray() ); -expectType(await typedCollection.find().project({ notExistingField: 1 }).toArray()); +expectNotType(await typedCollection.find().project({ notExistingField: 1 }).toArray()); -// Projection operator +// Projection operator -- NODE-3468: it is recommended that users override the T in project() expectType<{ listOfNumbers: number[] }[]>( await typedCollection .find() @@ -87,10 +100,7 @@ expectType<{ listOfNumbers: number[] }[]>( // Using the override parameter works expectType<{ name: string }[]>( - await typedCollection - .find() - .project<{ name: string }>({ name: 1 }) - .toArray() + await typedCollection.find().project<{ name: string }>({ name: 1 }).toArray() ); void async function () { @@ -128,7 +138,7 @@ const publicMemeProjection = { receiver: { $toString: '$receiver' }, likes: '$totalLikes' // <== (NODE-3454) cause of TS2345 error: Argument of type T is not assignable to parameter of type U }; -const memeCollection = new Db(new MongoClient(''), '').collection('memes'); +const memeCollection = db.collection('memes'); expectType( await memeCollection @@ -145,9 +155,17 @@ expectNotType( .toArray() ); +expectType>( + memeCollection.find({ _id: { $in: [] } }).project(publicMemeProjection) +); + +expectNotType>(memeCollection.find({ _id: { $in: [] } })); +expectType>(memeCollection.find({ _id: { $in: [] } })); +``; + // Returns generic document when there is no schema expectType( - await new Db(new MongoClient(''), '') + await db .collection('memes') .find({ _id: { $in: [] } }) .project(publicMemeProjection) diff --git a/test/types/list_collections.test-d.ts b/test/types/list_collections.test-d.ts index 8e6883107bc..d65db22faee 100644 --- a/test/types/list_collections.test-d.ts +++ b/test/types/list_collections.test-d.ts @@ -8,12 +8,10 @@ const db = new MongoClient('').db(); type EitherCollectionInfoResult = CollectionInfo | Pick; // We default to the CollectionInfo result type -expectType | CollectionInfo>>( - db.listCollections() -); +expectType>(db.listCollections()); // By default it isn't narrowed to either type expectNotType>>(db.listCollections()); -expectNotType>(db.listCollections()); +expectType>(db.listCollections()); // Testing each argument variation db.listCollections(); @@ -21,7 +19,7 @@ db.listCollections({ a: 2 }); db.listCollections({ a: 2 }, { batchSize: 2 }); const collections = await db.listCollections().toArray(); -expectType(collections); +expectNotType(collections); const nameOnly = await db.listCollections({}, { nameOnly: true }).toArray(); expectType[]>(nameOnly); @@ -29,17 +27,11 @@ expectType[]>(nameOnly); const fullInfo = await db.listCollections({}, { nameOnly: false }).toArray(); expectType(fullInfo); -const couldBeEither = await db.listCollections({}, { nameOnly: Math.random() > 0.5 }).toArray(); -expectType(couldBeEither); +const cannotBeEither = await db.listCollections({}, { nameOnly: Math.random() > 0.5 }).toArray(); +expectNotType(cannotBeEither); +expectType(cannotBeEither); // Showing here that: // regardless of the option the generic parameter can be used to coerce the result if need be // note the nameOnly: false, yet strings are returned -const overridden = await db - .listCollections>({}, { nameOnly: false }) - .toArray(); -expectType[]>(overridden); -const overriddenWithToArray = await db - .listCollections({}, { nameOnly: false }) - .toArray>(); -expectType[]>(overriddenWithToArray); +// NODE-3468: No longer permits overriding of return type via a generic