types: Update Expression Typings

[andreialecu]

Summary

Fixes #11952

[Uzlopak]

Let me see

[Uzlopak]

We have to think about this more

[Uzlopak]

No. Path is just a Path. Not ObjectIdLike.

Suggested change

	type Path = string | ObjectIdLike;
	type Path = string;

[Uzlopak]

I created a separate type, so that one day if we have good way to generate valid paths by mongoose Schema we would already have it properly encapsulated it.

[Uzlopak]

No this is not a Conditional Expression Operator
see https://www.mongodb.com/docs/manual/reference/operator/aggregation/#conditional-expression-operators

[Uzlopak]

Suggested change

Expression.Exists;

[Jule-]

I am not aware of all changes but it seems that $exists is not accepted anymore by the typings.

Wherever you put it, this should work:

const result = await Model.aggregate([{
  $match: { optional: { $exists: true } }
}]);

mongoose@6.4.0:

Type '{ $exists: true; }' is not assignable to type 'Expression'.

[Uzlopak]

Suggested change

	Expression.Switch |
	Expression.Switch;

[Uzlopak]

I think this is kind of wrong.

[andreialecu]

@Uzlopak just wanted to get the ball rolling.

If you want to open your own PR (please!), feel free to. You can copy the tests from mine.

[Uzlopak]

I think there is a mixup. What you want to add is a Query Operator, but the expressions are for aggregations. I have to think how we can solve this without mixing it up.

[andreialecu]

There's also a problem with booleans, like {$ne: true}, and I'd assume numbers as well.

Type '{ $ne: true; }' is not assignable to type 'Expression | undefined'.
              Types of property '$ne' are incompatible.
                Type 'true' is not assignable to type 'Expression | [Expression, Expression] | undefined'.

[Uzlopak]

Well it is wrong because in an aggregation pipeline $ne is an array with two values, which are compared.

[Uzlopak]

https://www.mongodb.com/docs/manual/reference/operator/aggregation/ne/

[andreialecu]

Both are valid in different contexts, see: https://stackoverflow.com/a/15330673/114732

[andreialecu]

From mongodb docs at https://www.mongodb.com/docs/manual/reference/operator/aggregation/match/#mongodb-pipeline-pipe.-match:

$match takes a document that specifies the query conditions. The query syntax is identical to the read operation query syntax; i.e. $match does not accept raw aggregation expressions. Instead, use a $expr query expression to include aggregation expression in $match.

[andreialecu]

I think $match is defined incorrectly now, and it needs to be something like this instead:

    export interface Match {
      /** [`$match` reference](https://docs.mongodb.com/manual/reference/operator/aggregation/match/) */
      $match: FilterQuery<any>;
    }

It would be great if instead of any, T could be better passed down, but that seems to require a bit of additional work to wire up everywhere.

[Uzlopak]

I think we have to rename expressions.d.ts to aggregationpipelineexpressions.d.ts and create also corresponding types for the other operations and then decide how and which expressions are allowed when and where

[andreialecu]

I think the main issue is with $match being defined as using expressions when instead it's a regular filter query.

There's a special case for that one. I had to step away for a bit so I can't verify, but simply fixing that might fix most of the issues in one shot.

[andreialecu]

Actually FilterQuery<T> won't work properly now that I think of it. It needs to be FilterQuery<any> because a previous step might do a projection which changes the shape of the document arbitrarily.

[vkarpov15]

Would just adding | null to Expression work better here? So you can use null in other places as well.

[vkarpov15]

I'll merge this for now to unblock @andreialecu .

However, there are still some issues with the expression typings. For one thing @Uzlopak , I think we'll have to add AnyObject, because the following code will still fail:

  {
    $match: {
      _id: new ObjectId('stringObjecId'),
      a: { $exists: true },
      b: null,
      c: 'test',
      d: { foo: 42 } // <-- valid, but current typings will fail
    }
  }

[Uzlopak]

I'll be honest:
I think we should revert the expressions PR. It needs more work to work properly.

[andreialecu]

Indeed, this wasn't totally ready to merge - I forgot to set it to draft. I thought @Uzlopak would work on his PR.

I also wanted to suggest reverting the expressions PR, but it might not be necessary with this change:

    export interface Match {
      /** [`$match` reference](https://docs.mongodb.com/manual/reference/operator/aggregation/match/) */
      $match: FilterQuery<any>;
    }

$match is supposed to be a regular filter; it's not an aggregation expression - I think that's where most of the problems originate. With this change, the scenario mentioned by @vkarpov15 should also work correctly.

I opened #11969 with a better fix

[lucamodica]

Does $nin operator still exists? For the moment I have to workaround with

{ $not: { $in: [<expression>, <arrayExpression>] } }

[lucamodica]

I ask the same question for the $exists operator

[vkarpov15]

@lucamodica yes $nin still is an operator on the MongoDB server. Does $nin not work for you in Mongoose 6.4?

[lucamodica]

Nope, or at least both $nin and $exists are not recognized as Expression .

[andreialecu]

#11969 should fix this.