-
Notifications
You must be signed in to change notification settings - Fork 119
Code duplication within union
and dispatch
#295
Comments
const Price = t.union((x) => t.Number.is(x)
? t.Number
: PriceObject
) Unions like that are not introspectable though Price.meta.types // ??? Admittedly const Price = t.union([t.Number, t.interface({
from: t.Boolean,
value: t.Number
}, { name: 'PriceObject', strict: true })])
const x = Price(1) // ok
const y = Price({from: true, value: 2}) // ok |
Yeap, good point. I realized that yesterday too. Definitely not an option then What do you think about passing types to dispatch then? For instance, after giving it a better thought, it seems to be quite an option, especially with destructuring: const PriceObject = t.struct({
from: t.Boolean,
value: t.Number
}, { name: 'PriceObject', strict: true })
const Price = t.union([t.Number, PriceObject])
Price.dispatch = (x, [Number, PriceObject) => Number.is(x)
? Number
: PriceObject It might seem like we have here even more code duplication, but it is different. In original code we had to pass same types few times and they don't have any real relation to union declaration itself: const PriceObject = t.struct({
from: t.Boolean,
value: t.Number
}, { name: 'PriceObject', strict: true })
const Price = t.union([t.Number, PriceObject])
Price.dispatch = (x) => t.Number.is(x) // I can't make here check against one of declared within union types, instead I'm making a wild guess, and if union will change, this part won't reflect change
? t.Number // this is completely new, and doesn't relate to union itself directly
: PriceObject // this is a thing from outside world, it will silently break if I will change second argument in union In case of types passing to dispatch and destructuring we're reusing already declared types within union and tied to them. Besides, it will allow to avoid declaring types within variables just to pass them to union and dispatch: const Price = t.union([t.Number, t.struct({
from: t.Boolean,
value: t.Number
}, { name: 'PriceObject', strict: true })])
Price.dispatch = (x, [Number, PriceObject) => Number.is(x)
? Number
: PriceObject
Ah, thanks for the hint! After reading API docs I didn't realize that That completely resolves particularly my issue, but my proposal above remains actual, since it will allow to express better relation between union and dispatch. |
Looks good to me, would you like to send a PR? |
Hey,
This way you get your desired functionality + you dont need to mess with internal tcomb code. Dont forget to add validations, I dropped them for the sake of the example. |
Version
3.2.23
The issue
Let's take an example:
Looks clean and elegant. But to use that union, we have to declare a
dispatch
method. And here comes the issue:To resolve that issue, we need to declare struct as a standalone variable:
And suddenly that doesn't look that elegant anymore. More of it, it still involves nasty code duplication:
Which boils down to that point that union doesn't solve that task as good as it would be expected to.
Proposals
To pass types to dispatch?
Doesn't look reliable, but in some cases might be better alternative
However, I have a feeling that it should look like this instead, where dispatch defines our "custom" union at first place, thus we don't need to declare relatively close thing twice:
It almost look like we need to use here refinement or irreducible:
However, it will make validation errors less informative. Besides, with such approach,
t.validation()
will stop on first error of irreducible and will not validate further.Instead of this (for union with large object, which has multiple Offer entries with errors):
It will return only:
The text was updated successfully, but these errors were encountered: