teamwalnut / graphql-ppx

GraphQL language primitives for ReScript/ReasonML written in ReasonML
https://graphql-ppx.com
MIT License
258 stars 53 forks source link

`FutureAddedValue` not a member of input enums #203

Closed amsross closed 3 years ago

amsross commented 4 years ago

Consider:

enum _WordEnum { foo bar baz }
type ThingType { word: _WordEnum! }
input ThingInput { word: _WordEnum! }

query ThingQuery($thingInput: ThingInput!) {
  things(thing: $thingInput) {
    word
  }
}

You end up with something like: ThingType.t_word = [ | `foo | `bar | `baz | `FutureAddedValue(string) ]; and ThingQuery.t_ThingInput.t_word = [ | `foo | `bar | `baz ];

The two types conflict, so you are unable to use a result of ThingType as an input to ThingQuery, although they both use the same _WordEnum:

  This has type:
    ThingType.t_word
  But somewhere wanted:
    [ `foo
    | `bar
    | `baz ]
  The second variant type does not allow tag(s) `FutureAddedValue
jfrolich commented 4 years ago

Yes it might be a good idea to accept it, let me check if it has other implications! The thinking currently is that it is a case that the user has to consider when it gets back data from the server (on an older client), and when you input it into a mutation you have to filter the future added value out and handle that case. If in practice you never have FutureAddedValue (static enums) you can also use the ppxOmitFutureValue directive (https://graphql-ppx.com/docs/directives#ppxomitfuturevalue).

amsross commented 4 years ago

In the case where I ran into this, it is likely that the enum will update more frequently than I would (prefer to) update the client, so ppxOmitFutureValue isn't terribly appealing. That doesn't mean it's out of the question, obviously.

At the same time, I'm having a hard time wrapping my mind around how this would work anyway, if it did. The only thing I could think of would be to somehow change the input's enum type to a string and skip the conversion to a polymorphic variant altogether. Then I would be free to make decisions about ThingQuery.t_word, convert them to strings, and then pass those strings forward as the input to ThingQuery.t_ThingInput.t_word. I don't believe I have any access to or control over ThingQuery.t_ThingInput when I write the query, though, correct? This seems like it's fairly close to how GraphQL works in practice as it is, but that may not translate sensibly to reason.

jfrolich commented 4 years ago

Yes I think having `FutureAddedValue(string) as part of the input type makes sense, then you can pass any string. If you got that in the first place back from the API you KNOW it's part of the schema.

amsross commented 4 years ago

In the short term, is it possible to disable the conversion of a property on a fragment or an input to a polymorphic variant using the currently available tools?

jfrolich commented 3 years ago

In the short term, is it possible to disable the conversion of a property on a fragment or an input to a polymorphic variant using the currently available tools?

You mean the conversion from poly variant to string? Yes we can remove that as it's zero cost now (we have bump up the minimal bs-platform version though. PR is welcome!

amsross commented 3 years ago

What I mean to ask is really a two-part question:

  1. Is it possible to put a directive on an input type somehow? I wasn't able to discern this from the docs or the code. (re: https://github.com/reasonml-community/graphql-ppx/issues/194#issuecomment-680856066)
  2. Is it currently possible to disable the conversion of enum->polyvariant for some property on a type (input, fragment, query, etc)?

Regarding a PR, I took a stab at adding FutureAddedValue to input types before I opened this issue but I struggled with getting the project to build with esy, having never used that path before. Once I was able to get it to build, I wasn't able to npm link the built code to manually test its usage after failing to decipher what the test failures were telling me. This seems like it might be another issue asking for a "how to get started" doc that makes fewer assumptions about the reader or a more general request for a "how to reason/dune/esy" reference.