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

System.Text.Json fixes #876

Merged
merged 4 commits into from
Sep 8, 2022
Merged

System.Text.Json fixes #876

merged 4 commits into from
Sep 8, 2022

Conversation

andrewheumann
Copy link
Member

@andrewheumann andrewheumann commented Aug 29, 2022

BACKGROUND:

  • There were some issues using the STJ version of Elements for client-side geometry generation
  • There were also some issues building / code-generating hypar functions that worked correctly with STJ.

DESCRIPTION:

  • Fixes enum code generation — STJ's JsonStringEnumConverter ignores [EnumMember] attributes, which means JSON from hypar containing enums was frequently failing to deserialize and not serializing with the correct member names. the JsonStringEnumConverter is replaced with Macross.Json.Extensions's JsonStringEnumMemberConverter.
  • deserializing ProxyElements of types that were not loaded was throwing an exception.
  • Adds a generic test that reads some example models (which were previously failing). Over time we can add more real models here to test.

TESTING:

  • I tested by using this branch in tandem with elements/wasm-explore-fixes and explore/wasm-playground and testing client-side geometry generation on explore
  • I also tested this branch in tandem with hypar/stj. Functions aren't working all the way yet, since we're basically forced to upgrade to .net 6.0 — but most codegen and local building/running is working.

This change is Reviewable

Copy link
Contributor

@ikeough ikeough left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @andrewheumann)


Elements/src/Serialization/JSON/ElementConverter.cs line 115 at r1 (raw file):

                            {
                                // This may occur if the object is a proxy of a type we don't have the class for. Fallback to `Element`.
                                genericType = typeof(Element);

Very nice.


Elements.CodeGeneration/src/HyparFilters.cs line 56 at r1 (raw file):

        /// Strip the nullable annotation (?) from a type.
        /// </summary>
        public static string Removenull(string input)

Do we use this in this PR?

Copy link
Member Author

@andrewheumann andrewheumann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @ikeough)


Elements/src/Serialization/JSON/ElementConverter.cs line 115 at r1 (raw file):

Previously, ikeough (Ian Keough) wrote…

Very nice.

Done.


Elements.CodeGeneration/src/HyparFilters.cs line 56 at r1 (raw file):

Previously, ikeough (Ian Keough) wrote…

Do we use this in this PR?

Ah, good catch — I was using it, but now it's no longer needed with the third-party Enum converter.

Copy link
Contributor

@ikeough ikeough left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! 1 of 1 approvals obtained

@andrewheumann andrewheumann merged commit 1bdeb26 into stj Sep 8, 2022
@andrewheumann andrewheumann deleted the stj-ah branch September 8, 2022 13:52
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.

2 participants