fix: Strict Projection Object Typing

[pshaddel]

Summary
Fixes this issue: #13840

Examples

[AbdelrahmanHafez]

One minor comment, otherwise LGTM. Great work!

[vkarpov15]

A couple of use cases I'd like to see tests for that don't look like this PR supports

1. Allow adding fields that aren't in the schema to the projection
2. Projection operators: $slice, $elemMatch
3. Dotted paths, like 'child.name': 1
4. Using $meta with text search: score: { $meta: 'textScore' }

Will have to review some more to see if there are any other projection cases that we've missed

[vkarpov15]

Another thing we'll need to make sure of: $slice and $meta are not "defining projections" in the sense that they can be used with either inclusive or exclusive projections. { name: 1, arr: { $slice: 5 } } is valid as is { name: 0, arr: { $slice: 5 } }.

[pshaddel]

Another thing we'll need to make sure of: $slice and $meta are not "defining projections" in the sense that they can be used with either inclusive or exclusive projections. { name: 1, arr: { $slice: 5 } } is valid as is { name: 0, arr: { $slice: 5 } }.
@vkarpov15

$slice and $elemMatch are implemented and whenever there is a array we can use these fields.

For using $meta, we do not know if there is a text index on the document or not; therefore, do you think we should allow this type({ score: { $meta: "textScore" } }) in any projection?

[pshaddel]

A couple of use cases I'd like to see tests for that don't look like this PR supports

1. Allow adding fields that aren't in the schema to the projection
2. Projection operators: $slice, $elemMatch
3. Dotted paths, like 'child.name': 1
4. Using $meta with text search: score: { $meta: 'textScore' }

Will have to review some more to see if there are any other projection cases that we've missed

1. resolved, we have this test:

Test.find({}, { someOtherField: 1 }); // This should be allowed since it's not a field in the schema

2. resolved
3. Dotted Path
   It was quite complicated due to existence of arrays and the way we can project arrays using . but finally I could implement it. I had to limit the depth of the dot path to 4 because of circular types(also it could easily slow down Typescript server when the depth of the object is too much.
   I also tried to support a combination of dot path and objects. For example:

interface X {
  docs: { users: { name: string } }[]
}

type X = Projection<X>
const x: X = { 'docs.users': { name: 1 } } // now supports typing of  this inner object and it suggests `name`

[fider]

@pshaddel great feature!
Will typed projection parameter also have influence on query return type?

type EntityA = { a: number, b: ... }
const a = A.findOne({}, { a: 1 }) // typeof a == { a: number }

[pshaddel]

@pshaddel great feature!
Will typed projection parameter also have influence on query return type?

type EntityA = { a: number, b: ... }
const a = A.findOne({}, { a: 1 }) // typeof a == { a: number }

Thanks! Still working on this feature. My intention is to implement Query Return Type in another task after tackling this one.

[vkarpov15]

LGTM. Will merge this into 8.1.

[vkarpov15]

@pshaddel looks like I merged a bit too early, literally the next issue I looked at highlighted an issue with this PR 😞 . #3230 (comment) shows that Mongoose lets you deselect the discriminator key in an inclusive projection in addition to _id. What do you think about this - should we just get rid of checking for mixed inclusive/exclusive projection? Or find a way to pass in the discriminator key to this logic?

[pshaddel]

@pshaddel looks like I merged a bit too early, literally the next issue I looked at highlighted an issue with this PR 😞 . #3230 (comment) shows that Mongoose lets you deselect the discriminator key in an inclusive projection in addition to _id. What do you think about this - should we just get rid of checking for mixed inclusive/exclusive projection? Or find a way to pass in the discriminator key to this logic?
@vkarpov15
This should not be a problem since this discriminator key is not in the interface, so the type of the key would be any. it can be anything.

[vkarpov15]

That's good, but it is still valid to add discriminatorKey to your schema: new Schema({ __t: String }) is fully valid.

[vkarpov15]

@pshaddel I reverted this PR in 8.1 for now with 5337da9, because I don't want the test failures and other issues to block releasing 8.1. Please create a new PR with the suggested fixes; we will likely take another look at this PR for a future release as well.

[pshaddel]

OK

[alaycock]

@vkarpov15 do you know if this was ever reintroduced? I don't see a subsequent PR from @pshaddel

If not, maybe it's worth reopening #13840

[pshaddel]

@vkarpov15 do you know if this was ever reintroduced? I don't see a subsequent PR from @pshaddel

If not, maybe it's worth reopening #13840

No further PR from my side, I think a test was failing because of some other changes. I can take a look.

[pshaddel]

Just opened a new one:
#15327