Allow passing string literal to custom scalar args

[ozanmakes]

This change allows the following query that's valid for Hasura:

mutation StartSessionMutation($sessionId: uuid!) {
  update_share_session_by_pk(
    pk_columns: { session_id: $sessionId }
    _set: { started_at: "now()" }
  ) {
    session_id
  }
}

started_at in this case has the type timestamptz which is then passed to Postgres as is (thus allows "now()" instead of an actual timestamp), but all custom scalars accept string literals.

[Zaid-Ajaj]

Hi @osener, although this is allowed by hasura, I find it a bit unsafe to allow any arbitrary string where timestamptz is expected. Some could say:

mutation StartSessionMutation($sessionId: uuid!) {
  update_share_session_by_pk(
    pk_columns: { session_id: $sessionId }
    _set: { started_at: "12344" }
  ) {
    session_id
  }
}

And snowflaqe would tell the users that the string is valid, which is not. Of course, it is possible to actually parse the string and check for exceptions (like "now()") but I would rather have the user go the type safe route and provide the value from code:

mutation StartSession($sessionId: uuid!, $startedAt: timestamptz!) {
  update_share_session_by_pk(
    pk_columns: { session_id: $sessionId }
    _set: { started_at: $startedAt }
  ) {
    session_id
  }
}

Then from your F# code:

open System 

client.StartSession { 
  sessionId = "7de395db-61ce-4e73-8231-e90c37f65fed"
  startAt = DateTimeOffset.Now
} 

Unless this is really problematic for you, I would like to keep the functionality as is. Please let me know otherwise

[ozanmakes]

Thanks for your quick response and the example!

In this particular instance there are possible differences like different timing because of latency or incorrect system time returned by the browser affecting this data. But maybe I can live with this. I think there are other things like "now()" Hasura accepts for different types that might not be as easy to work around though.

I'm on mobile and can't verify this, but outside timestamptz would the current version work for other sorts of custom scalars that the schema might be defining?

By the way I was under the impression that custom scalars can have any value, and not just strings, for example a scalar called NonNegativeInt (which would make my change here insufficient since I limited the value type to String). And that the validation would happen on server side.

It would of course be more convenient to accept any query (with literal values) GraphiQL accepts without a warning. I'm considering porting some code that uses BuckleScript + graphql-ppx where I didn't hit such a limitation.

[Zaid-Ajaj]

In this particular instance there are possible differences like different timing because of latency or incorrect system time returned by the browser affecting this data

The DateTimeOffset can be UtcNow instead of Now which sends UTC time to the server, though different time zones don't change what now means.

I'm on mobile and can't verify this, but outside timestamptz would the current version work for other sorts of custom scalars that the schema might be defining?

Outside of a couple of known custom types such as timestamptz which is a special case from hasura or DateTimeOffset from graphql-dotnet among others, all custom types defined in the schema become strings.

By the way I was under the impression that custom scalars can have any value, and not just strings, for example a scalar called NonNegativeInt (which would make my change here insufficient since I limited the value type to String). And that the validation would happen on server side.

Hmm I see, currently the code maps the NonNegativeInt simply to string. I think there is room for improvement here: for a while I was thinking of adding a configuration option to map custom types to other primitive types:

{
  "customTypeMappings": {
      "uuid": "String",
      "NonNegativeInt": "Int"
  }
}

if you feel comfortable implementing such feature, please go for it! I didn't implement it because the strings have worked just fine for my use cases as well much much more complicated use cases from people using hasura or the Github Api

It would of course be more convenient to accept any query (with literal values) GraphiQL accepts without a warning. I'm considering porting some code that uses BuckleScript + graphql-ppx where I didn't hit such a limitation.

That is great! Please open issues first to discuss the changes before actually working on a feature :smile:

[ozanmakes]

The DateTimeOffset can be UtcNow instead of Now which sends UTC time to the server, though different time zones don't change what now means.

Wouldn't it be affected by inaccurate system time though, compared to sending now()? Anyway, not that important for me to have first class support for this as long as I can find a workaround.

Hmm I see, currently the code maps the NonNegativeInt simply to string. I think there is room for improvement here: for a while I was thinking of adding a configuration option to map custom types to other primitive types:

{
  "customTypeMappings": {
      "uuid": "String",
      "NonNegativeInt": "Int"
  }
}

if you feel comfortable implementing such feature, please go for it! I didn't implement it because the strings have worked just fine for my use cases as well much much more complicated use cases from people using hasura or the Github Api

This makes a lot of sense to me. These PRs were the first lines of F# I've written, but it was fun and I'd be happy to give it a go! Here's some random thoughts to help me understand the right approach:

I believe the correct default type for custom scalar values are JSON, unless set through customTypeMappings. After all, they can be any JSON value including object. In fact, it is common to have a JSON scalar that can have arbitrary objects to work around the fact that GraphQL doesn't have a Heterogeneous Map type.

