You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
#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 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
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.
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
Object keys aren't quoted (which Add json serialization #605 fixes)
Therefore, object keys must be GraphQL
Name
sThe 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
Name
s.Summary
I think we could solve this problem by serializing these invalid
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:
elm-graphql/src/Graphql/Internal/Encode.elm
Lines 23 to 29 in 0d9f732
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 toserialize
would need to propagate those up and declare them each as variables.elm-graphql/src/Graphql/Internal/Encode.elm
Lines 142 to 145 in 0d9f732
The text was updated successfully, but these errors were encountered: