ChilliCream / graphql-platform

Welcome to the home of the Hot Chocolate GraphQL server for .NET, the Strawberry Shake GraphQL client for .NET and Banana Cake Pop the awesome Monaco based GraphQL IDE.
https://chillicream.com
MIT License
5.25k stars 744 forks source link

Support Option-wrapped lists #7472

Open cmeeren opened 1 month ago

cmeeren commented 1 month ago

Product

Hot Chocolate

Is your feature request related to a problem?

Currently working on #6884 by adapting the type interceptor from this gist. Unfortunately, I seem to be blocked by HotChocolate eagerly prohibiting Option-wrapped lists without actually calling the built-in OptionTypeConverter. That converter converts all option-wrapped values to their underlying representations (or null if the option value is None), and vice versa. However, HC does not invoke the converter and instead returns EXEC_LIST_TYPE_NOT_SUPPORTED whenever I try to query an option-wrapped list (array, in my case, though I believe it applies to all enumerables).

The solution you'd like

For example, make HotChocolate run any converters before determining whether the type is a supported list type.

cmeeren commented 1 month ago

For output fields, this can be solved with field middleware. However, for input types and arguments, I do not see the ability to use middleware.

michaelstaib commented 1 month ago

field middleware is costly performance wise only use when there is async code on that same field. Never use it to just convert a thing...

michaelstaib commented 1 month ago

Can you build some unit tests than I can point you to how to implement this.

michaelstaib commented 1 month ago

It might be that result formatters and input formatters are enough.

cmeeren commented 1 month ago

Thanks, I'll get a project with unit tests running and let you know.

(By the way, adding the middleware to the field definition seems to make this work for input types and arguments, too. No idea why.)

cmeeren commented 1 month ago

@michaelstaib, as mentioned in https://github.com/ChilliCream/graphql-platform/issues/6884#issuecomment-2363787392, a repo is up at cmeeren/HotChocolate.FSharp.

The important code is in Library.fs in the HotChocolate.FSharp project.

The part about unwrapping top-level option-wrapped lists is here (using this middleware).

Considering that that code may be merged into this repo at some point for proper built-in F# support in HC, you are more than welcome to look at the rest of the code in that file, specifically the Helpers module (and the short FSharpNullabilityInterceptor type definition). Additionally, for the same reason, I'd really appreciate some input on how to support Option-wrapped lists inside lists, and Option-wrapped enums inside lists.

Feel free to raise/comment on issues in that repo if that's more convenient to you. I'm happy as long as I get help.

michaelstaib commented 1 month ago

Yeah middleware is not meant for this ... try using a formatter .... Definition.FormatterDefinitions formatters work almost the same but are sync

michaelstaib commented 1 month ago
definition.FormatterDefinitions.Add(
            new ResultFormatterDefinition((ctx, result) => unwrap(result)));
michaelstaib commented 1 month ago

each input field btw can have Input formatters ... they exist on both sides ....

cmeeren commented 1 month ago

Excellent, thank you! I'll have a look at this.

If you have any insights about the other two items (Option-wrapped lists/enums/possibly other cases? and Async<_> support), then I think we'll be in a good place very quickly.

cmeeren commented 1 month ago

I can confirm that worked great, thanks!

Did not need them for inputs. Again I have no idea why it works without it, but I have bigger fish to fry, so I'll leave that.

cmeeren commented 1 month ago

Just want to state that FSharp.HotChocolate, now available on NuGet, has support for this.

@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.

michaelstaib commented 1 month ago

Can we pull the FSharp support entirely and your package deals with this?

michaelstaib commented 1 month ago

cc @glen-84

cmeeren commented 1 month ago

I guess that depends on what the current F# support is? I know about the Option type converter, but not more. That would be no problem (in fact, my library registers it anyway).

Also, it as mentioned in some other issues, "proper" F# support may still require some changes/additions to extension points the library can hook into.

cmeeren commented 1 month ago

FSharp.HotChocolate now includes OptionTypeFormatter and no longer has a dependency on HotChocolate.Types.FSharp. So if you want, HotChocolate.Types.FSharp can be deprecated with a note to use FSharp.HotChocolate instead.

By the way, I think it would be useful to have a doc section or similar about F# where you point users to FSharp.HotChocolate.

michaelstaib commented 1 month ago

OK ... we will remove F# support with V15 and leave it to you.

michaelstaib commented 1 month ago

I however do not think that more extension points are needed ... I think there might only be knowledge transfer needed ... but we can discuss it.

cmeeren commented 1 month ago

Absolutely, if there are existing ways to accomplish what we need, I'm all ears. (All current issues except # 10 requires "knowledge transfer" or adjustments to HC hooks.)

cmeeren commented 3 days ago

I however do not think that more extension points are needed ... I think there might only be knowledge transfer needed ... but we can discuss it.

@michaelstaib, about that knowledge transfer and discussion:

Since HC 15 removes F# support, I think FSharp.HotChocolate should aim for a feature complete 1.x release by the time HC 15 is out.

I have created a 1.0 milestone that lists the issues where I need this "knowledge transfer" or more extension points. I hope the issue descriptions (or the HC issues/discussions linked to there) are sufficient. Please don't hesitate to let me know if you need more information.