graphql-ppx icon indicating copy to clipboard operation
graphql-ppx copied to clipboard

Custom Scalar parsing/serializing not working in InputObjects

Open illusionalsagacity opened this issue 2 years ago • 10 comments

Reproduction: https://github.com/illusionalsagacity/graphql-ppx/commit/9630e024722b813603161dbfb8c181f73f7b4efd#diff-dccca68100be513fb2d07560888ab2c0785e6033bc6babd303fb6bfe07810f1dR52

tests_bucklescript/__snapshots__/Generate_Apollo_scalarsInput_re.bda5f930.0.snapshot line 52

when using customFields, the Scalar types in input objects do not seem to be using those custom parsers and instead show up as option(Js.Json.t)

Looks like this was just missing from the test cases?

illusionalsagacity avatar Nov 16 '21 19:11 illusionalsagacity

Hi @illusionalsagacity, I have not used GraphQL much. So please correct me if I am wrong.

According to the GraphQL spec the client must serialize the value of the scalar type. So it makes sense of graphql-ppx accepting a type Option(Js.Json.t). We might have to to use something like https://rescript-lang.org/docs/manual/v8.0.0/api/js/json#number and pass the result as input.

image

a-c-sreedhar-reddy avatar Nov 16 '21 20:11 a-c-sreedhar-reddy

My expectation was that since we've defined a customFields module for this DateTime scalar, it would be handled by the PPX for both responses and inputs; otherwise why does the module require a serialize function for the custom scalar handling?

https://github.com/reasonml-community/graphql-ppx/blob/ebb140459493f3f440f0c34f7ff3f34632fa1dad/tests_bucklescript/operations/customTypes.re#L48

https://graphql-ppx.com/docs/custom-fields

illusionalsagacity avatar Nov 16 '21 20:11 illusionalsagacity

Oh graphql-ppx allows us to provide a decoder! Great. Then I think this is bug.

a-c-sreedhar-reddy avatar Nov 17 '21 05:11 a-c-sreedhar-reddy

Custom fields have not been implemented for input objects indeed! I definitely would welcome a PR if you are up for it.

jfrolich avatar Nov 17 '21 08:11 jfrolich

Custom fields have not been implemented for input objects indeed! I definitely would welcome a PR if you are up for it.

Any idea on where/what would a starting point for this would be? Looking around in src/base/result_decoder.re at the moment.

illusionalsagacity avatar Dec 02 '21 01:12 illusionalsagacity

When using the GitHub GraphQL API, there used to be a workaround: it was possible to use String instead of custom scalars (like GitObjectID) because the types were not enforced. Following a recent update of one of their dependencies, this workaround no longer exists, making the issue more pressing. Check coq/bot#203 for details.

Zimmi48 avatar Mar 29 '22 12:03 Zimmi48

Custom scalars should be just the JSON type. So if it's a string natively, you need to convert it to a JSON type (YoJson on native, Js.Json.t on ReScript).

jfrolich avatar Mar 30 '22 09:03 jfrolich

It's actually not related to this issue, and should just work.

jfrolich avatar Mar 30 '22 09:03 jfrolich

I meant custom fields in the context of input objects (like you said above had not been implemented yet).

Zimmi48 avatar Mar 30 '22 10:03 Zimmi48

I meant custom fields in the context of input objects (like you said above had not been implemented yet).

Custom fields are just an ergonomic way to automatically convert custom scalars (or other custom fields). But you can use custom scalars in input objects, you just have to convert to a Json type.

jfrolich avatar Mar 30 '22 10:03 jfrolich