Skip to content

Switch to System.Text.Json #3375

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 52 commits into from
Jan 12, 2022
Merged

Switch to System.Text.Json #3375

merged 52 commits into from
Jan 12, 2022

Conversation

pbiggar
Copy link
Member

@pbiggar pbiggar commented Jan 12, 2022

Several functions in Dark used JSON.Net, which supported some of the esoteric features of OCaml's Yojson, such as generating and parsing tokens like NaN and Infinity.

However, .NET is moving to using System.Text.Json, an idiomatic, extremely performant Json parser/serializer, that also plays nicer with Blazor.

JSON.Net has some differences with Yojson, and also introduces some surface area of its own. That means that moving to STJ via JSON.Net will probably introduce more problems that just going straight to STJ (even if STJ doesn't support some non-JSON-complaint things we were parsing/generating before).

This PR splits the JSON functions into internal (DvalReprInternal) and external (DvalReprExternal) ones:

  • external ones are moving straight to STJ, and we need to warn users about the very minor differences (these have been added to the changelog)
  • internal ones can safely be moved later without notifying users, so there's no reason to change now (and likely things will break if we do)

The work was tracked in #3127

Major changes:

  • Formatting:
    • now using STJ:
      • toPrettyMachineJson
      • toPrettyResponseJson
      • ofUnknownJsonV0
    • Still using Json.Net:
      • toInternalQueryableV1
      • ofInternalQueryableV1
      • toInternalRoundtrippableV0
      • ofInternalRoundtrippableV0
      • LibJwt
    • Using something else:
      • BackendOnlyStdLib.LibHttpClient0.PrettyRequestJson.toPrettyRequestJson (just printing strings)
      • LibExecutionStdLib.LibObject.PrettyResponseJsonV0.toPrettyResponseJsonV0

Modules:

  • now moved to DvalReprExternal:
    • json stuff (all using STJ)
      • toPrettyMachineJsonStringV1
      • ofUnknownJsonV0
    • other serializations
      • toDeveloperRepr
      • toEnduserReadableTextV0
    • misc web stuff
      • typeToBCTypeName
      • typeToDeveloperReprV0
      • toFormEncoding
      • ofFormEncoding
      • toQuery
      • ofQueryString
      • queryToEncodedString
      • toStringPairs
  • now moved to DvalReprInternal:
    • hash
    • toInternalRoundtrippableV0
    • ofInternalRoundtrippableV0
    • toInternalQueryableV1
    • ofInternalQueryableV1
    • currentHashVersion
  • now moved to other modules
    • BackendOnlyStdLib.LibHttpClient0.PrettyRequestJson.toPrettyRequestJson

Refactoring:

  • removed {of,to}_internal_queryable_v0, which wasn't used anywhere
  • inlined a bunch of functions for clarity:
    • ofUnsafeDvalV0 into ofUnknownJsonV0

Json testing:

Misc Testing:

They would break if they were run twice in a row
The json parser was used to parse test results, and it glossed over a lot of tests where the output was a bit different.
@pbiggar pbiggar force-pushed the paul/switch-to-stj branch from a7963b5 to 4fc820a Compare January 12, 2022 05:09
@pbiggar pbiggar merged commit eca8626 into main Jan 12, 2022
@pbiggar pbiggar deleted the paul/switch-to-stj branch January 12, 2022 06:23
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.

1 participant