[Enhancement] StringEnum can opt-in to respect CompiledValueAttribute's on cases

[shayanhabibi]

• Adds optional boolean switch as second positional argument which toggles StringEnum respecting CompiledValue attributes.
• Adds tests for StringEnum respecting CompiledValue
• Adds remarks to xml docs to add this usage possibility

Example

Before:

[<StringEnum(CaseRules.LowerFirst)>]
type Behavior =
    | Replace
    | Combine
    | [<CompiledValue(false)>] None

console.log Behavior.None // 'none'
console.log Behavior.Replace // 'replace'

Now:

[<StringEnum(CaseRules.LowerFirst, true)>]
type Behavior =
    | Replace
    | Combine
    | [<CompiledValue(false)>] None

console.log Behavior.None // false
console.log Behavior.Replace // 'replace'

[shayanhabibi]

How do I manage the error with the standalone test build failure?

[MangelMaxime]

I not 100% sure but I think the Fable Standalone error is because we need to re-generate the metadata dll.

But before that what is the need behind this change?

I am not a fan of having a switch in StringEnum which then do something if we add another attribute somewhere else. It doesn't feels easy to understand to the user.

Potentially, we could decide that CompiledValue take precedence over StringEnum and so user would just need to add CompiledValue without having to pass a flag in StringEnum

[shayanhabibi]

Only because it could be 'potentially' a change which impacts code bases. I, for instance, was not aware that compiled value didnt work with StringEnums, and only found when debugging some errors. But more importantly, I don't know how this would effect things like TypeScript generation. Judging from your comment though, this is something you would be fine with? Honestly my preference is to just have CompiledValue take precedence too. Perhaps since we're doing a major version change, this is a better time than any to make the change. What do you think?

As for the reason behind this change, it is simply the fact that many enums in javascript land may consider true or false as independent special values, while at all other times they are strings.

[MangelMaxime]

But more importantly, I don't know how this would effect things like TypeScript generation.

I don't use TypeScript generation personally so I am not sure, but from my perspective if the CI is green then it "probably" means we didn't break anything.

Honestly my preference is to just have CompiledValue take precedence too.

Can you please do it this way?

We also need to add tests for all the different values that CompiledValue supports and check that the output is supported by JavaScript/TypeScript.

As for the reason behind this change, it is simply the fact that many enums in javascript land may consider true or false as independent special values, while at all other times they are strings.

Not 100% related but this feels similar to https://github.com/glutinum-org/cli/issues/111

And, from what I know I don't think we have a way in pure Fable/F# (without emit) to represent these types.

[shayanhabibi]

Tried to check [<CompiledValue(null)>] against JS.undefined but got this:

so I removed that test.

Everything else works as expected.

[MangelMaxime]

Regarding the failing CI, I will consider it normal and we are going to disable the test for Fable Standalone.

This is because on JavaScript we can't make the distinction between an int or a float:

match box 42 with
| :? int as value -> printfn "Int"
| :? float as value -> printfn "Int"
| _ -> printfn "Unknown"

compiles to

import { printf, toConsole } from "fable-library-js/String.js";

(function () {
    const matchValue = 42;
    if (typeof matchValue === "number") {
        const value = matchValue | 0;
        toConsole(printf("Int"));
    }
    else if (typeof matchValue === "number") {
        const value_1 = matchValue;
        toConsole(printf("Int"));
    }
    else {
        toConsole(printf("Unknown"));
    }
})();

There are ways to have a better distinction, but it will never be able to make a difference between 3 and 3.0 for example