Skip to content
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

[TS] Make discriminated union .Is* properties works #3982

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

MangelMaxime
Copy link
Member

I am not sure why but casting the tag to number make some others scenarios fails:

type Test<'T> =
    | A
    | Value of 'T

let testResult = Value "test"

printfn "%A" testResult.IsA
printfn "%A" testResult.IsValue

let apply (f: 'A -> 'B) (value : Test<'A>) : Test<'B> =
    match value with
    | A -> A
    | Value v -> Value (f v)
(function () {
    const arg: boolean = testResult.tag === (/* A */ 0 as int32);
    toConsole(printf("%A"))(arg);
})();

(function () {
    const arg: boolean = testResult.tag === (/* Value */ 1 as int32);
    toConsole(printf("%A"))(arg);
})();

export function apply<A, B>(f: ((arg0: A) => B), value: Test$1_$union<A>): Test$1_$union<B> {
    if (value.tag === (/* Value */ 1 as int32)) {
        const v: A = value.fields[0]; // <===================== Error here
        return Test$1_Value<B>(f(v));
    }
    else {
        return Test$1_A<B>();
    }
}
Type 'A | undefined' is not assignable to type 'A'.

But without the cast then in some scenario Is* don't works with TypeScript is able to compare the literal value directly.

@MangelMaxime MangelMaxime requested a review from ncave December 18, 2024 21:03
@MangelMaxime
Copy link
Member Author

By using as any I am able to make the compiler happy (no I am not hiding stuff under the rug).

Looking at the generated code it seems like even if we use as any it happens in places that are internal to Fable code and not expose to the outside world.

type Test<'T> =
    | A
    | Value of 'T

let apply (value : Test<'A>)  =
    match value with
    | A -> failwith "Cannot apply function to A"
    | Value v -> v

generates

export function apply<A>(value: Test$1_$union<A>): A {
    if ((value.tag as int32) === /* Value */ 1) {
        const v = value.fields[0] as any;
        return v;
    }
    else {
        throw new Error("Cannot apply function to A");
    }
}

You can see that the v end up typed in via the output of the function.

I tried to cast the value to the concrete type of the fields[x] but it caused issues at other places mostly because we are generating curried function instead of standard function.

@joprice
Copy link
Contributor

joprice commented Dec 18, 2024

Does the as int32 widen the discriminator type and lose the more specific type information that would allow the flow inference to figure out what type the field should be?

@MangelMaxime
Copy link
Member Author

Does the as int32 widen the discriminator type and lose the more specific type information that would allow the flow inference to figure out what type the field should be?

int32 is an alias to number and it does indeed widen the type.

This is need because TypeScript is "too smart" and when we do value.tag === (/* Value */ 1 it does not consider 1 as a number but a literal type which has only 1 as the possible value.

This is problematic because if you do something like:

type Test =
    | A
    | Value of string

let testA = A

testA.IsA
testA.IsValue

then we generate

(testA.tag) === /* A */ 0;

(testA.tag) === /* Value */ 1;

resulting in this error

CleanShot 2024-12-19 at 08 47 12@2x.

So my idea was to say to TS that tag is actually a number to allow IsValue to work in this situation. But changing that caused issue to the apply generation (see original comment).

@MangelMaxime
Copy link
Member Author

Strangely if the union has a generic the problem does not occur because TS infer tag to be 0 | 1 and not just 0 or 1 depending on the situation.

The code generation is little different with the generic:

type Test =
    | A
    | Value of string

type TestWithGeneric<'T> =
    | A
    | Value of 'T

testGenericA.IsValue
testA.IsValue
(testGenericA<any>().tag as int32) === /* Value */ 1;
// Hover on tag gives:
// (property) TestWithGeneric$1<T, Tag extends keyof TestWithGeneric$1_$cases<T>>.tag: 0 | 1

(testA.tag as int32) === /* Value */ 1;
// Hover on tag gives:
// (property) Test<0>.tag: 0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants