Skip to content

Remove JsonValue.fs from client and migrate to System.Text.Json#576

Draft
Copilot wants to merge 4 commits into
devfrom
copilot/remove-jsonvalue-fs-use-system-text-json
Draft

Remove JsonValue.fs from client and migrate to System.Text.Json#576
Copilot wants to merge 4 commits into
devfrom
copilot/remove-jsonvalue-fs-use-system-text-json

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 17, 2026

The custom JsonValue.fs deserializer inferred numeric types from JSON structure rather than the GraphQL schema, causing schema-declared Float fields returning 0.0 to be deserialized as int option instead of float option.

Core fix

JsonValueHelper.getFieldValue in BaseTypes.fs now resolves the CLR type from the schema IntrospectionTypeRef when handling JsonValueKind.Number, ensuring Float 0float, Int 0int, etc.

| JsonValueKind.Number ->
    // Use the schema type to determine the correct numeric CLR type
    let innerTypeRef =
        match schemaField.SchemaTypeRef.Kind with
        | TypeKind.NON_NULL -> schemaField.SchemaTypeRef.OfType |> Option.get
        | _ -> schemaField.SchemaTypeRef
    let numVal = getNumericValue innerTypeRef fieldValue
    makeSomeIfNeeded numVal

Removed

  • JsonValue.fs and TextConversions.fs removed from both FSharp.Data.GraphQL.Client and FSharp.Data.GraphQL.Client.DesignTime project files

Replaced with System.Text.Json

  • Serialization.fs: New SchemaParser module manually traverses JsonElement to parse the introspection schema. Exposes defaultSerializerOptions (lazy, backed by Shared.Json.getSerializerOptions). Request JSON building uses JsonSerializer.Serialize via a normalizeForSerialization pre-processor that handles camelCase key normalization on nested IDictionary<string,obj> values, System.Uri → string, and Upload → form-part name. deserializeMap uses JsonElement.
  • BaseTypes.fs (JsonValueHelper): All pattern matching rewritten from JsonValue DU cases to JsonElement.ValueKind.
  • OperationResultBase: Now accepts the raw response string and parses it internally with JsonDocument (implements IDisposable); removes the JsonValue.Parse call from ProvidedTypesHelper.fs.
  • GraphQLClient.fs: Request/multipart construction rewritten; variables serialized via JsonSerializer.Serialize with the context's JsonSerializerOptions.
  • GraphQLProviderRuntimeContext: Exposes a JsonSerializerOptions property (defaults to defaultSerializerOptions) so callers can inject custom options (e.g. custom scalar converters). GetContext accepts an optional jsonSerializerOptions parameter.

Test updates

OperationErrorTests.fs: JsonValue.Parse calls replaced with raw JSON strings, matching the new OperationResultBase(string) constructor.

Copilot AI linked an issue May 17, 2026 that may be closed by this pull request
Copilot AI changed the title [WIP] Remove JsonValue.fs and use System.Text.Json for deserialization Remove JsonValue.fs from client and migrate to System.Text.Json May 17, 2026
Copilot AI requested a review from xperiandri May 17, 2026 19:59
Copy link
Copy Markdown
Collaborator

@xperiandri xperiandri left a comment

Choose a reason for hiding this comment

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

What is the sense of having so much custom logic?
Cannще we pass JsonSerializerOptions into context and use JsonSerialzer to serialize and JsonSerialzer or JsonDocument/JsonElement to deserialize? Anyway we have already built the output types in the type provider

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 17, 2026

What is the sense of having so much custom logic?
Cannще we pass JsonSerializerOptions into context and use JsonSerialzer to serialize and JsonSerialzer or JsonDocument/JsonElement to dese...

Good point. Implemented in commit 9f7f617:

  • JsonSerializerOptions is now a property on GraphQLProviderRuntimeContext (defaults to Serialization.defaultSerializerOptions.Value)
  • GetContext accepts an optional jsonSerializerOptions parameter so callers can inject custom options (e.g. with custom scalar converters)
  • buildRequestJson now takes JsonSerializerOptions and uses JsonSerializer.Serialize for the variables object
  • A normalizeForSerialization helper pre-processes variable values before handing them to JsonSerializer.Serialize to handle edge cases that STJ doesn't cover out of the box (camelCase key normalization on nested IDictionary<string,obj> values, System.Uri → string, Upload → file name string)

The result is that the bulk of the variable serialization is now done by JsonSerializer with the provided options, and users can customize it by passing their own JsonSerializerOptions to GetContext.

Copilot AI requested a review from xperiandri May 17, 2026 20:38
@xperiandri xperiandri force-pushed the copilot/remove-jsonvalue-fs-use-system-text-json branch from 73fdc42 to 933ec05 Compare May 17, 2026 21:16
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.

Remove JsonValue.fs from client and use System.Text.Json

2 participants