rungwiroon / BlazorGoogleMaps

Blazor interop for GoogleMap library
MIT License
319 stars 102 forks source link

Fix bug #293 Incorrect serialization of Enums #294

Closed cruiserkernan closed 9 months ago

cruiserkernan commented 9 months ago

Fix Incorrect Serialization of Enums in BlazorGoogleMaps

Description

This pull request addresses a critical issue in the BlazorGoogleMaps repository concerning the incorrect serialization of enums. The issue not only affected the positioning of map controls but might have also impacted other parts of the library due to improper use of the EnumMember attribute in JSON serialization.

Changes Made

Impact

Please review the changes and let me know if there are any further modifications required. Looking forward to the feedback.

Thank you!

valentasm1 commented 9 months ago

I was kinda aware of this. When i finished migration to system.text.json i had some issues did quick fixes and all seems working. IMO this PR is just shifting responsibility from prop atribute to enum. I am not saying it is wrong. Need to think more about this.

cruiserkernan commented 9 months ago

Let me know if you need any help implementing it any other way, if you do not have time to do it.

valentasm1 commented 9 months ago

I am thinking this way. Added serializer to enum is harder to forget than compared to prop. Thats obvious. Just lats thing. Why you cache converters for each enum type? For all types it is the same Why not just return it?

cruiserkernan commented 9 months ago

Since there are some code running in the EnumConverter constructor for each enum type. I am caching the creation of each EnumConverter so that it only happens once. But maybe it is not needed?

valentasm1 commented 9 months ago

I dont see any reason why you need to cache it. If you know any let me know. I just tried create 1mln converters and it took 15ms

cruiserkernan commented 9 months ago

I noticed while using this change in my app that it does not handle nullable enums. I made a change to the converter that takes care of it. I'm not sure if that is of any interest for you? Also as a side note I noticed when using these code changes that animation of markers started working.

valentasm1 commented 9 months ago

Handling nullable enums is important. Maybe i didnt noticed but marker animations was always working. I will try merge this today.