diff --git a/package.json b/package.json index f126edfc32..fe57b8d028 100644 --- a/package.json +++ b/package.json @@ -107,8 +107,9 @@ "build:docs": "typedoc", "check:bench": "node test/benchmarks/driverBench", "check:coverage": "nyc npm run check:test", - "check:lint": "npm run build:dts && npm run check:dts && npm run check:eslint", + "check:lint": "npm run build:dts && npm run check:dts && npm run check:eslint && npm run check:tsd", "check:eslint": "eslint -v && eslint --max-warnings=0 --ext '.js,.ts' src test", + "check:tsd": "tsd --version && tsd", "check:dts": "tsc --noEmit mongodb.d.ts && tsd", "check:test": "mocha --recursive test/functional test/unit", "check:ts": "tsc -v && tsc --noEmit", @@ -124,6 +125,12 @@ "test": "npm run check:lint && npm run check:test" }, "tsd": { - "directory": "test/types" + "directory": "test/types", + "compilerOptions": { + "strict": true, + "target": "esnext", + "module": "commonjs", + "moduleResolution": "node" + } } } diff --git a/src/collection.ts b/src/collection.ts index 94bce4bc14..b70436149b 100644 --- a/src/collection.ts +++ b/src/collection.ts @@ -414,34 +414,39 @@ export class Collection { updateOne( filter: Filter, update: UpdateFilter | Partial - ): Promise; + ): Promise; updateOne( filter: Filter, update: UpdateFilter | Partial, - callback: Callback + callback: Callback ): void; updateOne( filter: Filter, update: UpdateFilter | Partial, options: UpdateOptions - ): Promise; + ): Promise; updateOne( filter: Filter, update: UpdateFilter | Partial, options: UpdateOptions, - callback: Callback + callback: Callback ): void; updateOne( filter: Filter, update: UpdateFilter | Partial, - options?: UpdateOptions | Callback, - callback?: Callback - ): Promise | void { + options?: UpdateOptions | Callback, + callback?: Callback + ): Promise | void { if (typeof options === 'function') (callback = options), (options = {}); return executeOperation( getTopology(this), - new UpdateOneOperation(this as TODO_NODE_3286, filter, update, resolveOptions(this, options)), + new UpdateOneOperation( + this as TODO_NODE_3286, + filter, + update, + resolveOptions(this, options) + ) as TODO_NODE_3286, callback ); } @@ -685,10 +690,10 @@ export class Collection { // 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): Promise; + findOne(filter: Filter, options?: FindOptions): Promise; findOne( - filter: Filter, + filter: Filter, options?: FindOptions, callback?: Callback ): void; @@ -725,7 +730,7 @@ export class Collection { */ find(): FindCursor; find(filter: Filter, options?: FindOptions): 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 d0ab65e117..02fd1fd4b1 100644 --- a/src/cursor/abstract_cursor.ts +++ b/src/cursor/abstract_cursor.ts @@ -280,8 +280,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) { @@ -296,9 +295,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()); @@ -311,9 +310,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()); @@ -329,10 +328,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') { @@ -341,7 +340,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 @@ -358,7 +357,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); @@ -402,15 +401,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); @@ -420,7 +419,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); @@ -458,11 +457,12 @@ export abstract class AbstractCursor< * Map all documents using the provided function * If there is a transform set on the cursor, that will be called first and the result passed to * this function's transform. - * @remarks * - * **NOTE:** adding a transform changes the return type of the iteration of this cursor, it **does not** return - * a new instance of a cursor. This means when calling map, you should always assign the result to a new - * variable. Take note of the following example: + * @remarks + * **Note for Typescript Users:** adding a transform changes the return type of the iteration of this cursor, + * it **does not** return a new instance of a cursor. This means when calling map, + * you should always assign the result to a new variable in order to get a correctly typed cursor variable. + * Take note of the following example: * * @example * ```typescript diff --git a/src/cursor/aggregation_cursor.ts b/src/cursor/aggregation_cursor.ts index 4afff532b9..a52f58a21e 100644 --- a/src/cursor/aggregation_cursor.ts +++ b/src/cursor/aggregation_cursor.ts @@ -149,9 +149,10 @@ export class AggregationCursor extends AbstractCursor extends AbstractCursor { * * @remarks * - * Adding a projection changes the return type of the iteration of this cursor, + * **Note for Typescript Users:** adding a transform changes the return type of the iteration of this cursor, * it **does not** return a new instance of a cursor. This means when calling project, - * you should always assign the result to a new variable. Take note of the following example: + * you should always assign the result to a new variable in order to get a correctly typed cursor variable. + * Take note of the following example: * * @example * ```typescript diff --git a/src/db.ts b/src/db.ts index 163d5163f0..2629cffe30 100644 --- a/src/db.ts +++ b/src/db.ts @@ -289,7 +289,10 @@ 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'); } diff --git a/src/error.ts b/src/error.ts index 0fd7cd539e..fb64d5c87a 100644 --- a/src/error.ts +++ b/src/error.ts @@ -78,6 +78,11 @@ export interface ErrorDescription extends Document { export class MongoError extends Error { /** @internal */ [kErrorLabels]: Set; + /** + * This is a number in MongoServerError and a string in MongoDriverError + * @privateRemarks + * Define the type override on the subclasses when we can use the override keyword + */ code?: number | string; topologyVersion?: TopologyVersion; @@ -132,12 +137,10 @@ export class MongoError extends Error { * @category Error */ export class MongoServerError extends MongoError { - code?: number; codeName?: string; writeConcernError?: Document; errInfo?: Document; ok?: number; - topologyVersion?: TopologyVersion; [key: string]: any; constructor(message: ErrorDescription) { @@ -164,7 +167,6 @@ export class MongoServerError extends MongoError { * @category Error */ export class MongoDriverError extends MongoError { - code?: string; constructor(message: string) { super(message); } diff --git a/src/operations/list_collections.ts b/src/operations/list_collections.ts index c9d54e2669..4ba93ea82c 100644 --- a/src/operations/list_collections.ts +++ b/src/operations/list_collections.ts @@ -115,7 +115,9 @@ export interface CollectionInfo extends Document { /** @public */ export class ListCollectionsCursor< - T extends Pick | CollectionInfo = CollectionInfo + T extends Pick | CollectionInfo = + | Pick + | CollectionInfo > extends AbstractCursor { parent: Db; filter: Document; diff --git a/test/types/.eslintrc.json b/test/types/.eslintrc.json index cdbf49a784..4831a1a4cf 100644 --- a/test/types/.eslintrc.json +++ b/test/types/.eslintrc.json @@ -24,6 +24,8 @@ "prettier/prettier": "error", "tsdoc/syntax": "warn", "@typescript-eslint/no-explicit-any": "off", - "@typescript-eslint/no-unused-vars": "error" + "@typescript-eslint/no-unused-vars": "error", + "@typescript-eslint/ban-ts-comment": "off", + "@typescript-eslint/no-empty-function":"off" } } diff --git a/test/types/community/collection/filterQuery.test-d.ts b/test/types/community/collection/filterQuery.test-d.ts index 2a70777784..7902002a90 100644 --- a/test/types/community/collection/filterQuery.test-d.ts +++ b/test/types/community/collection/filterQuery.test-d.ts @@ -50,6 +50,7 @@ const collectionT = db.collection('test.filterQuery'); // Assert that collection.find uses the Filter helper like so: const filter: Filter = {}; collectionT.find(filter); +collectionT.find(spot); // a whole model definition is also a valid filter // Now tests below can directly test the Filter helper, and are implicitly checking collection.find /** @@ -226,3 +227,7 @@ await collectionT.find({ playmates: { $elemMatch: { name: 'MrMeow' } } }).toArra expectNotType>({ name: { $all: ['world', 'world'] } }); expectNotType>({ age: { $elemMatch: [1, 2] } }); 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 } }); diff --git a/test/types/community/collection/findX.test-d.ts b/test/types/community/collection/findX.test-d.ts index 9fb33a75f4..173112574a 100644 --- a/test/types/community/collection/findX.test-d.ts +++ b/test/types/community/collection/findX.test-d.ts @@ -102,6 +102,16 @@ expectType<{ cost: number } | undefined>( await collectionBag.findOne<{ cost: number }>({ color: 'red' }, { projection: { cost: 1 } }) ); +// NODE-3468 The generic in find and findOne no longer affect the filter type +type Pet = { type: string; age: number }; +const pets = db.collection('pets'); + +expectType<{ crazy: number }[]>( + await pets + .find<{ crazy: number }>({ type: 'dog', age: 1 }) + .toArray() +); + interface Car { make: string; } @@ -186,12 +196,16 @@ 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'] } }); + // 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 503d70d408..afde3b97a7 100644 --- a/test/types/community/cursor.test-d.ts +++ b/test/types/community/cursor.test-d.ts @@ -68,14 +68,17 @@ typedCollection 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()); -expectNotType<{ age: number }[]>(await typedCollection.find().project({ name: 1 }).toArray()); +expectType(await typedCollection.find().project({ name: 1 }).toArray()); +expectNotType<{ age: string }[]>(await typedCollection.find().project({ name: 1 }).toArray()); // An unknown key -expectType<{ notExistingField: unknown }[]>( - await typedCollection.find().project({ notExistingField: 1 }).toArray() +expectType(await typedCollection.find().project({ notExistingField: 1 }).toArray()); +expectType<{ a: bigint }[]>( + await typedCollection + .find() + .project<{ a: bigint }>({ notExistingField: 1 }) + .toArray() ); -expectType(await typedCollection.find().project({ notExistingField: 1 }).toArray()); // Projection operator expectType<{ listOfNumbers: number[] }[]>( @@ -137,13 +140,19 @@ expectType( .toArray() ); -// Returns generic document when no override given +// Does not return whatever the publicMemeProjection states, returns generic Document expectNotType( await memeCollection .find({ _id: { $in: [] } }) .project(publicMemeProjection) .toArray() ); +expectType( + await memeCollection + .find({ _id: { $in: [] } }) + .project(publicMemeProjection) + .toArray() +); // Returns generic document when there is no schema expectType( @@ -153,3 +162,12 @@ expectType( .project(publicMemeProjection) .toArray() ); + +// Returns projection override when one is specified on a collection with no schema +expectType( + await new Db(new MongoClient(''), '') + .collection('memes') + .find({ _id: { $in: [] } }) + .project(publicMemeProjection) + .toArray() +); diff --git a/test/types/list_collections.test-d.ts b/test/types/list_collections.test-d.ts index 8e6883107b..be67027206 100644 --- a/test/types/list_collections.test-d.ts +++ b/test/types/list_collections.test-d.ts @@ -39,7 +39,3 @@ const overridden = await db .listCollections>({}, { nameOnly: false }) .toArray(); expectType[]>(overridden); -const overriddenWithToArray = await db - .listCollections({}, { nameOnly: false }) - .toArray>(); -expectType[]>(overriddenWithToArray); diff --git a/test/types/tsconfig.json b/test/types/tsconfig.json index 0a2bf85447..51d0b9db28 100644 --- a/test/types/tsconfig.json +++ b/test/types/tsconfig.json @@ -2,6 +2,7 @@ "compilerOptions": { "target": "esnext", "module": "esnext", - "moduleResolution": "node" + "moduleResolution": "node", + "importsNotUsedAsValues": "error" } } diff --git a/test/types/type_errors.test-d.ts b/test/types/type_errors.test-d.ts new file mode 100644 index 0000000000..683c50ae9f --- /dev/null +++ b/test/types/type_errors.test-d.ts @@ -0,0 +1,29 @@ +import { MongoClient } from '../../src/index'; + +/** + * This test file should contain examples of known compilation errors + * using the `@ts-expect-error` directive above a failing line will make it succeed to compile + * and it will catch the case where something stops being a compilation error. + * + * You should attempt to accompany any "failing" example with a succeeding one since `@ts-expect-error` + * will ignore any and all errors generated by the line beneath it. + */ + +const db = new MongoClient('').db(); +const pets = db.collection<{ type: string; age: number }>('pets'); + +const dogs = pets.find({ type: 'dog' }); + +// NODE-3468 generic parameters were removed from these cursor methods +// using the generic accepting methods: `.project` `.map` `.find` +// is the preferred way to strongly type your cursor result + +// @ts-expect-error +await dogs.toArray<{ age: number }>(); +await dogs.toArray(); +// @ts-expect-error +await dogs.forEach<{ age: number }>(() => {}); +await dogs.forEach(() => {}); +// @ts-expect-error +await dogs.next<{ age: number }>(); +await dogs.next(); diff --git a/test/unit/type_check.test.js b/test/unit/type_check.test.js deleted file mode 100644 index af27dd2b4a..0000000000 --- a/test/unit/type_check.test.js +++ /dev/null @@ -1,23 +0,0 @@ -'use strict'; - -const tsd = require('tsd').default; -const { expect } = require('chai'); - -const REPO_ROOT = __dirname.replace('test/unit', ''); - -describe('Typescript definitions', () => { - it('should pass assertions defined in test/types', async () => { - const diagnostics = await tsd(); - if (diagnostics.length !== 0) { - const messages = diagnostics - .map( - d => - `${d.fileName.replace(REPO_ROOT, '')}:${d.line}:${d.column} - [${d.severity}]: ${ - d.message - }` - ) - .join('\n'); - expect.fail(`\n\n${messages}\n\n${diagnostics.length} errors found.`); - } - }); -});