F# option-wrapped values are only nullable if inner type is a reference type

[cmeeren]

Product

Hot Chocolate

Version

13.8.1

Link to minimal reproduction

See zip below

Steps to reproduce

Repro: HotChocolateRepro.zip

Code for quick reference (using HotChocolate.Types.FSharp with .AddFSharpTypeConverters():

type Foo = {
    A: string option
    B: int option
}

type Query() =
    member _.GetStuff(a: string option, b: int option) =
        { A = a; B = b }

What is expected?

This schema definition:

type Query {
  stuff(a: String, b: Int): Foo
}

type Foo {
  a: String
  b: Int
}

What is actually happening?

This schema definition:

type Query {
  stuff(a: String, b: Int!): Foo
}

type Foo {
  a: String
  b: Int!
}

Relevant log output

No response

Additional context

No response

[michaelstaib]

Can you open a PR with a snapshot test? Once the PR is there. I can help fix it.

[cmeeren]

Sure, can you point me in the right direction? Is there a test I can copy, and where should I place the new test?

[cmeeren]

Hi, just a reminder that I'm happy to contribute. Could you point me in the right direction? Is there a test I can copy, and where should I place the new test?

[aerlingsson]

I ran into this issue today as well. @cmeeren did you find a workaround?

[cmeeren]

I did not.

Again, @michaelstaib, I'd like to help out. You asked me to submit a PR with a snapshot test. Can you point me in the right direction? Is there a test I can copy, and where should I place the new test?

[aerlingsson]

Though not ideal, I got it to work with a TypeInterceptor. My Program.fs looks like this:

open Microsoft.AspNetCore.Builder
open Microsoft.Extensions.Configuration
open Microsoft.Extensions.Hosting
open Microsoft.Extensions.DependencyInjection
open HotChocolate.Configuration
open HotChocolate.Types
open HotChocolate.Types.Descriptors.Definitions
open System

// Option-wrapped value types are forcibly wrapped in Nullable instead
type NullableOptionTypeInterceptor() =
  inherit TypeInterceptor()

  override _.OnBeforeCompleteType(context: ITypeCompletionContext, definition: DefinitionBase) =
    match definition with
    | :? ObjectTypeDefinition as objDef ->
      for field in objDef.Fields do
        if
          field.Type.Context = TypeContext.Output
          && not(isNull field.ResultType)
          && field.ResultType.IsGenericType
          && field.ResultType.GetGenericTypeDefinition() = typedefof<Option<_>>
        then
          let innerType = field.ResultType.GetGenericArguments()[0]

          if innerType.IsValueType then
            let nullableType = typedefof<Nullable<_>>.MakeGenericType(innerType)
            let nullableTypeRef = context.TypeInspector.GetTypeRef(nullableType)
            field.Type <- nullableTypeRef
    | _ -> ()

[<EntryPoint>]
let main args =
  let builder = WebApplication.CreateBuilder(args)

  builder.Services
    .AddGraphQLServer()
    .TryAddTypeInterceptor<NullableOptionTypeInterceptor>() // Register the TypeInterceptor
    .AddFSharpTypeConverters() // Add the OptionTypeConverter
    .AddQueryType<Query>()
    .ModifyRequestOptions(fun opt -> opt.IncludeExceptionDetails <- builder.Environment.IsDevelopment())
  |> ignore

  let app = builder.Build()

  app.MapGraphQL() |> ignore

  app.Run()

  0

[michaelstaib]

@cmeeren ideally we could create a new F# testing project here: https://github.com/ChilliCream/graphql-platform/tree/main/src/HotChocolate/Core/test called Types.FSharp.Tests ...

And the we would add a couple of integration tests here to show were hotchocolate does not work ideally for the fsharp community. We are happy to help fixing these but but we need help for the Fsharp community here.

[Stock44]

I encountered this issue a few weeks back, so I created a type interceptor similar to what @aerlingsson created. However, mine extends it so that FSharp types are updated to handle nullability in a more FSharp native way, with non null values being the default. Here a gist with the code I'm using: FSharp Nullability Type Conventions

[cmeeren]

@Stock44, that sounds awesome. From a cursory look, it seems like it applies the non-null-by-default policy only for F# types. Is that correct? I'd argue that it's idiomatic in F# that all types are non-null by default; even BCL reference types such as string should be Option/ValueOption-wrapped to indicate nullability.

[Stock44]

I initially wanted to make it work like that, but the problem with that approach is dealing with types not defined in FSharp, mainly middlewares and types generated by HotChocolate itself. Types such as string and int get wrapped depending on their context, whether they're used within FSharp types and so. The code I shared doesn't account for ValueOptions though, it only checks for Option.

[cmeeren]

A WIP repo is up at cmeeren/HotChocolate.FSharp. It has F# nullability functionality and quite a lot of tests.

If that code is completed and merged into HotChocolate, then I guess most if not all of the F#-related issues in this repo are considered solved.

The nullability works on a simple rule: F# Option-based nullability is used for all fields defined in F# (regardless of where the types used in the args/inputs/outputs of those fields are defined). This strikes me as the expected functionality: If you write field in F#, you control the nullability in F#. This also applies to type extensions (F# nullability is used for F#-defined extensions to C# types, but not vice versa) as well as the items of pagination connections.

There is a limitation right now that I hope to solve: Option-wrapped enumerables are only supported at the top level (serialization fails for nested lists where the inner lists are Option-wrapped). The same applies to nested Option-wrapped enums (serialization fails for lists of Option-wrapped enums).

I also hope to add support for F# lists/sets on input as well as F# Async support, though I'm not sure how. I will follow the thread from #5348 for the former. The latter is tracked in my own #7023, but there has been no response there.

[cmeeren]

By the way, big thanks to @Stock44 for the initial gist, without which I couldn't even have started looking into this.

[cmeeren]

This is supported in FSharp.HotChocolate, now available on NuGet.

@michaelstaib, I'll leave to you the decision of whether to treat that as sufficient to close this issue, or wait until support is added directly to HotChocolate.