strip option to Model.hydrate(...)

[AbdelrahmanHafez]

In #15638 we created a virtuals: boolean option to Model.hydrate(...) that allows people to hydrate virtual paths to aggregated results, the virtual path mentioned in #15627 doesn't have a particular type, it's the equivalent of "Mixed" which I assume is just added for this purpose. IMO the "virtual" path here is not special at all, it's just added to enable adding an arbitrary field to be hydrated.

I wonder if we should have a strip option that defaults to true, this option would dictate how Mongoose should handle fields that are not explicitly defined in the schema. I can now think of many cases where I would've liked to cast aggregation results without stripping the additional fields by using Model.hydrate(rawResult, null, { strip: false });.

Thinking out loud here: If we agree to implement that, we can even take it further and make Model.aggregate(...) accept a cast: boolean, strip: boolean, that would enable people to do cast: true, strip: false, which would be very useful for casting aggregation pipelines that mostly add/remove fields without fundamentally changing the shape of the document, while keeping the non-defined fields as is. I'd even think it's worth discussing making this the default behavior of aggregation pipelines, I can see a couple of arguments for both with and against this.

I'd be happy to implement these features if we agree to add them.

What do you think? @vkarpov15 @hasezoey

[vkarpov15]

Check out PR #15944

[yakubka]

The strip option idea is compelling. The current behavior where hydrate silently drops fields not in the schema is correct for most use cases but becomes an obstacle when the aggregation pipeline produces computed fields or lookup results that you want to keep attached to the hydrated document.

A strip: false option on hydrate would let you get the best of both: run getters and virtuals from the schema, access document methods like save, and still read back the extra aggregated fields.

The concern worth thinking through before implementing is what happens to TypeScript types. With strip: true the return type of hydrate is HydratedDocument which is well-typed. With strip: false the extra fields are unknown at the schema level, so the return type would need to accept a generic parameter for the extra shape, similar to how populate already takes a generic to describe populated paths.

A signature like Model.hydrate<Extra = {}>(obj, projection, { strip?: boolean }) returning HydratedDocument<T & Extra> would let callers type-assert the extra fields they expect without losing the schema-defined paths.

One edge case to consider is setter behavior on the extra fields. When strip is true, setters only run on schema-defined paths. When strip is false, the extra paths have no schema entry, so there is nothing to run a setter against. That is probably the right behavior but it should be documented clearly to avoid confusion with fields that happen to share a name with a schema path that has a setter.

[AbdelrahmanHafez]

@yakubka strict: true was already implemented in #15944, I created #16374 to handle the types issue you raised.

[vkarpov15]

"One edge case to consider is setter behavior on the extra fields. When strip is true, setters only run on schema-defined paths. When strip is false, the extra paths have no schema entry, so there is nothing to run a setter against. That is probably the right behavior but it should be documented clearly to avoid confusion with fields that happen to share a name with a schema path that has a setter." - I don't understand this comment. An extra field that has the same name as a schema path is not an extra field?

[yakubka]

Good to know, thanks for the pointer to #15944 and for opening #16374. I will take a look at the strict option in practice against the aggregation pipeline use case and leave any notes on #16374 if the getter interaction with extra paths behaves differently than expected.