-
Notifications
You must be signed in to change notification settings - Fork 310
[Enhancement] StringEnum can opt-in to respect CompiledValueAttribute's on cases #4144
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
base: main
Are you sure you want to change the base?
[Enhancement] StringEnum can opt-in to respect CompiledValueAttribute's on cases #4144
Conversation
- 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
How do I manage the error with the standalone test build failure? |
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 Potentially, we could decide that |
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. 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. |
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.
Can you please do it this way? We also need to add tests for all the different values that
Not 100% related but this feels similar to glutinum-org/cli#111 And, from what I know I don't think we have a way in pure Fable/F# (without emit) to represent these types. |
…types. CompiledName takes precedence over CompiledValue
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 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 |
Example
Before:
Now: