Skip to content

Fix for enums. #32

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

Merged
merged 2 commits into from
Feb 14, 2025
Merged

Fix for enums. #32

merged 2 commits into from
Feb 14, 2025

Conversation

bitzblitz
Copy link
Contributor

Write and read enum fields correctly.

@yueyinqiu
Copy link
Collaborator

yueyinqiu commented Feb 14, 2025

Really appreciate it that you have also added some tests.

I have reformat some of the code, and change the implemention to:

            if (targetType.IsEnum)
            {
                return Enum.ToObject(targetType, Convert.ToInt64(value));
            }

But it reveals another problem. When I was trying:

            if (targetType.IsEnum)
            {
                return Enum.ToObject(targetType, value);
            }

It failed because value is decimal.

I believe this happens due to the deserialization from js number. I wonder whether this will cause problems when it's a big interger. I would check this when fixing #24.

@yueyinqiu
Copy link
Collaborator

yueyinqiu commented Feb 14, 2025

@magiccodingman Maybe you should also check this as it was rejected in #25. I believe enums should work like this.

@magiccodingman magiccodingman merged commit 3209084 into magiccodingman:master Feb 14, 2025
@bitzblitz
Copy link
Contributor Author

bitzblitz commented Feb 14, 2025 via email

@magiccodingman
Copy link
Owner

@bitzblitz I can just package a new one for you in the morning when I wake up and post it.

@yueyinqiu
Copy link
Collaborator

yueyinqiu commented Feb 15, 2025

However the MagicNotMapped fix has not been done yet. Maybe we could create a release when there is a new version published?

@bitzblitz
Copy link
Contributor Author

bitzblitz commented Feb 15, 2025 via email

@yueyinqiu yueyinqiu mentioned this pull request Feb 16, 2025
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.

3 participants