Required oneOf relation

[timkolotov]

So I'm back to experimenting with the library and trying to implement real-life models with relations.

I see some inconsistency with oneOf relation. When a model definition has simple fields, the field always contains some data (e.g. empty string for string fields). So the field is kind of required unless explicitly nullable and will not be undefined. Same for manyOf, it just returns an empty array (https://github.com/mswjs/data/pull/201)

But oneOf relation field can have undefined because this field cannot be filled in with some mocked data. As a result, we can have two no data value: null - when the field is explicitly defined as nullable, and undefined when a field is not nullable but a value was not provided when creating an instance.

From my perspective, if a value for oneOf relation is not passed, the creation must fail. Maybe for keeping the existing behavior (can't think about the cases when it is needed, though) it can be done with required attribute for the relation like with unique.

It wouldn't be a big problem as the data is being created by devs in tests, but the resulting TS types for models just don't work with existing types.

[timkolotov]

@kettanaito, I would like to hear your thoughts about it. If this makes sense, I'd implement this and create PR.

A simple use case is: we have a model Employee with fields id, name, email, etc., and also a company which is oneOf relation to the Company model. Employee can never exist without Company so employee.company may never be undefined, otherwise we'd use nullable(oneOf('Company')).

[kettanaito]

Hi, @timkolotov. This is a great proposal. The consistency you describe is the way to go:

• oneOf() relationships must have initial value. If not provided, the library should throw. If provided, use the current logic as-is;
• If the consumer wishes to have a nullable one-of relationship, describe it explicitly via nullable(oneOf(modelName)).
  • If marked as nullable and no initial value was provided to the relationship, set its value as null.
  • If marked as nullable and initial value was provided to the relationship, set its value to the initially provided value.
  • If marked as nullable, allow updating this relational property to null (i.e. via update etc).

Most of this is already covered by nullable(). The change would be to treat oneOf relationship as non-nullable by default.

Does this sound interesting to you to give it a try? I think the change itself should be rather scoped. I can always help with the code review.

[timkolotov]

Yep, I'm in. Currently, I just patched the types to shut the TS compiler up, which isn't really safe...

[harry-gocity]

Any update on this issue? Really interested in this as I think I've run into a similar problem. Not sure of the best way around this, with strict mode due to undefined:

// My MSW handler response type
type BasketResponse = {
    basketId: string,
    currency: {
        code: string,
        symbol: string
    }
}

export const db = factory({
    basket: {
        basketId: primaryKey(String),
        currency: oneOf('currency'),
    },
    currency: {
        code: primaryKey(String),
        symbol: String,
    }
})

const currency = db.currency.create()
const basket = db.basket.create({ currency });

type currencyType = typeof currency; // PublicEntity< ... >
type basketCurrencyType = typeof basket.currency; // PublicEntity< ... > |  undefined

const value: BasketResponse = basket // 'Type 'undefined' is not assignable to type '{ code: string; symbol: string; }'.

This makes it a bit awkward to use the basket pulled directly from the db with a MSW handler typed as rest.post<CreateBasketRequest, PathParams, BasketResponse>

[timkolotov]

@harry-gocity it does look like the same issue indeed. I started working on it but didn't have much time so far. With enough luck I'll have a vacation next week and will be able to dedicate time for this.

[timkolotov]

@kettanaito finally finished the implementation, please take a look (especially check if the changes in tests make sense to you, some cases I just removed, because they were testing the behavior that doesn't exist anymore).