Feature/ts type improvements

[taxilian]

Summary

While the new types are pretty good, there are some shortcomings which I wanted to address; I've discussed with @vkarpov15 in the past and I hope that the way I've addressed these will be acceptable.

There are several improvements here, most tied to LeanDocument and related:

• If the DocType you are using has methods then LeanDocument will strip them out
• If any of the fields are of type "any" there is a minor optimization which will help prevent it from being treated as an array and going an extra level deep (mostly meaningless, but I've found typescript can sometimes be really picky about how many levels of recursion it allows)
• LeanDocument will -- when possible -- find the original non-mongoose type of Subdocuments both directly nested and in a DocumentArray instead of taking the derived type and trying to just remove the base types.
• LeanDocument will -- when possible -- infer the actual source DocType instead of taking the derived type and trying to remove the fields which were added. It will still process other fields in order to e.g. convert Types.Array to regular arrays and DocumentArrays to the source subdocument type.
• I added two new type helpers: QueryForModel and QueryOneForModel. These should help to construct the type returned by Model.find with the correct typings by inferring the types. Because of the overloads it's not possible to directly get the ReturnType of the find function, so this caused me some issues. I can pull them back out if you don't want them, but I found them necessary when updating from my own types (@alttypes/mongoose) to the official ones due to a few places where I wanted to return an instance of the Query.
• Where appropriate I have made Query helpers to be field aware based on the types in FilterQuery -- this way things like $gte and others are able to work as well as rejecting if the field type isn't valid for that operator. Since it relies on FilterQuery it will still work if the field isn't "known" falling back to the default.

Examples

if you have interface User and you construct a Schema from it you should be able to use LeanDocument and you will get your original User interface back. If there were methods they will be removed.

If you have a Types.DocumentArray field then LeanDocument will change it to User[] instead of something like Omit<User, .........>[] -- both much cleaner and also avoids incompatibilities that derive from expecting them to be a certain thing.

Please let me know what questions or concerns you have.

I would also strongly recommend that we make all added methods and members on Array and DocumentArray optional, because currently if you try to assign a normal array to a Types.Array field it will fail in the types because the types aren't compatible. I can do that but didn't want to tie it up in this PR.

I would also recommend that we make the _id field on ObjectId optional so that a regular bson ObjectId object (from mongodb or the bson package) can be assigned to it without type errors.

[Uzlopak]

Hi,

please add/modify typings Tests, which you find in test/types.

Only Change i don't understand is why Lean document should not strip methods from the Document. I understand lean Object to be a POJO.

[taxilian]

Yeah, the lean document test is strange; I'll look into and fix that. I didn't see that there were type tests -- that's fantastic, I'll add tests for what we need.

[vkarpov15]

Can we break up this PR into smaller PRs? There's some changes here that shouldn't break anything and I'd like to merge, but some other changes that I'm hesitant about.

[vkarpov15]

I'm very wary of using infer because it has caused some serious performance issues for us in the past. Can you please provide an example of why this type is helpful?

[taxilian]

This is just a helper utility type, so you can always not use it if it causes performance issues. Example of use would just be any time you need to return a query or create one but want to assign it to a variable declared earlier.

How I was using it:

function applyQueryRestrictions(userQ: mongoose.QueryForModel<typeof UserModel>, requiredPerm?: string) {
    if (requiredPerm) {
        userQ.where('permissions', requiredPerm);
    }
    userQ.select('passInvalid username call first last email modified permissions')
        .select('verifiedCall.callsign verifiedCall.displayName verifiedCall.sigdate')
        .select('providers.type');
    userQ.populate({
        path: 'veDocs',
        select: 'isTeamLead active deleted number display_name vec call',
        populate: {
            path: 'ulsRecord',
            select: 'callsign frn license_class expired_date',
        }
    });
    return userQ;
}

I certainly could have constructed the query differently, but I wanted to keep type checking as much as possible and it's much easier to do QueryForModel<MyModel> than it is to do QueryWithHelpers<ResultDoc[], ResultDoc, {}, DocType> -- not to mention that you have to figure out what all of those are. My original plan was to just do type MyQueryType = ReturnType<typeof MyModel.find> but since the return type of the different .find overrides aren't all identical that doesn't return a useful type.

Since this is just a utility method I can pull it and put it in my own code -- just seemed like something that others would also find useful and since it's purely optional it didn't seem like something which would cost anything to put in. Your call, just let me know.

[vkarpov15]

Good change 👍

[taxilian]

Well, I thought it was, but it turns out it breaks a few cases -- specifically cases where someone extends Document.

I'm still working through the tests

[taxilian]

After some tinkering I've made the following changes to toJSON and toObject:

• I removed the redundant signatures -- the generic version has a default template type, so there is no need to specify a type without it, that just confuses things and could throw off type inference in some cases
• I have left it using LeanDocument, though that can be overridden with a template.

The downside to this is that if your interface extends Document instead of using the new recommended way then you'll get a any type out of both methods unless you manually provide a type to the generic.

On the positive side, though, I could not find any way to use this in the return type which did not cause circular reference issues with my code in multiple places -- this change fixes that. Additionally, the "worst case" is that you get an any type out, so it won't break existing code it just may not catch issues that someone would expect it to catch. That can be mitigated by either migrating to the recommended method or manually providing the type.

[taxilian]

I've fixed the tests and added some additional tests; if you want more tests added just let me know what you want illustrations of / tests around. I still want to address the function thing because I don't understand how it could cause issues as long as it only omits the members if it can confirm that it's a function, but that can be addressed in a separate PR after I've had time to dig into the other issue mentioned and make sure I have reasonable responses to the arguments made =]

I haven't removed the two Query type helpers either, but let me know if you want me to. I think they are useful enough to leave in, and the performance shouldn't be a big deal unless you use them a lot which I don't expect would be the case but it's not a big deal.

Let me know what else you'd like to see on this PR.

[taxilian]

What further changes would you like made to this PR? Or additional tests / testing? This needs to be resolved for #11673 to be viable =]

[vkarpov15]

Works for me, thanks for the discussion here and on Slack 👍

The only thing I'll do after merging is remove the QueryForModel<> helpers for now. It's a reasonable helper, but I'd rather avoid having to support it in our types for now.

[iammola]

Hey, I was just reading through the diffs and noticed this minor error.

You changed the name of the function. From gt to gte and there is now only one overload for the gt function.

@vkarpov15 @taxilian

[taxilian]

Ack! You're right, good catch! @vkarpov15 very sorry about that! I'm assuming you'll want to just fix it since it's merged already? Let me know if you need me for anything

[Uzlopak]

covered by #11760