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

Idea: serialize invalid JSON Input Values as arguments #606

Open
dillonkearns opened this issue Aug 25, 2022 · 0 comments
Open

Idea: serialize invalid JSON Input Values as arguments #606

dillonkearns opened this issue Aug 25, 2022 · 0 comments

Comments

@dillonkearns
Copy link
Owner

Motivation

#605 addresses #570, but also makes me think of one longer term thing for a future change. This PR causes errors in fewer cases and doesn't introduce any new error cases so I think it's a great starting point. Though it does give something that people could rely on that could still cause errors, so it's worth considering those things for the future.

The GraphQL spec allows for Input Values that are exactly the same as what JSON allows, except

  1. Object keys aren't quoted (which Add json serialization #605 fixes)

  2. Therefore, object keys must be GraphQL Names

The longer-term issue is that we're allowing arbitrary JSON values to be sent, but the spec doesn't actually allow that. If people start depending on this functionality then they can hit up against cases where there's no way to send certain values because they aren't valid GraphQL Names.

Summary

I think we could solve this problem by serializing these invalid

mutation Mutation($jsonABC: MyJsonScalar!) {
  saveJsonBlob(input: $json1) {
      # ...
  }
}

Implementation Ideas

This type could change to better capture the full valid GraphQL Input Values, and try to turn values into non-JSON values whenever possible:

{-| Represents an encoded Value
-}
type Value
= EnumValue String
| Json Json.Encode.Value
| List (List Value)
| Object (List ( String, Value ))

Then for each Input Value that can't be serialized, the Encode.serialize function could hash the unserializable JSON and use that hash to refer to a variable name (similar to how field aliases are hashed). And it could change its type signature to return a (possibly empty) Set (String, String), where each tuple is a variable name that needs to be declared and its type name, like ( "jsonABC", "MyJsonScalar!" ). Then the recursive calls to serialize would need to propagate those up and declare them each as variables.

{-| Low-level function for serializing a `Graphql.Internal.Encode.Value`s.
-}
serialize : Value -> String
serialize value =

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

1 participant