-
Notifications
You must be signed in to change notification settings - Fork 1.8k
fix(NODE-3454): projection types are too narrow #2924
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 8 commits
b57859c
6810be3
fa9c234
8bf04f4
d02193f
8c5f29f
d8b55b4
0e1ca61
48aa991
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -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 of 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 of aggregation operations we have no plans to narrow this type further | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
*/ | ||||||
export type ProjectionOperators = Document; | ||||||
|
||||||
/** @public */ | ||||||
export type IsAny<Type, ResultIfAny, ResultIfNotAny> = true extends false & Type | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wouldn't setting this to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You mean setting |
||
/** 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}` */ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 | ||
|
||
|
@@ -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) | ||
|
@@ -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 }); | ||
|
@@ -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()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought this was supposed to yield Document[], which shouldn't match TypedDoc[]? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Corrected the test, I think this is correct, unless we want to change the return to always be generic? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is part of the larger issue that we discovered, we'll tackle it in NODE-3468 |
||
|
||
// Projection operator | ||
expectType<{ listOfNumbers: number[] }[]>( | ||
await typedCollection | ||
.find() | ||
.project({ listOfNumbers: { $slice: [0, 4] } }) | ||
.project<{ listOfNumbers: number[] }>({ listOfNumbers: { $slice: [0, 4] } }) | ||
.toArray() | ||
); | ||
|
||
|
@@ -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' // <== cause of TS2345 error | ||
}; | ||
const memeCollection = new Db(new MongoClient(''), '').collection<InternalMeme>('memes'); | ||
|
||
expectType<PublicMeme[]>( | ||
await memeCollection | ||
.find({ _id: { $in: [] } }) | ||
.project<PublicMeme>(publicMemeProjection) // <== Argument of type T is not assignable to parameter of type U | ||
.toArray() | ||
); | ||
|
||
// Returns generic document when no override given | ||
expectNotType<InternalMeme[]>( | ||
await memeCollection | ||
.find({ _id: { $in: [] } }) | ||
.project(publicMemeProjection) // <== Argument of type T is not assignable to parameter of type U | ||
.toArray() | ||
); | ||
|
||
// Returns generic document when there is no schema | ||
expectType<Document[]>( | ||
await new Db(new MongoClient(''), '') | ||
.collection('memes') | ||
.find({ _id: { $in: [] } }) | ||
.project(publicMemeProjection) // <== Argument of type T is not assignable to parameter of type U | ||
.toArray() | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.