-
-
Notifications
You must be signed in to change notification settings - Fork 54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Required oneOf relation #269
Comments
@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 |
Hi, @timkolotov. This is a great proposal. The consistency you describe is the way to go:
Most of this is already covered by 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. |
Yep, I'm in. Currently, I just patched the types to shut the TS compiler up, which isn't really safe... |
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 // 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 |
@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. |
@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). |
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 (#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 twono 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 withrequired
attribute for the relation like withunique
.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.
The text was updated successfully, but these errors were encountered: