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

Deserialize variables in Values.fs #327

Closed
xperiandri opened this issue Mar 23, 2021 · 5 comments
Closed

Deserialize variables in Values.fs #327

xperiandri opened this issue Mar 23, 2021 · 5 comments

Comments

@xperiandri
Copy link
Collaborator

Description

I propose to move variables deserialization into

let rec private coerceVariableValue isNullable typedef (vardef: VarDef) (input: obj) (errMsg: string) : obj =
and pass Map<string, obj>where obj can be either System.Text.Json.JsonElement or another Map<string, obj>.
Because in that function we have all the required info to correctly get value from JSON.

This way a scalar coerce function will always receive System.Text.Json.JsonElement and for objects options with converters can be applied.

@jberzy what do you think?

Alternatives

Define an interface that allows to convert arbitrary JSON toke (i.e. JsonElemen to JToken) to a requested value type and pass it.

@xperiandri
Copy link
Collaborator Author

Or actually, deserialization can happen in Executor.AsyncExecute, after the plan is created and before it is executed.
At that point, we already know variables types. So we can pass a deserialization function to Executor.AsyncExecute

@njlr
Copy link
Contributor

njlr commented Sep 17, 2021

I recently hit this issue in a nasty way, so would appreciate more type-safety in this area.

Some thoughts...

I'm not sure if System.Text.Json.JsonElement is the best choice since it ties the user to a particular JSON implementation. Perhaps the library should provide its own JSON-like type?

Anyway, I was thinking what an implementation might look like.

Taking the idea from Thoth.Json, we can have decoders like so:

type Decoder<'t> = JsonElement -> Result<'t, string>

The library could provide common decoders like int and string.

To apply this concept, the user would first define an F# type for their input:

type FooInput =
  {
    Bar : int
    Qux : string list
    Baz : string option
  }

Then a corresponding decoder:

let fooInputDecoder : Decoder<FooInput> =
  (fun el ->
    result {
      let! bar = el |> Decode.property "bar" Decode.int
      let! qux = el |> Decode.property "qux" (Decode.list Decode.string)
      let! baz = el |> Decode.property "baz" (Decode.optional Decode.string)

      return
        {
          Bar = bar
          Qux = qux
          Baz = baz
        }
    })

(Note that using reflection we can provide a default decoder for arbitrary types)

Then the decoder would be passed to the GraphQL type definition:

let fooInputObjectDef =
  Define.InputObject<FooInput>
    (
      "FooInput",
      [
        Define.Input ("bar", Types.SchemaDefinitions.Int)
        Define.Input ("qux", Types.SchemaDefinitions.ListOf Types.SchemaDefinitions.String)
        Define.Input ("baz", Types.SchemaDefinitions.Nullable Types.SchemaDefinitions.String)
      ],
      fooInputDecoder
    )

I think it would be quite easy for Values.fs to find the decoder - or it could run elsewhere.

Any thoughts?

@njlr
Copy link
Contributor

njlr commented Sep 26, 2021

I had a go here: #350

@njlr
Copy link
Contributor

njlr commented Oct 6, 2021

Implementation of "decoders" idea here #351

@xperiandri
Copy link
Collaborator Author

Implemented in #418

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

No branches or pull requests

2 participants