Implement Discriminated Unions #149
Conversation
If: - all the alternatives of a union are records - they have at least one field in common, that is a literal in each record with unique values Then when checking the alternatives we can pick the correct one first, and then perform the validation, thus producing more specific error messages. This is something already present in TypeScript itself https://www.typescriptlang.org/docs/handbook/advanced-types.html#discriminated-unions Though this implementation would not work in all the cases covered by TS, but its a start.
src/types/union.ts
Outdated
| @@ -966,6 +968,46 @@ export function Union(...alternatives: Rt[]): any { | |||
|
|
|||
| return create( | |||
| (value, visited) => { | |||
| if ( | |||
| alternatives.length > 1 && | |||
There was a problem hiding this comment.
Why is this condition necessary?
There was a problem hiding this comment.
I wouldn’t call it necessary, just a performance optimization that we can easily get rid off. If you think its not worth it I can remove it for sure. It might help with coverage as well.
Also we can implement some of the checks with type guards and that would help with coverage as well, but I figured I’d use existing types as much as possible. The current way Reflect is implemented I can’t see a way to reuse the types for typeguards.
There was a problem hiding this comment.
Yeah, I think for simplicity's sake we should remove it.
|
Hey all! Very interested in seeing this come through :). This is a godsend library, and I love to see it improving like this. |
First of all, AWESOME library! One thing has been bugging me about it though and its support for Discriminated Unions.
This is my attempt on addressing it, and I've seen similar issues being raised too.
So with this PR, when those conditions are met:
record with unique values
Then when checking the alternatives we can pick the correct one first, and then perform the validation, thus producing more specific error messages.
This is something already present in TypeScript itself https://www.typescriptlang.org/docs/handbook/advanced-types.html#discriminated-unions and seems to have been requested before #67 .
Though this implementation would not work in all the cases covered by TS, but it's a start.
I'll be glad to add more tests or work on the implementation on your direction. Sorry for the long implementation, but the you seem to be supporting way older version of JS than I'm used to so had to implement the logic in for loops and such.