-
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 all 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 |
---|---|---|
@@ -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' // <== (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() | ||
); |
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.
Wouldn't setting this to
TSchema
make the default functionality the same but still allow a more granular level of control without breaking things? And remove the need for the eslint disableThere 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.
You mean setting
projection?: TSchema
? I think that would cause issue with the typical projection of inclusion / exclusion like{ _id: 0, name: 1 }
. wherename: string
in the TSchema