Skip to content

Commit

Permalink
fix(NODE-3454): projection types are too narrow (#2924)
Browse files Browse the repository at this point in the history
  • Loading branch information
nbbeeken committed Aug 4, 2021
1 parent 4ecaa37 commit 48d6da9
Show file tree
Hide file tree
Showing 7 changed files with 150 additions and 46 deletions.
16 changes: 8 additions & 8 deletions src/collection.ts
Expand Up @@ -676,27 +676,27 @@ export class Collection<TSchema extends Document = Document> {
findOne(callback: Callback<TSchema | undefined>): void;
findOne(filter: Filter<TSchema>): Promise<TSchema | undefined>;
findOne(filter: Filter<TSchema>, callback: Callback<TSchema | undefined>): void;
findOne(filter: Filter<TSchema>, options: FindOptions<TSchema>): Promise<TSchema | undefined>;
findOne(filter: Filter<TSchema>, options: FindOptions): Promise<TSchema | undefined>;
findOne(
filter: Filter<TSchema>,
options: FindOptions<TSchema>,
options: FindOptions,
callback: Callback<TSchema | undefined>
): void;

// allow an override of the schema.
findOne<T = TSchema>(): Promise<T | undefined>;
findOne<T = TSchema>(callback: Callback<T | undefined>): void;
findOne<T = TSchema>(filter: Filter<T>): Promise<T | undefined>;
findOne<T = TSchema>(filter: Filter<T>, options?: FindOptions<T>): Promise<T | undefined>;
findOne<T = TSchema>(filter: Filter<T>, options?: FindOptions): Promise<T | undefined>;
findOne<T = TSchema>(
filter: Filter<T>,
options?: FindOptions<T>,
options?: FindOptions,
callback?: Callback<T | undefined>
): void;

findOne(
filter?: Filter<TSchema> | Callback<TSchema | undefined>,
options?: FindOptions<TSchema> | Callback<TSchema | undefined>,
options?: FindOptions | Callback<TSchema | undefined>,
callback?: Callback<TSchema>
): Promise<TSchema | undefined> | void {
if (callback != null && typeof callback !== 'function') {
Expand Down Expand Up @@ -728,9 +728,9 @@ export class Collection<TSchema extends Document = Document> {
* @param filter - The filter predicate. If unspecified, then all documents in the collection will match the predicate
*/
find(): FindCursor<TSchema>;
find(filter: Filter<TSchema>, options?: FindOptions<TSchema>): FindCursor<TSchema>;
find<T = TSchema>(filter: Filter<T>, options?: FindOptions<T>): FindCursor<T>;
find(filter?: Filter<TSchema>, options?: FindOptions<TSchema>): FindCursor<TSchema> {
find(filter: Filter<TSchema>, options?: FindOptions): FindCursor<TSchema>;
find<T = TSchema>(filter: Filter<T>, options?: FindOptions): FindCursor<T>;
find(filter?: Filter<TSchema>, options?: FindOptions): FindCursor<TSchema> {
if (arguments.length > 2) {
throw new MongoInvalidArgumentError(
'Method "collection.find()" accepts at most two arguments'
Expand Down
32 changes: 26 additions & 6 deletions src/cursor/aggregation_cursor.ts
Expand Up @@ -9,7 +9,6 @@ import type { Callback, MongoDBNamespace } from '../utils';
import type { ClientSession } from '../sessions';
import type { AbstractCursorOptions } from './abstract_cursor';
import type { ExplainVerbosityLike } from '../explain';
import type { Projection } from '../mongo_types';

/** @public */
export interface AggregationCursorOptions extends AbstractCursorOptions, AggregateOptions {}
Expand Down Expand Up @@ -135,22 +134,43 @@ export class AggregationCursor<TSchema = Document> extends AbstractCursor<TSchem
* In order to strictly type this function you must provide an interface
* that represents the effect of your projection on the result documents.
*
* **NOTE:** adding a projection changes the return type of the iteration of this cursor,
* By default chaining a projection to your cursor changes the returned type to the generic {@link Document} type.
* You should specify a parameterized type to have assertions on your final results.
*
* @example
* ```typescript
* // Best way
* const docs: AggregationCursor<{ a: number }> = cursor.project<{ a: number }>({ _id: 0, a: true });
* // Flexible way
* const docs: AggregationCursor<Document> = cursor.project({ _id: 0, a: true });
* ```
*
* @remarks
* In order to strictly type this function you must provide an interface
* that represents the effect of your projection on the result documents.
*
* Adding a projection 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:
*
* @example
* ```typescript
* const cursor: AggregationCursor<{ a: number; b: string }> = coll.aggregate([]);
* const projectCursor = cursor.project<{ a: number }>({ a: true });
* const projectCursor = cursor.project<{ a: number }>({ _id: 0, a: true });
* const aPropOnlyArray: {a: number}[] = await projectCursor.toArray();
*
* // or always use chaining and save the final cursor
*
* const cursor = coll.aggregate().project<{ a: string }>({
* _id: 0,
* a: { $convert: { input: '$a', to: 'string' }
* }});
* ```
*/
project<T = TSchema>($project: Projection<T>): AggregationCursor<T>;
project($project: Document): this {
project<T extends Document = Document>($project: Document): AggregationCursor<T> {
assertUninitialized(this);
this[kPipeline].push({ $project });
return this;
return (this as unknown) as AggregationCursor<T>;
}

/** Add a lookup stage to the aggregation pipeline */
Expand Down
31 changes: 25 additions & 6 deletions src/cursor/find_cursor.ts
Expand Up @@ -12,7 +12,6 @@ import type { ClientSession } from '../sessions';
import { formatSort, Sort, SortDirection } from '../sort';
import type { Callback, MongoDBNamespace } from '../utils';
import { AbstractCursor, assertUninitialized } from './abstract_cursor';
import type { Projection } from '../mongo_types';

/** @internal */
const kFilter = Symbol('filter');
Expand Down Expand Up @@ -344,22 +343,42 @@ export class FindCursor<TSchema = Document> extends AbstractCursor<TSchema> {
* In order to strictly type this function you must provide an interface
* that represents the effect of your projection on the result documents.
*
* **NOTE:** adding a projection changes the return type of the iteration of this cursor,
* By default chaining a projection to your cursor changes the returned type to the generic
* {@link Document} type.
* You should specify a parameterized type to have assertions on your final results.
*
* @example
* ```typescript
* // Best way
* const docs: FindCursor<{ a: number }> = cursor.project<{ a: number }>({ _id: 0, a: true });
* // Flexible way
* const docs: FindCursor<Document> = cursor.project({ _id: 0, a: true });
* ```
*
* @remarks
*
* Adding a projection 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:
*
* @example
* ```typescript
* const cursor: FindCursor<{ a: number; b: string }> = coll.find();
* const projectCursor = cursor.project<{ a: number }>({ a: true });
* const projectCursor = cursor.project<{ a: number }>({ _id: 0, a: true });
* const aPropOnlyArray: {a: number}[] = await projectCursor.toArray();
*
* // or always use chaining and save the final cursor
*
* const cursor = coll.find().project<{ a: string }>({
* _id: 0,
* a: { $convert: { input: '$a', to: 'string' }
* }});
* ```
*/
project<T = TSchema>(value: Projection<T>): FindCursor<T>;
project(value: Projection<TSchema>): this {
project<T extends Document = Document>(value: Document): FindCursor<T> {
assertUninitialized(this);
this[kBuiltOptions].projection = value;
return this;
return (this as unknown) as FindCursor<T>;
}

/**
Expand Down
25 changes: 12 additions & 13 deletions src/mongo_types.ts
Expand Up @@ -169,20 +169,19 @@ export type BSONType = typeof BSONType[keyof typeof BSONType];
/** @public */
export type BSONTypeAlias = keyof typeof BSONType;

/** @public */
export interface ProjectionOperators extends Document {
$elemMatch?: Document;
$slice?: number | [number, number];
$meta?: string;
/** @deprecated Since MongoDB 3.2, Use FindCursor#max */
$max?: any;
}
/**
* @public
* Projection is flexible to permit the wide array of aggregation operators
* @deprecated since v4.1.0: Since projections support all aggregation operations we have no plans to narrow this type further
*/
// eslint-disable-next-line @typescript-eslint/no-unused-vars
export type Projection<TSchema extends Document = Document> = Document;

/** @public */
export type Projection<TSchema> = {
[Key in keyof TSchema]?: ProjectionOperators | 0 | 1 | boolean;
} &
Partial<Record<string, ProjectionOperators | 0 | 1 | boolean>>;
/**
* @public
* @deprecated since v4.1.0: Since projections support all aggregation operations we have no plans to narrow this type further
*/
export type ProjectionOperators = Document;

/** @public */
export type IsAny<Type, ResultIfAny, ResultIfNotAny> = true extends false & Type
Expand Down
11 changes: 7 additions & 4 deletions src/operations/find.ts
Expand Up @@ -15,16 +15,19 @@ import { Sort, formatSort } from '../sort';
import { isSharded } from '../cmap/wire_protocol/shared';
import { ReadConcern } from '../read_concern';
import type { ClientSession } from '../sessions';
import type { Projection } from '../mongo_types';

/** @public */
export interface FindOptions<TSchema = Document> extends CommandOperationOptions {
/**
* @public
* @typeParam TSchema - Unused schema definition, deprecated usage, only specify `FindOptions` with no generic
*/
// eslint-disable-next-line @typescript-eslint/no-unused-vars
export interface FindOptions<TSchema extends Document = Document> extends CommandOperationOptions {
/** Sets the limit of documents returned in the query. */
limit?: number;
/** Set to sort the documents coming back from the query. Array of indexes, `[['a', 1]]` etc. */
sort?: Sort;
/** The fields to return in the query. Object of fields to either include or exclude (one of, not both), `{'a':1, 'b': 1}` **or** `{'a': 0, 'b': 0}` */
projection?: Projection<TSchema>;
projection?: Document;
/** Set to skip N documents ahead in your query (useful for pagination). */
skip?: number;
/** Tell the query to use specific indexes in the query. Object of indexes to use, `{'_id':1}` */
Expand Down
18 changes: 13 additions & 5 deletions test/types/community/collection/findX.test-d.ts
@@ -1,5 +1,6 @@
import { expectNotType, expectType } from 'tsd';
import { expectAssignable, expectNotType, expectType } from 'tsd';
import { FindCursor, FindOptions, MongoClient, Document } from '../../../../src';
import type { Projection, ProjectionOperators } from '../../../../src';
import type { PropExists } from '../../utility_types';

// collection.findX tests
Expand Down Expand Up @@ -35,7 +36,7 @@ await collectionT.findOne(
}
);

const optionsWithComplexProjection: FindOptions<TestModel> = {
const optionsWithComplexProjection: FindOptions = {
projection: {
stringField: { $meta: 'textScore' },
fruitTags: { $min: 'fruitTags' },
Expand Down Expand Up @@ -120,14 +121,15 @@ function printCar(car: Car | undefined) {
console.log(car ? `A car of ${car.make} make` : 'No car');
}

const options: FindOptions<Car> = {};
const optionsWithProjection: FindOptions<Car> = {
const options: FindOptions = {};
const optionsWithProjection: FindOptions = {
projection: {
make: 1
}
};

expectNotType<FindOptions<Car>>({
// this is changed in NODE-3454 to be the opposite test since Projection is flexible now
expectAssignable<FindOptions>({
projection: {
make: 'invalid'
}
Expand Down Expand Up @@ -190,3 +192,9 @@ expectType<FindCursor<{ color: { $in: number } }>>(colorCollection.find({ color:
// 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>(findOptions);
// This is just to check that we still export these type symbols
expectAssignable<Projection>({});
expectAssignable<ProjectionOperators>({});
63 changes: 59 additions & 4 deletions test/types/community/cursor.test-d.ts
@@ -1,6 +1,6 @@
import type { Readable } from 'stream';
import { expectNotType, expectType } from 'tsd';
import { FindCursor, MongoClient } from '../../../src/index';
import { FindCursor, MongoClient, Db, Document } from '../../../src/index';

// TODO(NODE-3346): Improve these tests to use expect assertions more

Expand All @@ -22,7 +22,7 @@ const cursor = collection
.min({ age: 18 })
.maxAwaitTimeMS(1)
.maxTimeMS(1)
.project({})
// .project({}) -> projections removes the types from the returned documents
.returnKey(true)
.showRecordId(true)
.skip(1)
Expand All @@ -31,6 +31,7 @@ const cursor = collection

expectType<FindCursor<{ foo: number }>>(cursor);
expectType<Readable>(cursor.stream());
expectType<FindCursor<Document>>(cursor.project({}));

collection.find().project({});
collection.find().project({ notExistingField: 1 });
Expand Down Expand Up @@ -74,13 +75,13 @@ expectNotType<{ age: number }[]>(await typedCollection.find().project({ name: 1
expectType<{ notExistingField: unknown }[]>(
await typedCollection.find().project({ notExistingField: 1 }).toArray()
);
expectNotType<TypedDoc[]>(await typedCollection.find().project({ notExistingField: 1 }).toArray());
expectType<TypedDoc[]>(await typedCollection.find().project({ notExistingField: 1 }).toArray());

// Projection operator
expectType<{ listOfNumbers: number[] }[]>(
await typedCollection
.find()
.project({ listOfNumbers: { $slice: [0, 4] } })
.project<{ listOfNumbers: number[] }>({ listOfNumbers: { $slice: [0, 4] } })
.toArray()
);

Expand All @@ -98,3 +99,57 @@ void async function () {
expectType<number>(item.foo);
}
};

interface InternalMeme {
_id: string;
owner: string;
receiver: string;
createdAt: Date;
expiredAt: Date;
description: string;
likes: string;
private: string;
replyTo: string;
imageUrl: string;
}

interface PublicMeme {
myId: string;
owner: string;
likes: number;
someRandomProp: boolean; // Projection makes no enforcement on anything
// the convenience parameter project<X>() allows you to define a return type,
// otherwise projections returns generic document
}

const publicMemeProjection = {
myId: { $toString: '$_id' },
owner: { $toString: '$owner' },
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<InternalMeme>('memes');

expectType<PublicMeme[]>(
await memeCollection
.find({ _id: { $in: [] } })
.project<PublicMeme>(publicMemeProjection) // <==
.toArray()
);

// Returns generic document when no override given
expectNotType<InternalMeme[]>(
await memeCollection
.find({ _id: { $in: [] } })
.project(publicMemeProjection)
.toArray()
);

// Returns generic document when there is no schema
expectType<Document[]>(
await new Db(new MongoClient(''), '')
.collection('memes')
.find({ _id: { $in: [] } })
.project(publicMemeProjection)
.toArray()
);

0 comments on commit 48d6da9

Please sign in to comment.