Deep defaults
We're currently in the process of migrating from mongoose to papr. Here's a question that I have:
Say I have schema for users collection:
const userSchema = schema(
{
numPosts: types.number(),
emailPreferences: types.object({
monthly: types.boolean(),
newsletter: types.boolean(),
followers: types.boolean(),
}),
appreciations: types.array(types.object({
badge: types.objectId(),
count: types.number(),
})),
},
{
defaults: {
numPosts: 0,
emailPreferences: {
monthly: true,
newsletter: true,
followers: true,
},
appreciations: [
{
count: 0
}
],
}
}
)
As you can see, I have 3 root fields in this schema: numPosts, emailPreferences, and appreciations. I use papr's default functionality for all 3.
For numPosts, it's straight forward.
As for emailPreferences, what happens when I try to insert an object that only has one field defined ? will papr merge that with the defaults of the 2 remaining fields ?
EDIT: The answer is no. It will not be merged w/ the other defaults. When I tried the following insert:
UserModel.insertOne({
name: "dsafnaisdhfujabsf1",
email: "[email protected]",
username: "dsafnaisdhfujabsf1",
emailPreferences: {
newsletter: false,
},
});
The resulting document only included newsletter: false:
...
emailPreferences: { newsletter: false },
This is not ideal. The expected behaviour here is that it should merge the one field I defined with other undefined fields from the defaults object.
And as for appreciations, how do I represent defaults for an array of objects ? is this the correct way ?
What would happen if I try to insert this document without appreciations field ? will it insert just the count field ?
EDIT: the answer is yes. Papr is going to insert an array that contains: appreciations: [{ count: 0 }] by default.
The expected behaviour here is that these defaults are only used when I try to insert an array. How do I achieve this ?
Does Papr allow me to use nested schemas ? EDIT: the answer is no. Papr models will work, but they will not make use of the nested schema.
Overall, I think the current API for defaults in papr is really limited and confusing. I believe defaults shouldn't be in a separate object. They should be included in the type definitions themselves. Which is the case in mongoose and ts-mongoose:
newsletter: Type.boolean({ default: false })
Is such a change possible/planned ? And if not, how can I achieve the same capabilities for defining defaults using Papr ?
Thanks a lot for your amazing package.
@avaly please take a look at this. Thanks a lot.
So I started today by testing things out. I edited the main issue body to reflect my findings.
Thank you so much for this issue and for the detailed notes and testing @BahaaZidan!!
You're absolutely right that defaults are quite limited currently. When we built papr we consciously wanted to keep the API as simple as possible, avoiding much of the complexity that makes a library like mongoose so unwieldy. For example, we likely won't want to support dynamic defaults because of the complexity they would introduce.
That said, you bring up some quite valid limitations of defaults as currently spec'd. All of your examples are absolutely valid use-cases we might consider supporting, though I wouldn't go so far as to give an estimate as to when we would have a chance to look at them. But we are always open to PRs!
Overall, I think the current API for defaults in papr is really limited and confusing.
I don't disagree with you on this one, and an API that more closely matches mongoose was considered when we initially designed the library. At the time it was decided that it would introduce too much overhead to the design, but we haven't revisited it since, and with the evolution of Typescript in the intervening time, it may be more feasible.
@ejmartin504 Thanks for the response.
If I'm to implement this, would you allow such a API change ? or will I have to implement in a way that's compatible with both APIs ?
@BahaaZidan there are two issues here:
- the defaults not working as expected for nested objects
- the API design
Are you suggesting working on the API design itself? I think we might be open to such a change, depending on the complexity, but I would recommend submitting PRs for the actual issues with deep defaults you're running into first.
@ejmartin504 I might be wrong here. But I think the deep default limitation is a result of the API design. As it is right now, I'm finding it really hard to implement it without limiting one use case in favor of the other.
Let's say I have this schema:
const userSchema = schema(
{
appreciations: types.array(types.object({
badge: types.objectId(),
count: types.number(),
})),
},
{
defaults: {
appreciations: [
{
count: 0
}
],
}
}
)
There are a couple different use cases that I can think of right now:
- I want the default value of
appreciationsto be an empty array. But at the same time I want that whenever someone tries to insert an object, the default value ofcountshould be taken into account. - I want the default value of
appreciationsto be an array that contains one object with the value ofcount. [What Papr currently supports]
How can we support both use cases with the current API ? Because I believe both of them are valid.
@BahaaZidan that is a good point. I guess I was thinking your example of emailPreferences might be a more reasonable place to start given the current API. The appreciations situation with the array is quite a bit more difficult to conceptualize with the current API design, I agree.
So if you want to propose and implement an alternative API that accounts for this use-case, we will certainly consider it!
@ejmartin504 From an API standpoint, I think the defaults API of ts-mongoose (may she rest in piece) is a good place to be at.
const userSchema = schema(
{
appreciations: types.array(types.object({
badge: types.objectId(),
count: types.number({ default: 0 }),
}), { default: [] }),
}
)
What do you think about this ?
I'm OOO now, but I saw this thread and wanted to comment.
Regarding your last comment about the API resembling the ts-mongoose one, I remember that was an initial implementation of our first version of papr, before we open sourced it. But we had to move away from that API, because we need the defaults type as a generic type as well at the top level of a Model in order to use it in certain methods like insertOne, etc.
If you can make this API change, and keep all the existing functionality in the tests, we're more than happy to consider such a change.
With #527 and #678 I think the defaults logic is pretty advanced at this point.