VariantCreator output hinting for custom discriminants

[paarthenon]

VariantCreator output hinting for custom discriminants

[paarthenon]

Hey there,

I want to thank you for your efforts. This looks great, and I've confirmed the matching on creator functions is working as expected. I'll try to cut another dev build soon so these changes are publicly accessible.

[cwstra]

Been looking at this for a bit. Unless I'm missing a trick, the general idea for the second bullet point would essentially be to add Record<K, T> to the Outputs<K, T> type. However, this changes how Outputs<K, T> relates to K, in a way that would be nasty to extract.

At the moment, Outputs<K, T> places K as a value in an object; this makes it covariant with regards to K. In other words, for any string type K, Outputs<K, T> is a subset of Outputs<string, T>. variant makes use of this in many places. For example, consider:

export type VMFromVC<T extends VariantCreator<string, Func, string>> = {
    [P in T['type']]: Extract<T, Record<'type', P>>;
}

Because VariantCreator is covariant with regards to all three of its properties (which includes a K!), this definition can have T extend VariantCreator<string, Func, string>, the most generic possible VariantCreator.

However, an object is not covariant with regards to its property names, but contravariant: the most generic object is {}, which has no property names whatsoever. This means that, if Outputs were to use K as a property name, one part of the object will be covariant with regards to K, and the other contravariant, which forces the overall Outputs to be neither. That would break a large number of the extends statements of the form mentioned above, since there would no longer be a 'most generic' VariantCreator. We would instead have to pass in K to each of these functions, which I believe would trickle all the way up to things like TypeNames and VariantOf.

That doesn't necessarily mean that the change is impossible, but it would probably require moving those extends checks into conditional types so as to use infer, which would be a rather large change, and possibly come with compiler performance implications.

[paarthenon]

@cwstra Thank you for your detailed overview of this. Just a heads up, I am recovering from a recent surgery and may take a couple of days to respond.

I see the concern you are raising. Lacking any common types, we need to drop the restriction and rely on conditional type checks which, like you mentioned, could complicate performance and possibly the user experience. I'm sorry I hadn't considered this.

The main goal of the change was to enable the processing of these functions with a match statement for the sake of things like procedural generation . We could also achieve this by

1. Expanding the match logic from accepting T | TType to T | TType | TCreator. In the case of a function, the match statement will look to .type (or .output.type) for the specific case instead of instance[key].
2. Creating a helper function to patch the discriminant onto the type at the last possible moment (input: VariantCreator<T, F, K>) => VariantCreator<T, F, K> & Record<K, T>.

I think I prefer the first approach. There is some risk of the match growing unwieldy as we tack on more edge cases, but I don't think we're quite there. These three (objects, raw strings, and variant creators) seem appropriate to me.

As far as this issue is concerned, I'd say we drop bullet 2 but I still see value in bullet 1. As is, the fact that type happens to work with match is sort of a happy accident that privileges type users.

How would you feel about that approach?

[cwstra]

On an initial read, seems reasonable enough to me. I'll mull it over 'til I get a proper look at it over the weekend.

[paarthenon]

Hey I see and appreciate this. An initial read of the code looked good but I've been too swamped to try it out. I hope to go through it soon.

[cwstra]

No worries. I kept you waiting long enough. ;)