What do you think about a change to the default representation? In BuckleScript this would correspond to the built-in Js.Json.t abstract type. What is the equivalent of this in Fable? I saw that SimpleJson has a Json type, while Thoth.Json seems to use obj under the hood.

In OCaml I'd normally map scalars such as RGBA into an abstract type:

module Rgba : sig
  type t

  val parse : Js.Json.t -> t

  val serialize : t -> Js.Json.t
end

reason-relay has the same functionality you're suggesting, and it treats custom scalars as Js.Json.t.

graphql-ppx has a slightly different approach that amounts to the same thing.

Once we have customTypeMappings, I can leave timestamptz as the equivalent of Js.Json.t, or make it String. Either way, I'd like to be able to send "now()" as is or with an unsafe cast.

Does the current type for timestamptz allow sending this string value via an unsafe cast? I'd guess not since Snowflaqe will take care of conversion from/to DateTimeOffset?

Here's a popular library that can give you an idea of what sort of custom scalars people define: https://github.com/Urigo/graphql-scalars

That is great! Please open issues first to discuss the changes before actually working on a feature :smile:

I don't mind having these two closed, I was quickly trying to unblock my investigation into Fable :). Will do that in the future!

[Zaid-Ajaj]

In fact, it is common to have a JSON scalar that can have arbitrary objects to work around the fact that GraphQL doesn't have a Heterogeneous Map type.

Hmm I am a but skeptic about this, for someone working with a type-safe environment and choosing GraphQL to maintain the type-safety across platforms from backend to frontend, how useful really is that JSON type that basically can be anything?

What do you think about a change to the default representation? In BuckleScript this would correspond to the built-in Js.Json.t abstract type. What is the equivalent of this in Fable? I saw that SimpleJson has a Json type, while Thoth.Json seems to use obj under the hood.

Fable.SimpleJson introduces an intermediate Json type which represents an AST of the JSON content which could be used to represent the JSON scalars but I am not sure I want to implement features that break out of the type safety that the current implementation offers: you get a string back and then you can parse it into any shape or form

What I would like to do is solve these problems on a per use-case basis and support as many types of backends as possible like I am doing with hasura and others. So to get back to your query:

mutation StartSessionMutation($sessionId: uuid!) {
  update_share_session_by_pk(
    pk_columns: { session_id: $sessionId }
    _set: { started_at: "now()" }
  ) {
    session_id
  }
}

Is thing currently being validated as incorrect? if that is the case, I would like to fix it and support any string literal for custom types. However, if you want to provide the string value from runtime to timestamptz then that is also possible but encourages unsafe casts which are not recommended (you can say startedAt = unbox<DateTimeOffset> "NOW()")

[Zaid-Ajaj]

Alright never mind @osener, I figured that indeed your query wasn't being correctly validated (string literals weren't allowed) but now it should work just fine. I have published snowflaqe v1.10.3 which include the fix

[ozanmakes]

Hmm I am a but skeptic about this, for someone working with a type-safe environment and choosing GraphQL to maintain the type-safety across platforms from backend to frontend, how useful really is that JSON type that basically can be anything?

I don't like it at all either :). But I've come across dynamic configuration for stuff like forms served like that. At least an abstract JSON type tells you to parse this using Elm-like json decoders for safe access, unlike JS.

In the meantime I copied all my queries and mutations and latest version of Snowflaqe works perfectly with everything I threw at it! So I won't probably revisit this discussion unless I hit another edge case Hasura allows but Snowflaqe doesn't. Thanks for your amazing responsiveness and quick releases! Can't wait to play more with Feliz ecosystem too.

Next think I'll need to figure out are subscriptions. My app is heavily based on real-time capabilities of Hasura and previously I was letting Relay deal with this. This time around I was wondering if it would be a good idea to not use something like Relay or Urql, and instead hook subscription-transport-ws into something like Feliz.Recoil or Fable.Reaction to keep track of updated queries. Do you have any suggestions regarding the best approach to this?

[Zaid-Ajaj]

In the meantime I copied all my queries and mutations and latest version of Snowflaqe works perfectly with everything I threw at it!

Really happy to hear that 😍

instead hook subscription-transport-ws into something like Feliz.Recoil or Fable.Reaction to keep track of updated queries. Do you have any suggestions regarding the best approach to this?

Subscriptions are the very first thing on the roadmap, right now Snowflaqe ignores and doesn't do anything with subscriptions but I think at least generating the types (very easy to add) is one step forward and afterwards somehow generate observables from them, also using subscription-transport-ws but that is a bit harder to do mainly for me because I haven't used subscriptions before at all, so it involves quite a lot of learning before I could dive into the code-gen aspects of it