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

Flags Enum Deserialization no longer works #38

Closed
0x53A opened this issue Nov 11, 2019 · 3 comments
Closed

Flags Enum Deserialization no longer works #38

0x53A opened this issue Nov 11, 2019 · 3 comments

Comments

@0x53A
Copy link
Contributor

0x53A commented Nov 11, 2019

After updating Fable.Remoting to the latest version (which updated Fable.SimpleJson from 3.2 to 3.8), I can no longer deserialize a [<Flags>] enum with multiple bits set.

This worked before because it was just treated like an int.

I assume the issue is here: 83f12b8#diff-dd3f6f39b238e426d14aaeaf582f183cR128

For a correcter validation, it would need to check whether it has the FlagsAttribute, then check either each individual bit, or the whole thing.

But to be honest, my preference would be to drop the check, because in .NET you can do

enum E {  A = 1, B = 2 }
E e = (E)0xff;
@Zaid-Ajaj
Copy link
Owner

Hello @0x53A, sorry for introducing the regression, it looks like I didn't have a proper test for enums with flags. Do you have an example for me that fails so I can look at it? A PR would be even better yet 😄

my preference would be to drop the check

That is fine by me if it is the only way to fix this

it would need to check whether it has the FlagsAttribute, then check either each individual bit, or the whole thing.

We don't have access to attribute metadata in Fable :/

@0x53A 0x53A mentioned this issue Nov 13, 2019
@0x53A
Copy link
Contributor Author

0x53A commented Nov 13, 2019

Hi @Zaid-Ajaj I added a test here: #39

That is fine by me if it is the only way to fix this

I guess this is also a bit a philosophical question.

In .net you can assign "invalid" values to an enum (like in my example above), so the question is whether you want to detect such cases, constraining them more than .net normally does, or not.

@Zaid-Ajaj
Copy link
Owner

I guess it's alright in this case 😄 released v3.8.1 with the fix 🚀 thanks a lot for the PR. I will be able to patch Fable.Remoting.Client soon

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

No branches or pull requests

2 participants